Bug 555059 - Review Request: clamsmtp - 1.10-1
Review Request: clamsmtp - 1.10-1
Status: CLOSED DUPLICATE of bug 557011
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
: 218022 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-01-13 09:57 EST by Andres Pascasio
Modified: 2013-10-19 10:42 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-06-22 17:40:21 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Andres Pascasio 2010-01-13 09:57:35 EST
Spec URL: http://morpheusv.fedorapeople.org/clamsmtp/clamsmtp.spec
SRPM URL: http://morpheusv.fedorapeople.org/clamsmtp/clamsmtp-1.10-1.fc12.src.rpm
Description: ClamSMTP is an SMTP filter that allows you to check for viruses using the ClamAV anti-virus software.
ClamSMTP aims to be lightweight, reliable, and simple rather than have a myriad of options. It's written in C without major dependencies.

This is my first package, and I am looking  a sponsor.
Comment 1 Mamoru TASAKA 2010-01-13 12:48:04 EST
*** Bug 218022 has been marked as a duplicate of this bug. ***
Comment 2 Mamoru TASAKA 2010-01-20 11:36:19 EST
*** Bug 557011 has been marked as a duplicate of this bug. ***
Comment 3 Nathanael Noblet 2010-01-20 12:31:52 EST
Hello morpheusv... I just completed creating a spec for the same thing but posted it a bit after you... You can find my spec at 

http://www.gnat.ca/clamsmtp.spec
http://www.gnat.ca/clamsmtp-1.10-0.1.fc12.src.rpm

It has a number of items that you should incorporate. Perhaps take a look and merge the two into one spec. It includes clamd integration and setup files so they all work together. It also follows the guidelines a bit more as this is my second spec file after having gone through this process for the first time over the last few months. You can also check out the issues I've raised in my bug (557011) above to see the remaining issues.
Comment 4 Andres Pascasio 2010-01-20 21:52:43 EST
Hi Nathanael Noblet
Thanks for sharing your spec file.
And of course   I take  a look and will work to incorporate the elements to merge the two spec.
All help is welcome.
Comment 5 Nathanael Noblet 2010-01-26 11:13:33 EST
Have you incorporated any changes yet? Is there a new spec file to view?
Comment 6 Andres Pascasio 2010-01-27 01:17:19 EST
(In reply to comment #5)
> Have you incorporated any changes yet? Is there a new spec file to view?    

You can find the new spec at 

http://morpheusv.fedorapeople.org/clamsmtp/1.10-1/fc12/clamsmtp.spec
http://morpheusv.fedorapeople.org/clamsmtp/1.10-1/fc12/clamsmtp-1.10-1.fc12.src.rpm

the rpmlint output 

clamsmtp.i686: W: non-standard-uid /var/run/clamsmtp clamsmtp
clamsmtp.i686: W: non-standard-uid /var/log/clamsmtp clamsmtp
clamsmtp.i686: W: non-standard-uid /var/lib/clamsmtp clamsmtp
clamsmtp.i686: E: no-status-entry /etc/rc.d/init.d/clamd-clamsmtp
clamsmtp.i686: W: no-reload-entry /etc/rc.d/init.d/clamd-clamsmtp
clamsmtp.i686: E: subsys-not-used /etc/rc.d/init.d/clamd-clamsmtp
clamsmtp.i686: W: no-reload-entry /etc/rc.d/init.d/clamsmtpd
1 packages and 0 specfiles checked; 2 errors, 5 warnings.

and justification according to https://bugzilla.redhat.com/show_bug.cgi?id=557011

This is so that the clamd instance and clamsmtp are isolated from someone on
the system injecting/overriding the file being scanned.

These are all pretty much bogus, the clamd initscript is a wrapper around
/usr/share/clamav/clamd-wrapper which has all of the parts this is warning
about.
Comment 7 Mamoru TASAKA 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@localhost ~]$ ps ww 28450
  PID TTY      STAT   TIME COMMAND
