Bug 217654 (tmda)

I would appreciate comments on the rpmlint output (my explanations in parenthesis):

E: tmda zero-length /usr/share/doc/tmda-1.1.9/contrib/dot-tmda/lists/blacklist
E: tmda zero-length /usr/share/doc/tmda-1.1.9/contrib/dot-tmda/lists/confirmed
E: tmda zero-length /usr/share/doc/tmda-1.1.9/contrib/dot-tmda/crypt_key

(these are zero byte files from upstream contrib)

W: tmda doc-file-dependency
/usr/share/doc/tmda-1.1.9/contrib/update-internaldomains /usr/bin/env
W: tmda doc-file-dependency
/usr/share/doc/tmda-1.1.9/contrib/update-internaldomains perl(Getopt::Std)
W: tmda doc-file-dependency
/usr/share/doc/tmda-1.1.9/contrib/update-internaldomains perl(strict)
W: tmda doc-file-dependency
/usr/share/doc/tmda-1.1.9/contrib/ofmipd-stunnel-xinetd/stunnel-wrapper /bin/sh
W: tmda doc-file-dependency
/usr/share/doc/tmda-1.1.9/contrib/ofmipd-stunnel-xinetd/tmda-ofmipd-wrapper /bin/sh
W: tmda doc-file-dependency /usr/share/doc/tmda-1.1.9/contrib/sendit.sh /bin/sh
W: tmda doc-file-dependency /usr/share/doc/tmda-1.1.9/contrib/vadduser-tmda /bin/sh
W: tmda doc-file-dependency /usr/share/doc/tmda-1.1.9/contrib/vmailmgr-vdir.sh
/bin/sh
W: tmda doc-file-dependency /usr/share/doc/tmda-1.1.9/contrib/vpopmail-vdir.sh
/bin/sh
W: tmda doc-file-dependency /usr/share/doc/tmda-1.1.9/contrib/wrapfd3.sh /bin/sh

(these are contrib files from upstream - no real dependency as they are %doc)

W: tmda-emacs no-documentation
W: tmda-ofmipd no-documentation
E: tmda-ofmipd use-tmp-in-%pre

(%pre creates user with /var/tmp as home directory - %pre does not use /tmp)

W: tmda-ofmipd dangerous-command-in-%postun userdel

(if we create a user, shouldn't we also remove it when the package is removed?)

W: tmda-ofmipd service-default-enabled /etc/rc.d/init.d/tofmipd

(will patch upstream file in a future rev)

W: tmda-ofmipd no-reload-entry /etc/rc.d/init.d/tofmipd

(does not support reload)

W: tmda-ofmipd incoherent-init-script-name tofmipd

(named from upstream; also daemon is a different name (TMDA Old Fashioned Mail
Injection Protocol Daemon - tofmipd) - not the TMDA program itself)

(Removing NEEDSPONSOR as bug 216723)

Well, for rpmlint issues:

* spurious-executable-perm
* doc-file-dependency
---------------------------------------------
W: tmda spurious-executable-perm /usr/share/doc/tmda-1.1.9/contrib/printdbm
W: tmda doc-file-dependency
/usr/share/doc/tmda-1.1.9/contrib/update-internaldomains /usr/bin/env
---------------------------------------------
  - You may have confused about these messages, however
---------------------------------------------
$ rpmlint -I spurious-executable-perm
spurious-executable-perm :
The file is installed *with executable permissions*, but was identified as one
that probably should not be executable.  Verify if the executable bits are
desired, and remove if not.

$ rpmlint -I doc-file-dependency
doc-file-dependency :
An included file marked as %doc creates a possible additional dependency in
the package.  Usually, this is not wanted and may be caused by eg. example
scripts *with executable bits set* included in the package's documentation.

-------------------------------------
    so please ensure that %doc files do not have executable
    permissions.

* zero-length
-------------------------------------
E: tmda zero-length /usr/share/doc/tmda-1.1.9/contrib/dot-tmda/lists/blacklist
-------------------------------------
  - How do empty documents make sense?

* User home directory
-------------------------------------
E: tmda-ofmipd use-tmp-in-%pre
-------------------------------------
> (%pre creates user with /var/tmp as home 
> directory - %pre does not use /tmp)
  Usually this should be %{_sysconfdir}/%{name} or something
  else (see xorg-x11-xfs or ntp, for example)

*  userdel, groupdel...
-------------------------------------
  /usr/sbin/userdel ofmipd >/dev/null 2>&1 || :
  /usr/sbin/groupdel ofmipd >/dev/null 2>&1 || :
-------------------------------------
  - Still under discussion, however, as far as I know
    current Fedora policy is that "some dangerous commands
    like userdel or so should not automatically done and
    should executed by sysadmin with care".

* init script recording
-------------------------------------
W: tmda-ofmipd service-default-enabled /etc/rc.d/init.d/tofmipd
-------------------------------------
   Well, if you don't have a strong reason you want to enable
   tofmipd daemon by default, please the line
-------------------------------------
# chkconfig: 2345 87 13
-------------------------------------
   in %{_initrddir}/tofmipd to
-------------------------------------
# chkconfig: - 87 13
-------------------------------------

* init daemon reload
-------------------------------------
W: tmda-ofmipd no-reload-entry /etc/rc.d/init.d/tofmipd
-------------------------------------
  - Just change
-------------------------------------
  restart)
