Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: clamsmtp - SMTP filter daemon for anti-virus checking using ClamAV https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=218022 ------- Additional Comments From wolfy@xxxxxxxxxxxxxxxxxx 2006-12-01 11:11 EST ------- Not an official review since I am just contributor. First sight observations: - the package is named clamsmtp but almost everywhere in config and script files it behaves as being named clamd.smtp. However it looks that this comes from upstream. - BuildRoot has a sane value, but not the one recommended at http://fedoraproject.org/wiki/Packaging/Guidelines#head-f196e7b2477c2f5dd97ef64e8eacddfb517f1aa1 - rpmlint on the src.rpm gives two warnings 1)W: clamsmtp strange-permission clamsmtpd.init 0744 A file that you listed to include in your package has strange permissions. Usually, a file should have 0644 permissions. Since the file is a startup script, this should be ignored 2)W: clamsmtp setup-not-quiet You should use -q to have a quiet extraction of the source tarball, as this generate useless lines of log ( for buildbot, for example setup -q should be used if possible - rpmlint on the binary gives a lot of errors about usage of non-standard UID/GID. Since the daemon runs under its own freshly created user, all these can be ignored. W: clamsmtp conffile-without-noreplace-flag /etc/clamd.d/smtp.conf This might be edited by the user, so I think that marking it as noreplace could be useful. E: clamsmtp non-standard-uid /var/lib/clamsmtp/tmp clamsmtp E: clamsmtp non-standard-gid /var/lib/clamsmtp/tmp clamsmtp E: clamsmtp non-standard-dir-perm /var/lib/clamsmtp/tmp 0750 This directory should not exist. At least not for pid, socket etc which should not be placed here. W: clamsmtp dangling-relative-symlink /usr/sbin/clamd.smtp clamd Symlink to clamd, provided by the (required) clamav-server package E: clamsmtp non-standard-uid /etc/clamsmtpd.conf clamsmtp E: clamsmtp non-standard-gid /etc/clamsmtpd.conf clamsmtp E: clamsmtp non-readable /etc/clamsmtpd.conf 0640 E: clamsmtp non-standard-uid /var/lib/clamsmtp clamsmtp E: clamsmtp non-standard-gid /var/lib/clamsmtp clamsmtp E: clamsmtp non-standard-dir-perm /var/lib/clamsmtp 0750 All these can be ignored due to daemon running as a new user which owns the above E: clamsmtp incoherent-logrotate-file /etc/logrotate.d/clamd.smtp logrotate file should be named clamsmtp E: clamsmtp init-script-name-with-dot /etc/rc.d/init.d/clamd.smtp recommended name is clamsmtp E: clamsmtp no-status-entry /etc/rc.d/init.d/clamd.smtp W: clamsmtp no-reload-entry /etc/rc.d/init.d/clamd.smtp E: clamsmtp subsys-not-used /etc/rc.d/init.d/clamd.smtp The init.d/clamd.smtp script just starts clamd.wrapper from the clamav package. If possible, adding a couple of lines to verify the status would be nice, but I guess that it can be ignored. How ever,I would say that a stop entry should exist W: clamsmtp no-reload-entry /etc/rc.d/init.d/clamsmtpd init.d/clamsmtpd claims it can do start, stop, restart, status, condrestart. However only start, stop and restart are implemented W: clamsmtp incoherent-subsys /etc/rc.d/init.d/clamsmtpd $prog the package is named clamsmtp so the script should be called clamsmtp too, not clamsmtpd - the %install stage makes use of the non-recommended %makeinstall macro (http://fedoraproject.org/wiki/Packaging/Guidelines#head-fcaf3e6fcbd51194a5d0dbcfbdd2fcb7791dd002) - configuration files are created as here documents at install time. I have not seen any comments about this in the wiki, but I think that using standard files included as %source would be saner and less error prone - a bunch of files are created in non-standard places (/var/lib/clamsmtp/clamd.smtp.log instead of /var/log/clamsmtp; /var/lib/clamsmtp/clamd.smtp.pid instead of /var/run/clamsmtp; I would also recommend something in the line of /var/run/clamav/clamd.smtp rather then /var/lib/clamsmtp/clamd.smtp.sock) Now, the review list: -OK: named according to packaging guidelines -OK: program licensed as GPL, original tar.gz includes the license, license filed in spec matches the actual license -OK: spec is written in AE but includes configurations files and startup scripts as here documents which make it hard to follow -OK: source matchs upstream (04da6aab94934641fcf9e7a7598346fb clamsmtp-1.8.tar.gz for both files) -OK: builds succesfully in mock/i386 -OK: no build dependency -OK: no locale -OK: no shared libraries, so no need for calling ldconfig -OK: not relocatable -OK: the package owns the files/directories it creates but does not respect FHS (see above my comments about this) -OK: does not include duplicates -OK: permissions are very sane -OK: includes a %clean section -OK: consistent usage of macros -OK: all content is permissable -OK: no large documents, so no need for separate -doc package -OK: %doc really includes just docs, runtime functionality not affected by them -OK: No header/static/.la/.pc files -OK: Not a GUI so no need for .desktop -OK: owns only its files -OK: scriptlets are sane; the daemons are added bit not started by default, conditionally restarted at upgrade, -SHOULD: works as advertised (at least does not segfault..) Bottom line: - I think all those here documents should be placed in independent SOURCE files - the startup scripts NEEDWORK (either implementing the missing advertised functions or removing those functions from the help line) - %makeinstall should be avoided - maybe something can be done about the inconsistent name usage ? clamsmtp (package name) is almost not used at all... -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review