[Bug 552331] Review Request: piranha - Tools for administration of Linux Virtual Server

[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.


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





--- Comment #2 from Milos Jakubicek <xjakub at fi.muni.cz>  2010-01-04 14:15:15 EDT ---
So:

* first, please make rpmlint happy:

>rpmlint piranha.spec                                                                             
piranha.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 80, tab: line 85)  

>rpmlint ../RPMS/x86_64/piranha-*.rpm
piranha-debuginfo.x86_64: W: no-url-tag
piranha.x86_64: W: no-url-tag

(add URL tag)

piranha.x86_64: W: dangling-symlink /etc/sysconfig/ha/modules
/usr/lib64/httpd/modules

This seems to be ok, the target is provided by httpd which is required, but see
my comment at the very end.

piranha.x86_64: E: non-standard-dir-perm /var/log/piranha 0775

Fix this please.

piranha.x86_64: W: dangling-symlink /usr/sbin/piranha_gui /usr/sbin/httpd

This is ok, as previously, though the "_gui" suffix isn't really motivating for
being linked to apache.

piranha.x86_64: E: zero-length /var/log/piranha/piranha-gui
piranha.x86_64: E: zero-length /etc/sysconfig/ha/lvs.cf

Zero-length files shouldn't be packaged, either provide some default
configuration (or at least explanation what users should use them for!) in them
or remove them. 

piranha.x86_64: E: incoherent-logrotate-file /etc/logrotate.d/piranha-httpd

Your logrotate file should be named /etc/logrotate.d/<package name>.

piranha.x86_64: E: zero-length /var/log/piranha/piranha-gui-access

Likewise.

piranha.x86_64: E: non-readable /usr/sbin/piranha-passwd 0700
piranha.x86_64: E: non-standard-executable-perm /usr/sbin/piranha-passwd 0700

The file doesn't seem to contain passwords => should be 755.

piranha.x86_64: W: dangerous-command-in-%postun userdel

Users/groups must not be removed after package removal, refer to:
http://fedoraproject.org/wiki/Packaging:UsersAndGroups

btw, you can also use getent to check whether a given user/group exists (see
the examples on the above referenced page).

Don't forget to remove shadow-utils from Requires(postun) as well.

piranha.x86_64: W: no-reload-entry /etc/rc.d/init.d/piranha-gui
In your init script (/etc/rc.d/init.d/your_file), you don't have a 'reload'
entry, which is necessary for good functionality.

See https://fedoraproject.org/wiki/Packaging/SysVInitScript

* please refer to Source and Patch policy here:

http://fedoraproject.org/wiki/Packaging:SourceURL
https://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment

* use macros in the %files section
(https://fedoraproject.org/w/index.php?title=Packaging:RPMMacros) --
%{_sysconfdir}, %{_sbindir}, %{_initddir}, %{_initddir}, %{_localstatedir}

* there is quite a mess what concerns licensing -- some source files (pulse.c,
linkstate.c, nanny.c) refer to GPLv2+ (not just GPLv2), most refer to
unspecified version of GPL (fos.c, lvsconfig.c, main.c, lvsd.c, util.c). Two
files have quite problematic licensing: ipvs_exec.c contains just Copyright
1999 Red Hat Inc., and send_arp.c contains funny licensing which however could
be troublesome (there were some cases that packages didn't pass review because
you couldn't use them to prepare nuclear war or something like that).

To sum up => Everywhere should be a licensed fully specified, I guess you
wanted GPLv2+. Can you satisfy this condition for send_arp.c? If not, I'd ask
spot what to do with this.

* directory layout: the package misuses /etc/sysconfig heavily. The apache
configuration files should placed under /etc/httpd/conf.d, "web" and "logs"
should be moved to /usr/share. What's the purpose of the "modules" symlink?
=> There shouldn't be anything besides the "pulse" in /etc/sysconfig.

-- 
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.