-------------------------------------
    in %{_initrddir}/tofmipd to
-------------------------------------
  restart|reload)
-------------------------------------

(In reply to comment #3)
> W: tmda spurious-executable-perm /usr/share/doc/tmda-1.1.9/contrib/printdbm
> W: tmda doc-file-dependency
> E: tmda-ofmipd use-tmp-in-%pre
> W: tmda-ofmipd service-default-enabled /etc/rc.d/init.d/tofmipd
> W: tmda-ofmipd no-reload-entry /etc/rc.d/init.d/tofmipd

I have fixed these problems.

> -------------------------------------
> E: tmda zero-length /usr/share/doc/tmda-1.1.9/contrib/dot-tmda/lists/blacklist
> -------------------------------------
>   - How do empty documents make sense?

Actually, in this case they do.  The dot-tmda directory is made for a user to cp
-r .../dot-tmda to ~/.tmda and then they have a fully setup boilerplate to use
TMDA.  The blacklist & confirmed list are initially blank, and the crypt_key
must be customized.


> *  userdel, groupdel...
> -------------------------------------
>   /usr/sbin/userdel ofmipd >/dev/null 2>&1 || :
>   /usr/sbin/groupdel ofmipd >/dev/null 2>&1 || :
> -------------------------------------
>   - Still under discussion, however, as far as I know
>     current Fedora policy is that "some dangerous commands
>     like userdel or so should not automatically done and
>     should executed by sysadmin with care".

I switched this to fedora-usermgmt style.  I believe this takes a little of the
danger out of the process anyway.  In any case, whatever Fedora policy is chosen
I will adhere to.

Spec URL: http://www.symetrix.com/~bjohnson/projects/Fedora-Extras/tmda.spec
SRPM URL:
http://www.symetrix.com/~bjohnson/projects/Fedora-Extras/tmda-1.1.10-1.fc6.src.rpm

* Thu Feb 15 2007 Bernard Johnson <bjohnson> 1.1.10-1
- version 1.1.10
- substitute RPM_BUILD_ROOT for %%{buildroot} macro
- preserve file timestamps
- change tofmipd user to /etc home directory
- switch to fedora-usermgmt user management
- add Require(post,preun) of /sbin/chkconfig

Well, for 1.1.10-1.fc7:

* Scriptlets
  - Well, on my environ no one has /etc/ as the home directory.
    For the user tofmipd, I think the home directory should be
    %{_sysconfdir}/tmda or %{_sysconfdir}/tofmipd with the group
    of the directory set as tofmipd.

* Requires
----------------------------------------------
Requires(post): /sbin/chkconfig
Requires(preun):/sbin/chkconfig
Requires(pre):  fedora-usermgmt
Requires(postun):fedora-usermgmt
----------------------------------------------
  - All these are not needed for main package. These are needed
    by -tofmipd package.

  - By the way, why do you use the mixed use of
----------------------------------------------
  /sbin/service tofmipd stop &> /dev/null || :
  %{_initrddir}/tofmipd condrestart &> /dev/null || :
----------------------------------------------
    (i.e. use of /sbin/service v.s. directly calling
          scripts under %{_initrddir} ) ?
    On Fedora, the use of /sbin/service seems to be recommended,
    and Requires(....): /sbin/service is needed.

* Documentation
  - Check if the document "INSTALL" is needed.

* Functionality:
  On FC7 i386, "service tofmipd start" fails 100% as following:
----------------------------------------------------------------
[root@localhost bin]# service tofmipd start
Starting tofmipd: Traceback (most recent call last):
  File "/usr/bin/tmda-ofmipd", line 42, in <module>
    from TMDA import Util
  File "/usr/lib/python2.5/site-packages/TMDA/Util.py", line 27, in <module>
    import email
  File "/usr/lib/python2.5/site-packages/TMDA/pythonlib/email/__init__.py", line
118, in <module>
    import email.mime
ImportError: No module named mime
                                                           [失敗]

(The last word is Japanese, meaning "failed" in English)
----------------------------------------------------------------
  It seems that python tries to search email.mime module from 
  /usr/lib/python2.5/site-packages/TMDA/pythonlib/email/, because
  /email directory is found first. This is because
  /usr/lib/python2.5/site-packages/TMDA/pythonlib
  directory is inserted to the first place in sys.path. 

  However email.mime is on /usr/lib/python2.5/email/mime and
  python seems to search any more...

  The following diff seems to work.
----------------------------------------------------------------
--- tmda-ofmipd.orig    2007-02-16 00:09:06.000000000 +0900
+++ tmda-ofmipd 2007-02-17 15:20:39.000000000 +0900
@@ -36,7 +36,7 @@
     # Prepend /usr/lib/python2.x/site-packages/TMDA/pythonlib
     sitedir = os.path.join(sys.prefix, 'lib', 'python'+sys.version[:3],
                            'site-packages', 'TMDA', 'pythonlib')
-    sys.path.insert(0, sitedir)
+    sys.path.append(sitedir)
 
 
 from TMDA import Util
----------------------------------------------------------------

(In reply to comment #5)
> * Scriptlets
>   - Well, on my environ no one has /etc/ as the home directory.
>     For the user tofmipd, I think the home directory should be
>     %{_sysconfdir}/tmda or %{_sysconfdir}/tofmipd with the group
>     of the directory set as tofmipd.

Can we agree that the actual home directory of the daemon does not matter as
long as:
1) The directory in not on a mounted filesystem (other than /)
2) It is not a privileged directory (since the program initially runs as root)

