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 #6 from Kevin Fenzi <kevin(a)tummy.com> 2008-12-14 21:33:20 EDT ---
rpmlint on SRPM:
spamassassin.src:37: W: redundant-prefix-tag
The Prefix tag is uselessly defined as %{_prefix} in your spec file. It should
be removed, as it is redundant with rpm defaults.
Removed.
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. ;(
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.
spamassassin.src:542: W: macro-in-%changelog postun
Macros are expanded in %changelog too, which can in unfortunate cases lead to
the package not building at all, or other subtle unexpected conditions that
affect the build. Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally odd
entries eg. in source rpms, which is rarely wanted. Avoid use of macros in
%changelog altogether, or use two '%'s to escape them, like '%%foo'.
Fixed.
spamassassin.src:580: W: macro-in-%changelog post
Macros are expanded in %changelog too, which can in unfortunate cases lead to
the package not building at all, or other subtle unexpected conditions that
affect the build. Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally odd
entries eg. in source rpms, which is rarely wanted. Avoid use of macros in
%changelog altogether, or use two '%'s to escape them, like '%%foo'.
Fixed.
spamassassin.src: W: mixed-use-of-spaces-and-tabs (spaces: line 135,
tab: line
108)
The specfile mixes use of spaces and tabs for indentation, which is a cosmetic
annoyance. Use either spaces or tabs for indentation, not both.
At least that instance fixed up. ;)
spamassassin.src: W: summary-ended-with-dot Spam filter for email
which can be
invoked from mail delivery agents.
Summary ends with a dot.
Fix.
Fixed.
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.
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.
spamassassin.i386: W: non-conffile-in-etc /etc/logrotate.d/sa-update
A non-executable file in your package is being installed in /etc, but is not a
configuration file. All non-executable files in /etc should be configuration
files. Mark the file as %config in the spec file.
Fix.
Good catch. Fixed.
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.
spamassassin.i386: E: non-readable /etc/cron.d/sa-update 0600
The file can't be read by everybody. If this is expected (for security
reasons), contact your rpmlint distributor to get it added to the list of
exceptions for your distro (or add it to your local configuration if you
installed rpmlint from the source tarball).
Probably OK.
This makes little sense to me. I would expect 644 here.
Perhaps Warren can chime in with a reason for 600 ?
spamassassin.i386: E: non-standard-executable-perm
/usr/share/spamassassin/sa-update.cron 0744
A standard executable should have permission set to 0755. If you get this
message, it means that you have a wrong executable permissions in some files
included in your package.
Fix or document.
I suppose this is due to it not making much sense to run as non root
but yet, it's fine it anyone looks at it.
Will document.
spamassassin.i386: E: executable-marked-as-config-file
/etc/rc.d/init.d/spamassassin
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.
Fix.
Fixed.
spamassassin.i386: W: summary-ended-with-dot Spam filter for email
which can be
invoked from mail delivery agents.
Summary ends with a dot.
Fixed.
spamassassin.i386: W: obsolete-not-provided perl-Mail-SpamAssassin
If a package is obsoleted by a compatible replacement, the obsoleted package
must also be provided in order to provide clean upgrade paths and not cause
unnecessary dependency breakage. If the obsoleting package is not a
compatible replacement for the old one, leave out the provides.
See above.
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.
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.
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.
--
Configure bugmail:
https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.