Bug 184080
Summary: | Review Request: webmin | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Jason Vas Dias <jvdias> |
Component: | Package Review | Assignee: | 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: | rawhide | CC: | 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 16:23:56 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 201449 |
Description
Jason Vas Dias
2006-03-05 23:39:01 UTC
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 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/ 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. 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. I'm closing this review. Please re-submit with a new spec if there is still interest. (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 . 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. 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? 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. |