With that in mind, I really do not want to create an empty directory to just
have a home directory for the daemon.  In my mind, it doens't really matter
outside of that.  I wanted /tmp but rpmlint complains for an entirely different
reason.

Seems to me, the next best choice is /etc or even /.  At least for now, I've
changed it to /.

On my system, the following daemons (users) use / as their home directory:
nobody, dbus, avahi, rpc, nscd, haldaemon


> ----------------------------------------------
> Requires(post): /sbin/chkconfig
> Requires(preun):/sbin/chkconfig
> Requires(pre):  fedora-usermgmt
> Requires(postun):fedora-usermgmt
> ----------------------------------------------
>   - All these are not needed for main package. These are needed
>     by -tofmipd package.

Fixed that for next release.

>   - By the way, why do you use the mixed use of
> ----------------------------------------------
>   /sbin/service tofmipd stop &> /dev/null || :
>   %{_initrddir}/tofmipd condrestart &> /dev/null || :
> ----------------------------------------------
>     (i.e. use of /sbin/service v.s. directly calling
>           scripts under %{_initrddir} ) ?
>     On Fedora, the use of /sbin/service seems to be recommended,
>     and Requires(....): /sbin/service is needed.

It was my mistake.  I had intended to not use service at all as it provides no
additional functionality and adds more requirements to the package.

Unless you have a strong argument for it, I would rather not use service.

> * Documentation
>   - Check if the document "INSTALL" is needed.

It did have a few pointers to documentation, but one of them was wrong, so I
choose to not install it.

Everything in the file can be found elsewhere.

>   On FC7 i386, "service tofmipd start" fails 100% as following:
> ImportError: No module named mime

This was caused by a packaging error which I've fixed.  I also added a note to
the spec file explaining why the package contains pythonlib

Spec URL: http://www.symetrix.com/~bjohnson/projects/Fedora-Extras/tmda.spec
SRPM URL:
http://www.symetrix.com/~bjohnson/projects/Fedora-Extras/tmda-1.1.10-2.fc6.src.rpm

* Sat Feb 17 2007 Bernard Johnson <bjohnson> 1.1.10-2
- consistent start/stop of daemon in scriptlets
- move requirements created by scripts to daemon package that requires them
- don't include the INSTALL file - it's information can be found elsewhere
- %%{python_sitelib}/TMDA/pythonlib/email/mime files were improperly packaged
- dependency on initscripts because of use of daemon function
- note regarding tmda inclusion of pythonlib/email
- change tofmipd user to / home directory

