[Bug 184080] Review Request: webmin

[bugzilla at redhat.com]

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

Summary: Review Request: webmin


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





------- Additional Comments From kevin at tummy.com  2006-04-08 18:26 EST -------
Here's A prelim review:

MUST items:

See below - rpmlint output
OK - Package name.
OK - Spec file name matches.
See below - Package guidelines.
See below - License.
See below - License field matches in spec.
See below - License included in files.
OK - Spec in american english.
OK - md5sum of source from upstream
c45fe387902405cb36a1a5c6a240ad0d  webmin-1.260.tar.gz
c45fe387902405cb36a1a5c6a240ad0d  webmin-1.260.tar.gz.1
OK - Compiles and builds on one arch at least.
OK - No forbidden buildrequires included
See below - Owns all directories it creates.
OK - No duplicate files in %files listing.
See below - Permissions on files correct.
See below - Clean section correct.
OK - Macros consistant.
OK - Code not content.
OK - Doesn't own any files/dirs that are already owned by other packages.

Items needing attention:

Given the number of issues here, (and I didn't even finish looking through
everything), perhaps it would be better to start off with a clean spec
generated from 'fedora-newrpmspec -t perl -o webmin.spec' and build from
there? Let me know how you want to move forward.

1. Don't use 'fe5' in release, instead use %{?dist} tag.
http://fedoraproject.org/wiki/DistTag

2. Don't include a 'Vendor' tag. It gets set by the build system.

3. You shouldn't need the [ "%{buildroot}" != "/" ] in the %clean section.

4. You shouldn't "echo "Running uninstall scripts .."" in the un-install.
rpm shouldn't output anything to stdout. There are several other places
with echos that should be removed.

5. You copy the /etc/webmin dir to a backup, is that likely to be large?
rpm can't account for that space when it says ok to the install, so if the
disk is close to full, wouldn't that cause a nasty failure?

6. version 1.270 is now out.

7. so the LICENSE file is BSD, but it looks like several of the modules
have GPL headers on them. Doesn't that mean the entire package has
to be distributed under the GPL?

8. There is quite a lot of rpmlint output. (around 250 lines) available
on request.

9. You create/remove the /var/webmin dir in post/postun/triggers. This is
not acceptable. You must include it in files. Does this have log files
and other state? You shouldn't delete it on package removal.

10. The run-setup.sh script needs a LOT of work. It runs perl commands on stdin?
It checks for about 100 diffrent types of OSes, which this package will know
is fedora. It ends up just running /usr/libexec/webmin/setup.sh with some env
set. Perhaps you could re-write it as a clean/simple perl/shell script?
11. Why are you doing "%define __spec_install_post %{nil}" ?

12. Why 'mkdir -p %{buildroot}/etc/rc.d/{rc0.d,rc1.d,rc2.d,rc3.d,rc5.d,rc6.d}' ?
You don't install anything there, and chkconfig should handle that on install.

13. /usr/libexec/webmin/run-uninstalls.pl is totally unacceptable.
It asks if the user wants to un-install webmin and then runs 'rpm -e --nodeps
webmin'This would be executed from a rpm %preun and totally fail in a lot of ways.


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.