Bug 184080

Summary: Review Request: webmin
Product: [Fedora] Fedora Reporter: Jason Vas Dias <jvdias>
Component: Package ReviewAssignee: Thorsten Leemhuis (ignored mailbox) <bugzilla-sink>
Status: CLOSED WONTFIX QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: kevin, rob.townley, sberry
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-07-03 12:23:56 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 201449    

Description Jason Vas Dias 2006-03-05 18:39:01 EST
Spec Name or Url: http://people.redhat.com/~jvdias/webmin/webmin.spec
SRPM Name or Url: http://people.redhat.com/~jvdias/webmin/webmin-

Description:

Upstream URL: http://webmin.com

webmin is a very popular web-based system configuration and administration
package, which can configure these sub-systems with a pure-perl HTTP GUI
interface:

acl             dfsadmin        ipsec           passwd          smf
adsl-client     dhcpd           jabber          postfix         software
apache          dnsadmin        krb5            postgresql      spam
at              dovecot         lang            ppp-client      squid
backup-config   exports         ldap-useradmin  pptp-client     sshd
bandwidth       fdisk           lilo            pptp-server     status
bind            fetchmail       logrotate       proc            stunnel
burner          file            lpadmin         procmail        syslog
caldera         firewall        lvm             proftpd         telnet
cfengine        frox            mailboxes       pserver         time
change-user     fsdump          majordomo       qmailadmin      tunnel
cluster-copy    grub            man             quota           updown
cluster-cron    heartbeat       mon             raid            useradmin
cluster-passwd  htaccess-htpass mount           samba           usermin
cluster-shell   idmapd          mscstyle3       sarg            vgetty
cluster-softwar images          mysql           sendmail        webalizer
cluster-useradm inetd           net             sentry          webmin
cluster-usermin init            nis             servers         Webmin
cluster-webmin  inittab         openslp         shell           webminlog
cpan            ipfilter        pam             shorewall       wuftpd
cron            ipfw            pap             smart-status    xinetd
custom

I've obtained permission from the upstream maintainer, Jamie Cameron,
to import webmin into Fedora Extras.

I've submitted the webmin SRPM to CVS, but will not build it until the
Review process is completed .
Comment 1 Dennis Gilmore 2006-03-05 18:52:38 EST
your not supposed to import into cvs until approved ever. 
 
http://fedoraproject.org/wiki/Extras/NewPackageProcess 
 
step 7  get approval 
step 8 import into cvs 
 
 
 
Comment 2 Jason Vas Dias 2006-03-06 09:50:52 EST
RE: Comment #1: sorry for the misunderstanding - I checked in the version for
    review so that reviewers could check it out.
    
Both the .spec file (modified from upstream default at 
http://prdownloads.sourceforge.net/webadmin/webmin-1.260-1.src.rpm 
) and the proposed srpm for FE submission are at:
  http://people.redhat.com/~jvdias/webmin/
