[Bug 712921] Review Request: zabbix18 - Open-source monitoring solution for your IT infrastructure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]