Bug 589867

[mrunge@mrungexp SPECS]$ rpmlint logcheck.spec ../RPMS/noarch/logcheck-1.3.8-1.fc13.noarch.rpm ../SRPMS/logcheck-1.3.8-1.fc13.src.rpm 
2 packages and 1 specfiles checked; 0 errors, 0 warnings.

Package Review
==============

Key:
- = N/A
x = Check
! = Problem
? = Not evaluated

=== REQUIRED ITEMS ===
[x]  Package is named according to the Package Naming Guidelines.
[x]  Spec file name must match the base package %{name}, in the format
%{name}.spec.
[x]  Package meets the Packaging Guidelines.
[x]  Rpmlint is silent
[x]  Package is not relocatable.

[!]  Buildroot is correct
(%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n))
its not needed anymore, you can remove it 

[x]  Package is licensed with an open-source compatible license and meets other
legal requirements as defined in the legal section of Packaging Guidelines.
[x]  License field in the package spec file matches the actual license.
License type:GPLv2
[x]  If (and only if) the source package includes the text of the license(s) in
its own file, then that file, containing the text of the license(s) for the
package is included in %doc.
[x]  Spec file is legible and written in American English.
[x]  Sources used to build the package matches the upstream source, as provided
in the spec URL.
MD5SUM this package    :86ea9f35183f28f95deb0aba509efb61
MD5SUM upstream package:86ea9f35183f28f95deb0aba509efb61

[x]  Package is not known to require ExcludeArch, OR:
[x]  All build dependencies are listed in BuildRequires, except for any that
[-]  The spec file handles locales properly.
[-]  ldconfig called in %post and %postun if required.
[-]  Package must own all directories that it creates.
[-]  Package requires other packages for directories it uses.
[x]  Package does not contain duplicates in %files.

[!]  Permissions on files are set properly.
retain %config(noreplace) lines,remove %attr(0755,root,root) lines,also group can be logcheck ?
your makefile handles all this