Well, 
(In reply to comment #6)
> (In reply to comment #5)
> > * Scriptlets
> Seems to me, the next best choice is /etc or even /.  At least for now, I've
> changed it to /.
Okay.

Almost clean.  For -2:
* Version/Release specific dependency:
  - Usually the dependency against main package should be release specific.
    i.e.
-------------------------------------------------------------
Requires:       tmda = %{version}-%{release}
-------------------------------------------------------------

* Cosmetic issue: consistent macro use
  - Well, you use both
--------------------------------------------------------------
%{_sysconfdir}/rc.d/init.d
%{_initrddir}
--------------------------------------------------------------
   Please unify them.

* Again documentation:
  - Maybe the following documents are useful?
--------------------------------------------------------------
NEWS (usually this should be included)
--------------------------------------------------------------

* For -emacs package:
  - Well, I don't think it is useful to split only one file with 27K 
    and to create another package with have no dependency essentially.
    And.. tmda.el is installed in main package as a documentation.

    IMO simply unifying .el file into main package and having both
    %{_datadir}/emacs and %{_datadir}/emacs/site-lisp directories owned
    also by main package, removing emacs dependency is simpler.

Forgot to check the following..

* python module dependency
  - /usr/bin/tmda-ofmipd contains:
-------------------------------------------------
   346  if remoteauth['proto'] == 'ldap':
   347      try:
   348          import ldap
   349      except ImportError:
   350          raise ImportError, \
   351                'python-ldap (http://python-ldap.sf.net/) required.'
--------------------------------------------------
    Consider to add "Requires: python-ldap"

  - site-packages/TMDA/FilterParser.py contains:
--------------------------------------------------
   724      def __search_cdb(self, pathname, keys, actions, source):
   725          """
   726          Search DJB's constant databases; see <http:/cr.yp.to/cdb.html>.
   727          """
   728          import cdb
   729          cdb = cdb.init(pathname)
   730          found_match = 0
--------------------------------------------------
    Does this package need "python-cdb" (currently not available
    on Fedora)?

(In reply to comment #8)
>     Consider to add "Requires: python-ldap"

ldap authentication is optional.  It is one of several types available.  It is
only required if you use it.  Most people use file authentication or rpop/rimap
authentication as they are the simplest to setup.

>     Does this package need "python-cdb" (currently not available
>     on Fedora)?

cdb storage is optional.  Most people will use dbm instead since it is built-in.

(In reply to comment #7)
>   - Usually the dependency against main package should be release specific.
>     i.e.

Duh, I knew that :-/

> * Cosmetic issue: consistent macro use
>   - Well, you use both
> %{_sysconfdir}/rc.d/init.d
> %{_initrddir}

Good catch, I will fix that.

>   - Maybe the following documents are useful?
> NEWS (usually this should be included)

I will add this.

> * For -emacs package:
>   - Well, I don't think it is useful to split only one file with 27K 
>     and to create another package with have no dependency essentially.
>     And.. tmda.el is installed in main package as a documentation.
> 
>     IMO simply unifying .el file into main package and having both
>     %{_datadir}/emacs and %{_datadir}/emacs/site-lisp directories owned
>     also by main package, removing emacs dependency is simpler.

Isn't this contrary to the same argument being used against logrotate scripts now?

The packaging guidelines
(http://fedoraproject.org/wiki/Packaging/Guidelines#head-a5931a7372c4a00065713430984fa5875513e6d4),
one file and directory ownership say:

"The rule of thumb is that your package should own all of the directories it
creates except those owned by packages which your package depends on."

If I create a subpackage, it has two beneficial effects:
1) I don't break the rules and make exceptions for my package.
2) I don't drag in emacs dependencies to the main package.

>     And.. tmda.el is installed in main package as a documentation.

And to address this specifically...  Yes, it came from documentation, to add
additional functionality as I did with print{cdb,dbm}.  We always have the
option to just include it in %docs and not pull it in like I do.  What do you
think about that?

Well, for -emacs subpackage, I respect your judgement.
So please fix other issues I pointed out and reupload 
spec/srpm.

Spec URL: http://www.symetrix.com/~bjohnson/projects/Fedora-Extras/tmda.spec
SRPM URL:
http://www.symetrix.com/~bjohnson/projects/Fedora-Extras/tmda-1.1.10-3.fc6.src.rpm

* Sat Feb 17 2007 Bernard Johnson <bjohnson> 1.1.10-3
- added NEWS to %%doc
- macro cleanup
- exact version dependency for subpackage

Ooops.  Missed one small item:
Spec URL: http://www.symetrix.com/~bjohnson/projects/Fedora-Extras/tmda.spec
SRPM URL:
http://www.symetrix.com/~bjohnson/projects/Fedora-Extras/tmda-1.1.10-4.fc6.src.rpm


* Sun Feb 18 2007 Bernard Johnson <bjohnson> 1.1.10-4
- missed a %%{version}-%%{release} dependency, fixed it

Okay.

-----------------------------------------------
  This package (tmda) is APPROVED by me.
-----------------------------------------------
  Please add this package to CVSSyncNeeded and wait
  until cvsadmin creates the initial directories for
  this package.

Additional Branches: EL-4, EL-5

EL-4 and EL-5 branches created

Perhaps they did not get created correctly.  I can't find them in my checkout
directory, and they don't show up in the viewvc view either.

Not sure what happened before, but I have just now created EL-4 and EL-5 and
added it to owners.epel.list.

Package Change Request
======================
Package Name: tmda
New Branches: epel7
Owners: bjohnson

Git done (by process-git-requests).