Bug 196101 - Review Request: mimedefang
Summary: Review Request: mimedefang
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-06-21 01:25 UTC by Robert Scheck
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-09-16 18:16:14 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Robert Scheck 2006-06-21 01:25:44 UTC
Spec URL: http://labs.linuxnetz.de/bugzilla/mimedefang.spec
SRPM URL: http://labs.linuxnetz.de/bugzilla/mimedefang-2.57-2.src.rpm
Description: MIMEDefang is an e-mail filter program which works with Sendmail
8.12 and later. It filters all e-mail messages sent via SMTP. MIMEDefang splits
multi-part MIME messages into their components and potentially deletes or 
modifies the various parts. It then reassembles the parts back into an e-mail 
message and sends it on its way.

There are some caveats you should be aware of before using MIMEDefang. 
MIMEDefang potentially alters e-mail messages. This breaks a "gentleman's
agreement" that mail transfer agents do not modify message bodies. This could 
cause problems, for example, with encrypted or signed messages.

Unfortunately rpmlint isn't quiet on x86_32:
E: mimedefang non-standard-uid /var/log/mimedefang defang
E: mimedefang non-standard-gid /var/log/mimedefang defang
E: mimedefang non-standard-dir-perm /var/log/mimedefang 0750
E: mimedefang non-standard-uid /var/spool/MD-Quarantine defang
E: mimedefang non-standard-gid /var/spool/MD-Quarantine defang
E: mimedefang non-standard-dir-perm /var/spool/MD-Quarantine 0750
E: mimedefang non-standard-uid /var/spool/MIMEDefang defang
E: mimedefang non-standard-gid /var/spool/MIMEDefang defang
E: mimedefang non-standard-dir-perm /var/spool/MIMEDefang 0750
W: mimedefang service-default-enabled /etc/rc.d/init.d/mimedefang
W: mimedefang incoherent-subsys /etc/rc.d/init.d/mimedefang $prog

IMHO all lines marked with error can't be really avoided, the first warning line should be correct...when installing MIMEDefang, it should be enabled, too. Last warning line seems to be caused by some rpmlint confusion ;-)

Comment 1 Paul Howarth 2006-06-21 11:13:04 UTC
(In reply to comment #0)
> Unfortunately rpmlint isn't quiet on x86_32:
> E: mimedefang non-standard-uid /var/log/mimedefang defang
> E: mimedefang non-standard-gid /var/log/mimedefang defang
> E: mimedefang non-standard-dir-perm /var/log/mimedefang 0750
> E: mimedefang non-standard-uid /var/spool/MD-Quarantine defang
> E: mimedefang non-standard-gid /var/spool/MD-Quarantine defang
> E: mimedefang non-standard-dir-perm /var/spool/MD-Quarantine 0750
> E: mimedefang non-standard-uid /var/spool/MIMEDefang defang
> E: mimedefang non-standard-gid /var/spool/MIMEDefang defang
> E: mimedefang non-standard-dir-perm /var/spool/MIMEDefang 0750
> W: mimedefang service-default-enabled /etc/rc.d/init.d/mimedefang
> W: mimedefang incoherent-subsys /etc/rc.d/init.d/mimedefang $prog
> 
> IMHO all lines marked with error can't be really avoided, the first warning
line should be correct...when installing MIMEDefang, it should be enabled, too.
Last warning line seems to be caused by some rpmlint confusion ;-)

I would agree with you on all of these apart from the service-default-enabled
one. The system admin should be the one to enable a service if they want it, and
sendmail needs to be reconfigured manually to talk to it anyway.

Comment 2 Robert Scheck 2006-06-21 13:06:51 UTC
Accepted and changed, but 'chkconfig --add %{name}' has to be executed only at 
installing then. Otherwise it will disabled per default during each upgrade...

