[Bug 226426] Merge Review: spamassassin

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





--- Comment #7 from Jon Ciesla <limb at jcomserv.net>  2008-12-15 09:49:48 EDT ---
(In reply to comment #6)

> >spamassassin.src:72: W: unversioned-explicit-obsoletes perl-Mail-SpamAssassin
> >The specfile contains an unversioned Obsoletes: token, which will match all
> >older, equal and newer versions of the obsoleted thing.  This may cause update
> >problems, restrict future package/provides naming, and may match something it
> >was originally not inteded to match -- make the Obsoletes versioned if
> >possible.
> >
> >Fix.
> 
> Well, the problem here is that upsteam uses that package name. 
> So, if someone installs the upstream rpms, then decides to upgrade 
> to the fedora one, without this they will get a confusing mix. ;( 

Then commenting this in the spec should be sufficient.

> 
> >spamassassin.src:101: W: rpm-buildroot-usage %build %{__perl} Makefile.PL
> >DESTDIR=$RPM_BUILD_ROOT/ SYSCONFDIR=%{_sysconfdir} INSTALLDIRS=vendor
> >ENABLE_SSL=yes < /dev/null
> >$RPM_BUILD_ROOT should not be touched during %build or %prep stage, as it will
> >break short circuiting.
> >
> >There may be a good reason for this.  Is there?
> 
> It's not clear to me where it is using the build root. It's setting DESTDIR to
> it, but it shouldn't be using it. Will dig more, but ideas welcome. 

I'll peek at it.


> >spamassassin.src: W: strange-permission spamassassin-helper.sh 0755
> >A file that you listed to include in your package has strange permissions.
> >Usually, a file should have 0644 permissions.
> >
> >Fix, or document in spec.
> 
> It's a shell script that runs and shows the exit code (spam/notspam). 
> I guess I can add a comment that it's expected to be executable. 

That'd be perfect.

> 
> >rpmlint on RPMS:
> >
> >spamassassin.i386: E: incoherent-logrotate-file /etc/logrotate.d/sa-update
> >Your logrotate file should be named /etc/logrotate.d/<package name>.
> >
> >Fix, if it won't be too catastrophic.
> 
> Well, it's not spamassassin itself that logs anything, it's the daily
> sa-update job that pulls updates to rules. I think it makes more sense 
> to leave it as sa-update since thats the command that generates the logs. 

Agreed, might want to comment in spec.


> 
> >spamassassin.i386: E: executable-marked-as-config-file
> >/etc/mail/spamassassin/spamassassin-helper.sh
> >Executables must not be marked as config files because that may prevent
> >upgrades from working correctly. If you need to be able to customize an
> >executable, make it for example read a config file in /etc/sysconfig.
> >
> >????
> 
> Humm. Not sure why thats marked as config. No one should ever change it. 
> Sadly, thats generated the file that the make process generates. 
> It might need a patch or getting upstream to fix it. 

Hmm.



> >spamassassin.i386: W: conffile-without-noreplace-flag
> >/etc/rc.d/init.d/spamassassin
> >A configuration file is stored in your package without the noreplace flag. A
> >way to resolve this is to put the following in your SPEC file:
> >%config(noreplace) /etc/your_config_file_here
> 
> Fixed above by making it not a config file. 
> 
> >spamassassin.i386: W: dangerous-command-in-%post cp
> 
> This is so that updates with old config file options that are no longer 
> supported will get updated. I don't see any easy way around it. 

Neither do I.  Comment in spec.

> >spamassassin.i386: W: no-reload-entry /etc/rc.d/init.d/spamassassin
> >In your init script (/etc/rc.d/init.d/your_file), you don't have a 'reload'
> >entry, which is necessary for good functionality.
> 
> spamd doesn't have any functionality to do a reload without just restarting
> as far as I know. 

In this instance, just make reload do what restart does.

> >Otherwise, full review looks good, no other blockers.
> 
> Ok. 
> 
> new spec: http://www.scrye.com/~kevin/fedora/spamassassin.spec
> diff against old: http://www.scrye.com/~kevin/fedora/spamassassin.diff
> scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=998690
> 
> Warren is going to look it over as well.

I'll await his input.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.