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=712921 Toshio Ernie Kuratomi <a.badger@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |a.badger@xxxxxxxxx AssignedTo|nobody@xxxxxxxxxxxxxxxxx |a.badger@xxxxxxxxx --- Comment #1 from Toshio Ernie Kuratomi <a.badger@xxxxxxxxx> 2011-06-27 14:47:24 EDT --- Notes: Many of the to-be-fixed items apply to both this package and the main zabbix package. With the exception of implementing this as alternatives, these seem to be relatively easy changes. You said something about looking into using alternatives with the main zabbix package and I'd be willing to approve this package with the alternatives changes to be implemented when the main zabbix package implements them if: 1) there's a timetable for that. 2) doing that doesn't seem like it would be an incompatible change that would keep it from going into EPEL. Good: * Package named properly * spec file named properly * License is approved, correctly listed in spec, and included in package * Spec is readable * Upstream source matches what's in the srpm * Builds in koji (EL5 target) * No shared libraries * Only bundled code that I could find were javascript which are currently allowed * Package not relocatable * Package owns all files and directories it creates * builds in koji for EL-5 * Consistent use of macros * Contains code, not content * Nothing in doc is needed at runtime * Not a GUI app * All filenames valid UTF-8 * License text included * scriptlets almost seem sane -- see below Mustfix: * Source0 needs to change from http://downloads.sourceforge.net/%{name}/%{srcname}-%{version}.tar.gz to: http://downloads.sourceforge.net/%{srcname}/%{srcname}-%{version}.tar.gz * Some of the Requires and Provides are still for zabbix-* instead of %{name}-* * locale files like this are not marked with %lang(): /usr/share/zabbix/include/locales/it_it.inc.php It looks like zabbix works if some translations are left out. (locales.php loads the proper locale file for a request and it seems to detect when a locale file is not present and continue to operate. If that's correct, they should be marked with the proper language so that they can be excluded by system administrators that don't want to install them. * FPC says that currently it looks like the Conflicts between the various backends should use alternatives instead of Conflicts. I don't see anything to prevent this from working. * I see that we're protecting the %{_sysconfdir}/zabbix/web directory. Should %{_sysconfdir}/zabbix/web/zabbix.conf.php also have 0640 permissions? Are these made unreadable because they contain database passwords? * The scriptlet that's creating the users and groups should create user and group separately for the reason documented on the official packaging guideline page: http://fedoraproject.org/wiki/Packaging/UsersAndGroups * Each subpackage should Conflict with the equivalent zabbix-* package ie: zabbix18-web should have Conflict: %{srcname}-web The main zabbix18 Conflicts on zabbix does this implicitly but explicit is better here to make things clear. Also, it leaves open the possibility of creating subpackages that do not Conflict: see below Questions: * The various zabbix18-web packages seem to just be there to keep us from having to download php-mysql or php-pgsql. If so, it seems that the web-sqlite3 doesn't require anything extra. It feels like the sqlite3 backend can be folded into the main packages so that we always have the sqlite backend available and just the others are loaded on demand. * Conflicts: It would be great if the zabbix18-agent and zabbix-agent packages didn't conflict as that would allow sites that want to upgrade to run two separate server and have clients talk to both during a transition period. This is in no way a blocker, just a nice to have feature. rpmlint: rpmlint has a long list of items: * Fix spelling zabbix18.i386: W: spelling-error %description -l en_US visualisation -> visualization, visualizer, visualize zabbix18.i386: W: spelling-error %description -l en_US organisations -> organizations, organizational, organization * Submit upstream: zabbix18.i386: E: incorrect-fsf-address /usr/share/doc/zabbix18-1.8.5/COPYING zabbix18-debuginfo.i386: E: incorrect-fsf-address /usr/src/debug/zabbix-1.8.5/src/libs/zbxmedia/eztexting.c (and other source files) * Fix by just chmod 0644 the tarball http://fedoraproject.org/wiki/Packaging/UsersAndGroups * MUSTFIX noted above: zabbix18.src: W: invalid-url Source0: http://downloads.sourceforge.net/zabbix18/zabbix-1.8.5.tar.gz HTTP Error 404: Not Found * May be okay -- see the above question about permissions on config files zabbix18-proxy.i386: E: non-readable /etc/zabbix/zabbix_proxy.conf 0640L * Not sure which is safer. We currently have: # Apply permissions also in *.rpmnew upgrades from old permissive ones chmod 0640 %{_sysconfdir}/%{srcname}/zabbix_server.conf Would it be safer to simply remove permissions for other?: chmod o-rwx %{_sysconfdir}/%{srcname}/zabbix_server.conf I think that the sysadmin may have locked this down to 0600 manually and things would still work. So this may be better but I'm not sure. zabbix18-proxy.i386: W: dangerous-command-in-%post chmod zabbix18-server.i386: W: dangerous-command-in-%post chmod * Not sure why this exists and it's not referened in any file. Something to ask upstream to clean up, perhaps (not a blocker) zabbix18-web.i386: E: zero-length /usr/share/zabbix/styles/ie_css_bb.css zabbix18-web.i386: E: zero-length /usr/share/zabbix/styles/ie_css_ob.css * Should be able to jusst delete these: zabbix18-web.i386: E: backup-file-in-package /usr/share/zabbix/include/config.inc.php.orig * This is a good point. Doing these centrally in /etc is a better overall architecture zabbix18-web.i386: E: htaccess-file /usr/share/zabbix/conf/.htaccess zabbix18-web.i386: E: htaccess-file /usr/share/zabbix/api/.htaccess * May be ignored - zabbix is a daemon and it makes sense to run it under its own uid/gid: zabbix18.i386: W: non-standard-uid /var/run/zabbix zabbix zabbix18.i386: W: non-standard-gid /var/run/zabbix zabbix - logrotate configs are in the subpackages that actually create logs zabbix18.i386: W: log-files-without-logrotate /var/log/zabbix - Does no harm and are probably more legible to the intended audience as they are zabbix18.src:435: W: macro-in-comment %{srcname} zabbix18.src:721: W: macro-in-%changelog %40zabbix - Files are remaining with the zabbix names -- we're using Conflicts instead zabbix18-agent.i386: E: incoherent-logrotate-file /etc/logrotate.d/zabbix-agent zabbix18-agent.i386: W: incoherent-init-script-name zabbix-agent ('zabbix18-agent', 'zabbix18-agentd') zabbix18-proxy.i386: W: incoherent-init-script-name zabbix-proxy ('zabbix18-proxy', 'zabbix18-proxyd') zabbix18-server.i386: E: incoherent-subsys /etc/init.d/zabbix-server zabbix zabbix18-server.i386: W: incoherent-init-script-name zabbix-server ('zabbix18-server', 'zabbix18-serverd') - Man pages are nice to have but not requirements: zabbix18-agent.i386: W: no-manual-page-for-binary zabbix_snmptrap zabbix18-agent.i386: W: no-manual-page-for-binary zabbix_agent - Some spelling errors are false positives - This is a versioned obsolete and the new packages take care of this. However, the old version actually should not exist (since this is the first version of the compat package) so you could get rid of it but it's doing no harm. zabbix18-server-mysql.i386: W: obsolete-not-provided zabbix18 zabbix18-web-mysql.i386: W: obsolete-not-provided zabbix18-web - This one looks sane zabbix18-web.i386: W: dangerous-command-in-%post mv -- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review