[Bug 472098] Review Request: dekiwiki - a powerful opensource wiki which runs on Mono

bugzilla at redhat.com bugzilla at redhat.com
Thu Nov 20 14:35:58 UTC 2008


Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=472098





--- Comment #3 from Andreas Thienemann <andreas at bawue.net>  2008-11-20 09:35:56 EDT ---
David mentioned you'll be needing someone sponsering you. I'll therefore
continue with the review.

About the dependencies: RPM as used by fedora currently does not offer soft
dependencies. Either they are must be fulfilled or they are not needed.
I'm usually leaning towards including them as hard dependencies for the users
sake. Not everyone reads manuals. :)

Some notes about the spec:

Wrapping lines at 78chars makes it much more readable. For Requires or
BuildRequires lines, the tag can be repeated several times.

For the sake of readability: It might be worth considering different .spec
files for fedora based and opensuse based distros, the %if/%endif conditionals
make reading the spec a bit harder.
If you want to keep the single .spec file, at least consider using only one %if
%ifelse %endif block and execute all needed actions inside of it instead of
repeating the %if %endif construct for a single command.


The mkdir -p/cp combo used in the %install phase can be abbreviated as "install
-D -m perms source dest". This way the directories leading up to it are created
automatically and usually the install part can be written much more condensed.


The bigger blocker is IMHO the %post script.
Most of the commands it executes are unnecessary and can be implemented during
the %install phase. e.g. mkdirs are better done during the %install phase and
packaged in the %file section. That way these files are included in the rpmdb
and are linked to a package.
chmod commands are equally best done during the %install phase. %attr macros
can be used in the %file part of the spec to assign special permissions to
single files.

Calling mozroots to change a local certificate store is a bad idea IMHO. This
way possibly existing local policies are circumvented in a install script.

Starting mysqld is a bad idea as well. It might be that the real mysqld is on a
different machine and the local mysqld is therefore not needed. The dependency
on it is acceptable IMHO, but starting it specifically is not. Furthermore,
what if the mysqld init script is disabled? Just starting it once will not
survice a reboot.

Updating the database schema might be acceptable but the general suggestion is
to not execute any SQL commands during a %post script but leave it to the
system admin in question. They know their system much better then any packager
can ever do.

David's complaint about the %post script echoing messages is perfectly valid.
RPM installations but especially upgrades might be performed at night and fully
automated. Nobody is going to see these messages. The concept of rpm is
unattended installation and this design principle should not be violated.

Can the different %{webroot}/dekiwiki/bin %{webroot}/dekiwiki/editor etc. lines
in the %file-section be condensed into one single %{webroot}/dekiwiki/ line?

I'll go about building and installing the software later today for detailed
review. So much for my first notes about the .spec file.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.




More information about the Fedora-package-review mailing list