no need to list %dir %{_datadir}/logtail, just %{_datadir}/logtail/* is enough

replace
%{_sbindir}/logcheck
%{_sbindir}/logtail
%{_sbindir}/logtail2

with %{_sbindir}/*


[x]  Package has a %clean section, which contains rm -rf %{buildroot} (or
$RPM_BUILD_ROOT).
[x]  Package consistently uses macros.
[x]  Package contains code, or permissable content.

[!]  Large documentation files are in a -doc subpackage, if required.
make -doc package,also there is man page which you need to install in man directory

[x]  Package uses nothing in %doc for runtime.
[-]  Header files in -devel subpackage, if present.
[-]  Static libraries in -devel subpackage, if present.
[-]  Package requires pkgconfig, if .pc files are present.
[-]  Development .so files in -devel subpackage, if present.
[-]  Fully versioned dependency in subpackages, if present.
[x]  Package does not contain any libtool archives (.la).
[-]  Package contains a properly installed %{name}.desktop file if it is a GUI
application.
[x]  Package does not own files or directories owned by other packages.


Needs to be Fixed
==================
According to install instructins
#-Extract logcheck and run make install.
-Add an unpriviliged user for running logcheck. (typicallly named "logcheck")
-chown -R logcheck /etc/logcheck /var/lock/logcheck /var/lib/logcheck
-Be sure this user can access your log files
-Edit logcheck configuration files in /etc/logcheck.  Most importantly
logcheck.conf.  logcheck.logfiles contains a list of logfiles to be scanned.
-Install logcheck cron job.  There is a sample in debian/logcheck.cron.d

where you are performing these steps ?
you may use use scriptlets
see http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
Please Fix the above issues 
please do read
http://fedoraproject.org/wiki/Packaging/Guidelines

Thank you for your review.

I did not perform the install routine from debian, mainly because it creates lots of rpmlint warnings and some errors regarding wrong dir-permissions.

I changed my mind about this. You'll find the corrected spec and srpm under the following urls.

Spec URL: http://www.matthias-runge.de/fedora/logcheck.spec
SRPM: http://www.matthias-runge.de/fedora/logcheck-1.3.8-2.fc13.src.rpm

changelog
* Sat May 8 2010 Matthias Runge <mrunge> 1.3.8-2
- install man page
- clean up permissions and files section
- user logcheck added

Even if Fedora does not require the BuildRoot tag, I'll leave it there; I'd like to include logcheck in EPEL, too.

Version 1.3.8-3 contains a modified list of logfiles to watch, since fedora does not use 
-/var/log/syslog
-/var/log/auth.log

Spec URL: http://www.matthias-runge.de/fedora/logcheck.spec
SRPM: http://www.matthias-runge.de/fedora/logcheck-1.3.8-3.fc13.src.rpm

Update:

SRPM: http://www.matthias-runge.de/fedora/logcheck-1.3.8-5.fc13.src.rpm
SPEC: http://www.matthias-runge.de/fedora/logcheck.spec

Version 1.3.8-5 contains some fixes:
- add lockfile-progs as requirement 
- debians run-parts accepts --list parameter, fedoras doesn't need it

lockfile-progs is a new review request
https://bugzilla.redhat.com/show_bug.cgi?id=601115

Mohammed Imran, do you plan make this review? No flag set there.

Hi pavel,

         If you are interested, you can take the review ;).Im bit busy now,so wont be able to make the review till weekend.

Mohammed, no, I do not want take it form you! If you plan do it - please.
Now I see it have long tail of dependencies instead I simple help you with it.

I see, it looks like much work to review logcheck or it's dependencies.
Luckily those dependants are really small and any help is greately appreciated.

Thanks anyway!

I've submitted all dependand packages for review, in total 5 dependant packages. As I already wrote in #c9 , those are really small and any help is grateful taken

(In reply to comment #2)
> no need to list %dir %{_datadir}/logtail, just %{_datadir}/logtail/* is enough

Not quite. You actually need:

%{_datadir}/logtail/

This includes the directory and its contents. Using just %{_datadir}/logtail/* would mean that the package did not own the directory itself, which would be wrong since none of the package's dependencies own it either.

Similarly you should add:
%dir %{_sysconfdir}/%{name}/
for ownership of this directory. The %dir is needed here because you want to mark the contents of the directory as %config(noreplace).

> replace
> %{_sbindir}/logcheck
> %{_sbindir}/logtail
> %{_sbindir}/logtail2
> 
> with %{_sbindir}/*

That's a matter of style and I would personally prefer the original form.

I wouldn't use "logcheck" as the default group in %defattr as it will make it the default group for everything, including the binaries, documentation, cron scripts etc., which is probably not what you want.

I would have your package create and own the /var/lib/logcheck directory rather than having useradd create it in %pre, so that the directory gets removed if the package is removed from the system.

> %{_datadir}/logtail/

That suits me better.
> 

> 
> I wouldn't use "logcheck" as the default group in %defattr as it will make it
> the default group for everything, including the binaries, documentation, cron
> scripts etc., which is probably not what you want.
> 
> I would have your package create and own the /var/lib/logcheck directory rather
> than having useradd create it in %pre, so that the directory gets removed if
> the package is removed from the system.    

That's a little difficult. First of all, I fully agree with you, but what happens with the "logcheck"-user when the package is removed. Created users should be left on the system, afaik. When the user is left, why remove his home dir?

(In reply to comment #12)
> > I wouldn't use "logcheck" as the default group in %defattr as it will make it
> > the default group for everything, including the binaries, documentation, cron
> > scripts etc., which is probably not what you want.
> > 
> > I would have your package create and own the /var/lib/logcheck directory rather
> > than having useradd create it in %pre, so that the directory gets removed if
> > the package is removed from the system.    
> 
> That's a little difficult. First of all, I fully agree with you, but what
> happens with the "logcheck"-user when the package is removed. Created users
> should be left on the system, afaik. When the user is left, why remove his home
> dir?    

The user would be left on the system, but wouldn't be actively used for anything - it would just own any files that may have been left around from the running of the program - if any. The absence of a home directory wouldn't matter for that purpose.

I've updated from 1.3.8 to version 1.3.11. I took the hint from Paul and let install section create homedir for logcheck, the package owns homedir, too.

To enhance readability, I substituted the two mini-patches by sed-invocations.

Updated files:
SPEC: http://www.matthias-runge.de/fedora/logcheck.spec
SRPM: http://www.matthias-runge.de/fedora/logcheck-1.3.11-1.fc13.src.rpm

lockfile-progs (as last remaining dependency) is in -testing now. All other dependencies are approved and hit the repositories.

Can we proceed with the review now?

(In reply to comment #15)
> lockfile-progs (as last remaining dependency) is in -testing now. All other
> dependencies are approved and hit the repositories.
> 
> Can we proceed with the review now?

Yes, I'll do it in the next day or two.

Sorry for the delay. There's a few issues that need to be addressed

+ rpmlint output

 rpmlint logcheck*
logcheck.noarch: W: non-standard-uid /var/lib/logcheck logcheck
logcheck.noarch: W: non-standard-gid /var/lib/logcheck logcheck
logcheck.noarch: E: non-standard-dir-perm /var/lib/logcheck 0700L
logcheck.noarch: E: non-standard-dir-perm /etc/logcheck/cracking.d 02750L
logcheck.noarch: E: non-standard-dir-perm /etc/logcheck/violations.ignore.d 02750L
logcheck.noarch: E: non-readable /etc/logcheck/logcheck.logfiles 0640L
logcheck.noarch: E: non-standard-dir-perm /etc/logcheck/ignore.d.server 02750L
logcheck.noarch: E: non-standard-dir-perm /etc/logcheck/ignore.d.paranoid 02750L
logcheck.noarch: E: non-readable /etc/logcheck/logcheck.conf 0640L
logcheck.noarch: W: non-standard-uid /var/lock/logcheck logcheck
logcheck.noarch: W: non-standard-gid /var/lock/logcheck logcheck
logcheck.noarch: E: non-standard-dir-perm /var/lock/logcheck 0700L
logcheck.noarch: W: non-standard-gid /etc/logcheck logcheck
logcheck.noarch: E: non-standard-dir-perm /etc/logcheck 0750L
logcheck.noarch: E: non-standard-dir-perm /etc/logcheck/ignore.d.workstation 02750L
logcheck.noarch: E: non-standard-dir-perm /etc/logcheck/violations.d 02750L
logcheck.noarch: E: non-standard-dir-perm /etc/logcheck/cracking.ignore.d 02750L
logcheck.noarch: W: manual-page-warning /usr/share/man/man1/Logcheck.8.gz 1: warning: `\"' not defined
logcheck.noarch: W: no-manual-page-for-binary logcheck
logcheck.src: W: invalid-url Source0: http://ftp.de.debian.org/debian/pool/main/l/logcheck/logcheck_1.3.11.tar.gz HTTP Error 404: Not Found
logcheck.spec: W: invalid-url Source0: http://ftp.de.debian.org/debian/pool/main/l/logcheck/logcheck_1.3.11.tar.gz HTTP Error 404: Not Found
2 packages and 1 specfiles checked; 12 errors, 9 warnings.

+ package name satisfies the packaging naming guidelines
+ specfile name matches the package base name
+ package should satisfy packaging guidelines
+ license meets guidelines and is acceptable to Fedora
+ license matches the actual package license
- latest version packaged
  version 1.3.13 is available

+ %doc includes license file
+ spec file written in American English
+ spec file is legible
- upstream sources match sources in the srpm
  packaged version doesn't seem to be available any longer
+ package successfully builds on at least one architecture
  tested using koji scratch build
+ BuildRequires list all build dependencies
n/a %find_lang instead of %{_datadir}/locale/*
n/a binary RPM with shared library files must call ldconfig in %post and %postun+ does not use Prefix: /usr
+ package owns all directories it creates
+ no duplicate files in %files
+ Package perserves timestamps on install
- Permissions on files must be set properly
  rpmlint complains about a number of permissions
+ %defattr line
+ consistent use of macros
+ package must contain code or permissible content
n/a large documentation files should go in -doc subpackage
+ files marked %doc should not affect package runtime 
n/a header files should be in -devel
n/a static libraries should be in -static
n/a packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
n/a libfoo.so must go in -devel
n/a devel must require the fully versioned base
n/a packages should not contain libtool .la files
n/a packages containing GUI apps must include %{name}.desktop file
+ packages must not own files or directories owned by other packages
+ filenames must be valid UTF-8

Optional:

+ if there is no license file, packager should query upstream to include it
n/a translations of description and summary for non-English languages, if
available
+ reviewer should build the package in mock/koji
+ the package should build into binary RPMs on all supported architectures
  package built using koji scratch build
n/a review should test the package functions as described
+ scriptlets should be sane
n/a non -devel packages should require fully versioned base
n/a pkgconfig files should go in -devel
  shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or
/usr/sbin
+ Package should have man files

Thanks for the review.

Some of those errors resp. warnings are not so easy to fix. I decided to set the permissions more fedora-like. INSTALL-file says, logcheck-user should own the config-files. I cannot see a reason for this, or why root may not own them, esp. why those config files may not me read by other users. Information hiding does not work in most cases. 

mrunge@sofja noarch]$ rpmlint logcheck-1.3.13-1.fc14.noarch.rpm
logcheck.noarch: W: non-standard-uid /var/lib/logcheck logcheck
logcheck.noarch: W: non-standard-gid /var/lib/logcheck logcheck
logcheck.noarch: E: non-standard-dir-perm /var/lib/logcheck 0700L
logcheck.noarch: W: manual-page-warning /usr/share/man/man8/logcheck.8.gz 1: warning: macro `\"' not defined
logcheck.noarch: W: non-standard-uid /var/lock/logcheck logcheck
logcheck.noarch: W: non-standard-gid /var/lock/logcheck logcheck
logcheck.noarch: E: non-standard-dir-perm /var/lock/logcheck 0700L

Adding a new user and group makes such messages. logcheck should own /var/lib/logcheck and /var/lock/logcheck. Those permissions are sane to me.

Fixed files:
SPEC: http://www.matthias-runge.de/fedora/logcheck.spec
SRPM: http://www.matthias-runge.de/fedora/logcheck-1.3.13-1.fc14.src.rpm

Seems sane. APPROVED!

Thank you Peter!

New Package SCM Request
=======================
Package Name: logcheck
Short Description: analyzes logfiles and sends email
Owners: mrunge
Branches: f13 f14 devel el5 el6

Git done (by process-git-requests).

logcheck-1.3.13-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/logcheck-1.3.13-1.fc14

logcheck-1.3.13-1.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/logcheck-1.3.13-1.fc13

logcheck-1.3.13-2.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/logcheck-1.3.13-2.el5

logcheck-1.3.13-2.el5 has been pushed to the Fedora EPEL 5 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update logcheck'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/logcheck-1.3.13-2.el5

logcheck-1.3.13-2.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/logcheck-1.3.13-2.fc14

logcheck-1.3.13-2.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/logcheck-1.3.13-2.fc13

logcheck-1.3.13-3.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/logcheck-1.3.13-3.el5

logcheck-1.3.13-3.el5 has been pushed to the Fedora EPEL 5 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update logcheck'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/logcheck-1.3.13-3.el5

logcheck-1.3.13-2.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

logcheck-1.3.13-2.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

Thanks very much for adding logcheck to the repository - great tool.  But it doesn't seem to be working at all: it's not filtering any entries out on my system.

I've tested using log messages from a few daemons with /usr/bin/logcheck-test, and the ignore rules are matching correctly, but a run of logcheck then spews out all those log messages without filtering any out.

Here's what I think is happening: logcheck uses /usr/bin/run-parts to pick up the filter rule files from subdirectories of /etc/logcheck.  run-parts looks for executables, and runs them.  But that's not what logcheck wants it to do - logcheck wants it to spit out the names of matching files, not actually execute them.

The Debian version of run-parts has a --list parameter, which does exactly this.  When the logcheck package has been ported across to Fedora, the --list parameter has been removed from the invocation of run-parts (because our version of run-parts doesn't have that parameter), but that simply results in run-parts producing no output.  Which means that logcheck thinks it has no filter rule files.  Which means no log messages ever get filtered out.

I'm surprised that this is the case, because the package should never have been pushed to stable with such a fundamental bug in it.  But I'm fairly sure that's what's going on (on my system, at least - Fedora 13 with logcheck-1.3.13-2.fc13.noarch and crontabs-1.10-32.fc13.noarch).

In confirmation of the above: simply replacing the invocation of run-parts with /bin/ls in the cleanrules() function of logcheck fixes the problem.

We don't want to do this, because it loses the benefits of run-parts (ignoring .rpmsave files, etc.) - but it does demonstrate that that's where the problem is.

Thank you for your report. I'll take a deeper look into this later this day.

Could you please open a new bug instead of reopening an existing one next time?

Thanks for the speedy attention.

Sure - I thought about opening a new bug, but was just obeying the instruction in comment 30: "If problems still persist, please make note of it in this bug report."

logcheck-1.3.13-4.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/logcheck-1.3.13-4.el5

logcheck-1.3.13-5.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.

Package Change Request
======================
Package Name: logcheck
New Branches: epel7
Owners: mrunge

Git done (by process-git-requests).