[Bug 555059] Review Request: clamsmtp - 1.10-1

[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=555059

--- Comment #7 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp> 2010-02-26 15:03:10 EST ---
Some notes for morpheusv's 1.10-1
! Note
  From next time please change the release number of your spec file
  every time you modify your spec file (if version number does not change).

* Stripping binaries
  - rpmlint complains:
-------------------------------------------------------------
clamsmtp-debuginfo.i686: E: empty-debuginfo-package
-------------------------------------------------------------
    You must not strip binaries by yourself and let find-debuginfo.sh
    create correct debuginfo rpm:
   
https://fedoraproject.org/wiki/Packaging:Debuginfo#Checking_your_debuginfo_package_for_usefulness

* Timestamp
  - When using "install" or "cp" commands, add "-p" option to
    keep timestamps on installed files:
    https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps

  - Also please consider to use
--------------------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
--------------------------------------------------------------
    to keep timestamps on installed files as much as possible.
    This method usually works for Makefiles generated by recent
    autotools.

* Consistent macros usage
  - If you want to use "install" command with macro (i.e. if you want
    to use %{__install}), change other commands also to macro,
    like %{__make} , %{__rm} , %{__mkdir_p} , etc)

  - Please choose to use one from $RPM_BUILD_ROOT and %{buildroot},
    and don't use both:
   
https://fedoraproject.org/wiki/Packaging/Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS

  - Use %{_localstatedir} or %{_var} instead of using /var .
    https://fedoraproject.org/wiki/Packaging/RPMMacros

  - Use %{_initddir} instead of %{_initrddir}:
   
https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_on_the_filesystem

* Requires
  - For written %postun script, proper Requires(post) is needed, ref:
   
https://fedoraproject.org/wiki/Packaging/SysVInitScript#InitscriptScriptlets

* Documents
  - Please also add "AUTHORS COPYING ChangeLog" to %doc

* Some issues with installed rcsysv scripts
  By the way:
  - Installed rcsysv scripts have names "clamsmtpd" and "clamd-clamsmtp",
    however your spec file reads
---------------------------------------------------------------
    84  /sbin/service clamd.clamsmtp stop &>/dev/null || :
    90  /sbin/service %{name} condrestart &>/dev/null || :
---------------------------------------------------------------
    ( note: %name here is expanded as clamsmtp, not clamsmtpd )

  - Installed %{_initddir}/clamsmtpd does not support "condrestart"
    actually.

  - Installed /etc/sysconfig/clamsmtpd don't seem to be source'd
    by any installed rcsysv scripts (i.e. clamsmtpd rcsysv script does not
    seem to source /etc/sysconfig/clamsmtpd).

  - %{_initddir}/clamd-clamsmtp seem to require
/usr/share/clamav/clamd-wrapper,
    which is in clamav-server but this package does not require clamav-server.

  - My system does not have "clamav" group. clamav-filesystem seem to
    create "clamupdate" user/group instead.

  - As far as I read /usr/share/clamav/clamd-wrapper, clamd-clamsmtp tries
    to create pid file under /var/run/clamav.clamsmtp/, however this package
    does not own (i.e. does not create) this directory.

  - Your logrotate file /etc/logrotate.d/clamsmtp contains:
-----------------------------------------------------------------
     7          killall -HUP clamd.clamsmtp 2>/dev/null || :
-----------------------------------------------------------------
    However actually this does nothing.
    ! Note:
      /usr/share/clamav/clamd-wrapper reads:
-----------------------------------------------------------------
    42  start () {
    43          echo -n $"Starting $prog: "
    44          daemon --pidfile=${CLAMD_PIDFILE} \
    45              exec -a $procname /usr/sbin/clamd \
    46              ${CLAMD_CONFIGFILE:+-c $CLAMD_CONFIGFILE} ${CLAMD_OPTIONS}
--pid ${CLAMD_PIDFILE}
-----------------------------------------------------------------
      i.e.  when "$ service clamav-clamsmtp start" is typed, actually
            binary /usr/sbin/clamd is executed with argv[0] changed to
            clamad.clamsmtp. With this "killall clamd.clamsmtp" won't
            work as the executed binary is actually "clamd" 

            (i.e. /proc/<pid number>/exe points to /usr/sbin/clamd. Also
                  Please check "man bash" and see "-a" option of "exec"
                  builtin command )
      example:
------------------------------------------------------------------
[tasaka1 at localhost ~]$ ps ww 28450
  PID TTY      STAT   TIME COMMAND
28450 pts/17   S      0:01 foo
[tasaka1 at localhost ~]$ ls -al /proc/28450/exe
lrwxrwxrwx. 1 tasaka1 tasaka1 0 Feb 27 04:58 /proc/28450/exe ->
/usr/libexec/xscreensaver/glhanoi
[tasaka1 at localhost ~]$ killall foo
foo: no process killed
[tasaka1 at localhost ~]$ pkill -HUP -f foo
[1]+  Hangup                  sh -c "exec -a foo
/usr/libexec/xscreensaver/glhanoi"
------------------------------------------------------------------

  Would you check if installed rcsysv scripts really work as you
  expect?

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