Comment 4 Kevin Fenzi 2006-04-08 18:26:39 EDT
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.
Comment 5 Kevin Fenzi 2006-06-29 16:24:41 EDT
Since there are so many issues here and no activity for the last 2 months, I am 
going to close this bug in a few days unless there is any objection. 
Jason: Feel free to resubmit with a new spec if there is still interest. 
Comment 6 Kevin Fenzi 2006-07-03 12:23:56 EDT
I'm closing this review. 
Please re-submit with a new spec if there is still interest. 
Comment 7 Scott Berry 2007-06-07 20:03:55 EDT
(In reply to comment #0)
Jason,

I am interested in either helping with Webmin or taking over the project.  
Wou7ld you be interested in sharing your spec file with me?

> Spec Name or Url: http://people.redhat.com/~jvdias/webmin/webmin.spec
> SRPM Name or Url: http://people.redhat.com/~jvdias/webmin/webmin-
> Description:
> Upstream URL: http://webmin.com
> webmin is a very popular web-based system configuration and administration
> package, which can configure these sub-systems with a pure-perl HTTP GUI
> interface:
> acl             dfsadmin        ipsec           passwd          smf
> adsl-client     dhcpd           jabber          postfix         software
> apache          dnsadmin        krb5            postgresql      spam
> at              dovecot         lang            ppp-client      squid
> backup-config   exports         ldap-useradmin  pptp-client     sshd
> bandwidth       fdisk           lilo            pptp-server     status
> bind            fetchmail       logrotate       proc            stunnel
> burner          file            lpadmin         procmail        syslog
> caldera         firewall        lvm             proftpd         telnet
> cfengine        frox            mailboxes       pserver         time
> change-user     fsdump          majordomo       qmailadmin      tunnel
> cluster-copy    grub            man             quota           updown
> cluster-cron    heartbeat       mon             raid            useradmin
> cluster-passwd  htaccess-htpass mount           samba           usermin
> cluster-shell   idmapd          mscstyle3       sarg            vgetty
> cluster-softwar images          mysql           sendmail        webalizer
> cluster-useradm inetd           net             sentry          webmin
> cluster-usermin init            nis             servers         Webmin
> cluster-webmin  inittab         openslp         shell           webminlog
> cpan            ipfilter        pam             shorewall       wuftpd
> cron            ipfw            pap             smart-status    xinetd
> custom
> I've obtained permission from the upstream maintainer, Jamie Cameron,
> to import webmin into Fedora Extras.
> I've submitted the webmin SRPM to CVS, but will not build it until the
> Review process is completed .

(In reply to comment #0)
> Spec Name or Url: http://people.redhat.com/~jvdias/webmin/webmin.spec
> SRPM Name or Url: http://people.redhat.com/~jvdias/webmin/webmin-
> Description:
> Upstream URL: http://webmin.com
> webmin is a very popular web-based system configuration and administration
> package, which can configure these sub-systems with a pure-perl HTTP GUI
> interface:
> acl             dfsadmin        ipsec           passwd          smf
> adsl-client     dhcpd           jabber          postfix         software
> apache          dnsadmin        krb5            postgresql      spam
> at              dovecot         lang            ppp-client      squid
> backup-config   exports         ldap-useradmin  pptp-client     sshd
> bandwidth       fdisk           lilo            pptp-server     status
> bind            fetchmail       logrotate       proc            stunnel
> burner          file            lpadmin         procmail        syslog
> caldera         firewall        lvm             proftpd         telnet
> cfengine        frox            mailboxes       pserver         time
> change-user     fsdump          majordomo       qmailadmin      tunnel
> cluster-copy    grub            man             quota           updown
> cluster-cron    heartbeat       mon             raid            useradmin
> cluster-passwd  htaccess-htpass mount           samba           usermin
> cluster-shell   idmapd          mscstyle3       sarg            vgetty
> cluster-softwar images          mysql           sendmail        webalizer
> cluster-useradm inetd           net             sentry          webmin
> cluster-usermin init            nis             servers         Webmin
> cluster-webmin  inittab         openslp         shell           webminlog
> cpan            ipfilter        pam             shorewall       wuftpd
> cron            ipfw            pap             smart-status    xinetd
> custom
> I've obtained permission from the upstream maintainer, Jamie Cameron,
> to import webmin into Fedora Extras.
> I've submitted the webmin SRPM to CVS, but will not build it until the
> Review process is completed .

Comment 8 Kevin Fenzi 2007-06-07 20:54:14 EDT
Hey Scott. 

I don't think Jason works for redhat anymore, so email from this review might
not be getting to him. 

I am pretty sure the spec that he used was the one from the upstream project. 

Unfortunately there are a number of issues with this spec file, it would take a
lot of work to clean it up to the point that it would be acceptable to the
Fedora guidelines. 

I would suggest that you perhaps start submitting a easier package and get to
where you understand the guidelines and how spec files are written. 

This link, http://fedoraproject.org/wiki/PackageMaintainers/WishList has a
number of packages that are not yet packaged and someone is wishing for. 

If you are set on packaging webmin, I would suggest you start from a clean spec
file and build it up to match all the guidelines, which will be a lot of work,
but I think easier than trying to clean up the upstream project one. 
Comment 9 Scott Berry 2007-06-10 11:46:16 EDT
Kevin, I am working with the upstream maintainer Jamie Cammeron on the renewing 
of the spec file so it much more closely matches Fedora spec files.  As 
information is presented to me I will update this request.  Can this request be 
opened up again or am I going to need to start a new request?
Comment 10 Kevin Fenzi 2007-06-10 23:22:50 EDT
Excellent to hear Scott. 
I would suggest you open a new request, since you are a new submitter of the
package. Of course you should feel free to gain any info you can from this
submission to make yours better! :) 
good luck.