28450 pts/17   S      0:01 foo
[tasaka1@localhost ~]$ ls -al /proc/28450/exe
lrwxrwxrwx. 1 tasaka1 tasaka1 0 Feb 27 04:58 /proc/28450/exe -> /usr/libexec/xscreensaver/glhanoi
[tasaka1@localhost ~]$ killall foo
foo: no process killed
[tasaka1@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?
Comment 8 Mamoru TASAKA 2010-03-12 11:27:56 EST
ping?
Comment 9 Nathanael Noblet 2010-03-12 11:47:10 EST
Hi Mamoru... if the poster doesn't respond I'd like to take over getting it in..
Comment 10 Andres Pascasio 2010-03-12 15:50:08 EST
Currently I am working on the basis of observation mentioned, Mamoru.
On the other hand is very much welcome the help of Nathanael.
Comment 11 Mamoru TASAKA 2010-03-27 11:42:57 EDT
morpheusv, ping again?
Comment 12 Andres Pascasio 2010-03-29 02:44:07 EDT
I have taken into account comments made by Mamura Tasaka and I created a new spec.
You can find the new spec at

http://morpheusv.fedorapeople.org/clamsmtp/1.10-2/fc12/clamsmtp.spec
http://morpheusv.fedorapeople.org/clamsmtp/1.10-2/fc12/clamsmtp-1.10-2.fc12.src.rpm
Comment 13 Mamoru TASAKA 2010-03-30 05:02:47 EDT
The new srpm doesn't seem to have modificatior rcsysv scripts issue
I mentioned on the comment 7.
Comment 14 Matthias Saou 2010-04-08 05:44:17 EDT
Two new reviews created while mine had been pending for 3 years, waiting for some selinux love, and no one commenting on it nor helping out :-(
Anyway, please do check http://thias.fedorapeople.org/review/clamsmtp/ and make sure all features are also supported by this package, thanks!
Comment 15 Mamoru TASAKA 2010-04-08 05:55:04 EDT
Unfortunately I am $ getenforce = Disabled user, I cannot check
selinux issue.
Comment 16 Nathanael Noblet 2010-04-08 11:24:12 EDT
I run it under selinux... works fine. My review for this package has been merged into this and closed in favor of getting this one in...
Comment 17 Mamoru TASAKA 2010-04-19 13:58:51 EDT
morpheusv, would you address sysv scripts issue in
comment 7 and upload new srpm?
Comment 18 Andres Pascasio 2010-04-19 16:46:06 EDT
I'm working on solving the comments (comment 7) rcsysv script, would expect to have the new spec ready for this week.
Comment 19 Mamoru TASAKA 2010-05-03 13:26:29 EDT
morpheusv, if you don't have enough time for working on
this package, I think it is good currently that you let
Nathanael work on this package. How do you think?
Comment 20 Nathanael Noblet 2010-05-03 13:48:30 EDT
for what its worth I'd love to take it...
Comment 21 Andres Pascasio 2010-05-04 00:48:35 EDT
Sorry for the delay, but I was kind of busy with the event FLISOL 2010, plus a family problem.

You can find the new spec at

http://morpheusv.fedorapeople.org/clamsmtp/1.10-2/fc12/clamsmtp.spec
http://morpheusv.fedorapeople.org/clamsmtp/1.10-2/fc12/clamsmtp-1.10-2.fc12.src.rpm
Comment 22 Mamoru TASAKA 2010-05-05 11:42:27 EDT
Again please change the release number every time you modify
your spec file to avoid confusion.

For -2:

* BR:
  - Would you explain why "BR: autoconf automake" is needed?
    No autotools seem to be called during build process.

* BuildRoot
  - For Fedora BuildRoot is no longer needed.

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

* Useradd
  - Change the comment 'User to own clamsmtp directories and default processes'
    for "clamsmtp" to be more specific to clamsmtp

* Comments in scriptlets
  - Please remove #%postun comments completely. These comment now appears
    in %preun scriptlet (please check $rpm -q --scripts clamstmp) and are
    very confusing.

* rcsysv scripts issue again
  - /etc/rc.d/init.d/clamsmtpd says:
-------------------------------------------------------------------
    29  # The location for pid file
    30  piddir=/var/run/clamav
-------------------------------------------------------------------
    However:
-------------------------------------------------------------------
# rpm -qf /var/run/clamav
error: file /var/run/clamav: No such file or directory
-------------------------------------------------------------------

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

  - clamd-clamsmtp service seems badly broken. For example:
--------------------------------------------------------------------
[root@localhost ~]# service clamd-clamsmtp status
clamd.clamsmtp is stopped
[root@localhost ~]# service clamd-clamsmtp start 
Starting clamd.clamsmtp: LibClamAV Warning: **************************************************
LibClamAV Warning: ***  The virus database is older than 7 days!  ***
LibClamAV Warning: ***   Please update it as soon as possible.    ***
LibClamAV Warning: **************************************************
                                                           [  OK  ]
[root@localhost ~]# service clamd-clamsmtp status
clamd.clamsmtp dead but subsys locked
[root@localhost ~]# ps auwwx | grep [c]lamd
clamsmtp 20758  0.0 10.9  96264 82408 ?        Ssl  00:22   0:00 clamd.clamsmtp -c /etc/clamd.d/clamsmtp.conf --pid /var/run/clamd.clamsmtp/clamd.pid
--------------------------------------------------------------------
    Actually with "service clamd-clamsmtp start", clamd.clamsmtp starts,
    however "service clamd-clamsmtp status" cannot find clamd.clamsmtp
    process.

  - /etc/logrotate.d/clamsmtp issue is not yet fixed.
    ( i.e. this file contains:
---------------------------------------------------------------------
     6      postrotate
     7          killall -HUP clamd.clamsmtp 2>/dev/null || :
     8      endscript
---------------------------------------------------------------------
      however killall -HUP clamd.clamsmtp does not do anything.
      See the explanation in my previous comment )

Please check if 2 rcsysv scripts work correctly, and fix other things
raised above.
Comment 23 Mamoru TASAKA 2010-05-30 01:38:21 EDT
What is the status of this bug?
Comment 24 Andres Pascasio 2010-06-04 12:15:10 EDT
In these moments the observations done in the comment 22, even estan earrings of checking.
 Provided that in these moments my this daughter a bit badly of health, I have not dedicated him time to the package.

 I wait to take again it in the next days. Or if you consider and there exists someone who could take charge of the package to overcome the observations.
Comment 25 Mamoru TASAKA 2010-06-15 12:35:53 EDT
Nathanael, would you interested in importing this package
into Fedora?
Comment 26 Nathanael Noblet 2010-06-15 18:21:20 EDT
For sure. Should I take he latest posted spec or can we go back to the one I did and posted the day after this one?
Comment 27 Mamoru TASAKA 2010-06-15 21:08:21 EDT
You can base on the srpm you want to use (i.e. either will do). 

However anyway to make it
clear, please post the URLs of the spec file and srpm to show which
ones are to be reviewed (if you want, open a new review request and
mark this one as a duplicate, and I prefer this way).
Comment 28 Nathanael Noblet 2010-06-16 02:17:25 EDT
Sounds good, I'm out of town till next monday so will restart a review request then...
Comment 29 Nathanael Noblet 2010-06-22 17:40:21 EDT

*** This bug has been marked as a duplicate of bug 557011 ***

Note You need to log in before you can comment on or make changes to this bug.