Comment 3 Paul Howarth 2006-06-21 13:27:23 UTC
(In reply to comment #2)
> Accepted and changed, but 'chkconfig --add %{name}' has to be executed only at 
> installing then. Otherwise it will disabled per default during each upgrade...

Running "chkconfig --add" at upgrade time doesn't result in default-disabled
initscripts getting disabled; at least it doesn't for spamass-milter and
milter-regex, which I maintain. Try it and see...


Comment 4 Robert Scheck 2006-06-21 13:39:08 UTC
Oh okay...didn't know that, yet. Further comments regarding MIMEDefang?

Comment 5 Paul Howarth 2006-06-21 13:52:48 UTC
(In reply to comment #4)
> Oh okay...didn't know that, yet. Further comments regarding MIMEDefang?

I'm happy to review this but it'll take some time and I'm rather busy at the
moment, so I left this review request in NEW state so someone else might step in
and review it earlier.

Comment 6 Jason Tibbitts 2006-09-10 01:33:23 UTC
This fails to build on x86_64 rawhide:

configure: WARNING: Oops.. I couldn't find libmilter.a or libmilter.so.  Please
install Sendmail
configure: WARNING: and its libraries.  You must run Build in the libmilter/
directory
configure: WARNING: to compile libmilter.
error: Bad exit status from /var/tmp/rpm-tmp.96120 (%build)

/usr/lib64/libmilter.a does exist; I'd guess it's looking in /usr/lib.

Comment 7 Robert Scheck 2006-09-10 10:55:44 UTC
Jason, could you please add --with-milterlib=%{_libdir} to %configure for 
testing whether it resolves the problem and building of mimedefang works?

Comment 8 Jason Tibbitts 2006-09-11 15:54:44 UTC
Yes, that does indeed fix the problem.

I'm seeing a couple of additional rpmlint issues that I don't see in the
discussion above:

W: mimedefang mixed-use-of-spaces-and-tabs
  Your indentation is a bit odd (both tabs and spaces) but I think this warning
is bogus anyway.

E: mimedefang-debuginfo empty-debuginfo-package
  Indeed, there is nothing at all in the debuginfo package.  I think the
executables are being stripped:

test "" != "1" && strip md-mx-ctrl
test "" != "1" && strip mimedefang
test "" != "1" && strip mimedefang-multiplexor

The Makefile seems to use DONT_STRIP for this.  I changed the make line to:

make %{?_smp_mflags} DONT_STRIP=1

and things are better.

Comment 9 Robert Scheck 2006-09-11 18:38:57 UTC
Okay, fixed everything we talked about and pushing -3 now...

Comment 10 Jason Tibbitts 2006-09-16 03:59:05 UTC
Unfortuantely I have no way to test this; I long ago dumped Sendmail for Exim. 
But I'll go ahead and review the form of the package and work from the
assumption that you've done the necessary testing.

Comment 11 Jason Tibbitts 2006-09-16 04:28:18 UTC
There seems to be something resembling tests in the "tests" directory, but I
don't see how you would actually run them.  I think you actually have to set up
the system with the test filter and then send the test messages through it,
which wouldn't be doable in an rpm.

It looks like RPM's automatic Perl dependency generation gets confused and comes
up with duplicated dependencies for perl(Digest::SHA1) and perl(MIME::Tools)
with different versioning requirements.  Unfortunately these will need to be
filtered.

Review:
* source files match upstream:
   e55b22dda54c4a3b52e1fbeb9135b0cf  mimedefang-2.57.tar.gz
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible.  License text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* debuginfo package looks complete.
* rpmlint has only ignorable errors.
X final provides and requires are sane:
   config(mimedefang) = 2.57-3.fc6
   mimedefang = 2.57-3.fc6
  =
   /bin/bash
   /bin/sh
   /sbin/chkconfig
   /sbin/service
   /usr/bin/perl
   /usr/sbin/useradd
   config(mimedefang) = 2.57-3.fc6
   libperl.so()(64bit)
   perl >= 0:5.001
X  perl(Digest::SHA1)
   perl(Digest::SHA1) >= 2.00
   perl(Getopt::Std)
   perl(IO::Handle)
   perl(IO::Select)
   perl(IO::Socket)
   perl(IO::Stringy) >= 1.212
   perl(MIME::Base64) >= 3.03
   perl(MIME::Parser)
X  perl(MIME::Tools) >= 5.410
   perl(MIME::Tools) >= 5.413
   perl(MIME::Words)
   perl(Mail::SpamAssassin) >= 1.6
   perl(POSIX)
   perl(Socket)
   perl(Sys::Hostname)
   perl(Sys::Syslog)
   perl(Time::Local)
   perl(lib)
   perl(strict)
   perl(vars)
   perl(warnings)
   perl-MailTools >= 1.15
   sendmail-cf >= 8.12.0
* %check is not present; running test suite not feasible within rpmbuild.
* no shared libraries are added to the regular linker search paths.
* package is not relocatable.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* scriptlets are OK (adding a service and controlling the daemon)
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no libtool .la droppings.

Comment 12 Robert Scheck 2006-09-16 10:32:41 UTC
Well, I think removing the versionified requirement to perl(Digest::SHA1) would 
be the best as the first official perl-Digest-SHA1 package ever build for a Red 
Hat Linux system was 2.00 and it was build in 2002 when 2.00 appeared in 1998.

Same applies to my own versionified requirement to perl(MIME::Tools), which is 
originally taken from the README file, but I absolutely can't see any problem 
when running it using perl(MIME::Tools) 5.410 which seems to be required as the 
minimum requirement by the code, too.

Removing these two added-by-hand requirements fits to me, is this acceptable?

Comment 13 Jason Tibbitts 2006-09-16 13:46:57 UTC
Sure, that's the simplest thing to do.  Since that's the only issue and the fix
is trivial, I'll go ahead and approve and you can just take them out before you
check in.

APPROVED

Comment 14 Robert Scheck 2006-09-16 18:16:14 UTC
17722 (mimedefang): Build on target fedora-development-extras succeeded.
17723 (mimedefang): Build on target fedora-5-extras succeeded.


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