Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 226448 - Merge Review: sysklogd
Summary: Merge Review: sysklogd
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Kevin Fenzi
QA Contact: Fedora Package Reviews List
Depends On:
TreeView+ depends on / blocked
Reported: 2007-01-31 21:03 UTC by Nobody's working on this, feel free to take it
Modified: 2007-11-30 22:11 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2007-02-26 19:17:26 UTC
kevin: fedora-review+

Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 21:03:52 UTC
Fedora Merge Review: sysklogd
Initial Owner:

Comment 1 Kevin Fenzi 2007-02-16 02:42:35 UTC
I'd be happy to review this package. Look for a full review in a bit. 

Comment 2 Kevin Fenzi 2007-02-17 00:53:36 UTC
Sorry for the delay in reviewing this. My build machine had issues... 

OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
OK - License (GPL)
OK - License field in spec matches
See below - License file included in package
OK - Spec in American English
OK - Spec is legible.
See below - Sources match upstream md5sum:
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.

OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
See below - Package owns all the directories it creates.
See below - No rpmlint output.
OK - final provides and requires are sane:


OK - Should build in mock.
OK - Should build on all supported archs
See below - Should have dist tag
OK - Should package latest version
7 outstanding bugs - check for outstanding bugs on package.


1. Might include the COPYING file?

2. What is the upstream source for the sysklogd-1.4.1rh.tar.gz?
This looks like a locally modified version of the upstream sysklogd-1.4.1.tar.gz.
Why is this done instead of using the upstream source and applying patches to it?

3. Since this package has a logrotate file, shouldn't it 'Require: logrotate' ?

4. Our handy little scripty friend rpmlint says:

W: sysklogd prereq-use fileutils /sbin/chkconfig /etc/init.d

The use of PreReq is deprecated. In the majority of cases, a plain Requires
is enough and the right thing to do. Sometimes Requires(pre), Requires(post),
Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

W: sysklogd unversioned-explicit-provides syslog

Is this needed for an old package? Or to smooth transition to something like

W: sysklogd macro-in-%changelog postun
W: sysklogd macro-in-%changelog post
W: sysklogd macro-in-%changelog preun
W: sysklogd macro-in-%changelog preun
W: sysklogd macro-in-%changelog clean
W: sysklogd macro-in-%changelog post

Suggest: Change macros in changelog to use %% so rpm doesn't expand them.

W: sysklogd mixed-use-of-spaces-and-tabs (spaces: line 165, tab: line 59)

Suggest: pick tabs or spaces, don't use both.

E: sysklogd incoherent-logrotate-file /etc/logrotate.d/syslog

This is due to this package being called sysklogd, and the init
script being called syslog. Can we rename this to sysklogd? Or would
that end up breaking too much? As a side note, I have found myself
doing 'service sysklogd restart' and then having to go look and see
that it's called syslog. ;)

W: sysklogd service-default-enabled /etc/rc.d/init.d/syslog

In this case we can ignore that, we want a syslog by default. ;)

5. You could add a dist tag, but this package appears dead upstream,
so it's unlikely that it would need to be rebased much if any.

6. There are 7 outstanding bugs. You might look through them and see
if any of them could be addressed while other items are being taken care
of for this review.

Comment 3 Jose Pedro Oliveira 2007-02-17 02:11:30 UTC
(In reply to comment #2)
> ...
> b)
> W: sysklogd unversioned-explicit-provides syslog
> Is this needed for an old package? Or to smooth transition to something like
> syslog-ng?
To smooth the transition to an alternative log daemon.  More details in bug #172885.

 * Bug 172885: syslog-ng gets removed when sysklod is updated


Comment 4 Peter Vrabec 2007-02-20 16:38:12 UTC
fixed in sysklogd-1.4.1-47.fc7

2) I don't know, but upstream doesn't seem to be alive.
3) I think "no". I didn't find many "Require: logrotate" in spec files of 
packages, which rotate its files.
4e) I have renamed logrotate.d/syslog but not going to do the same for 
init.t/syslog. I'm worry it could make more problems.
5) What dist tag do you mean?
6) I'm not going to fix any of these.

Comment 5 Jose Pedro Oliveira 2007-02-20 16:53:07 UTC
(In reply to comment #4)
> fixed in sysklogd-1.4.1-47.fc7

> 3) I think "no". I didn't find many "Require: logrotate" in spec files of 
> packages, which rotate its files.

In the syslog-ng.spec there is a requirement for logrotate: syslog-ng installs
a file in the /etc/logrotate.d directory which is owned by the logrotate package.

   $ rpm -qf /etc/logrotate.d/

> 4e) I have renamed logrotate.d/syslog but not going to do the same for 
> init.t/syslog. I'm worry it could make more problems.

The rename broke the usage of syslog-ng as a sysklogd replacement. Syslog-ng
needs to ship the same file as sysklogd (same MD5 digest to avoid file
conflicts): logrotate doesn't like that two different configuration scripts
rotate the same log files.


Comment 6 Jose Pedro Oliveira 2007-02-20 17:01:58 UTC
Another improvement would be to preserve the files timestamps during
installation, i.e., use install -p .


Comment 7 Peter Vrabec 2007-02-20 18:09:39 UTC
thnx. Jose, all issues you metioned are fixed in sysklogd-1.4.1-48.fc7.

Comment 8 Kevin Fenzi 2007-02-21 03:44:03 UTC
Thanks Peter. 

1. good. ok. 
2. Not sure what to do here... The package review guidelines say: 
"- MUST: The sources used to build the package must match the upstream source,
as provided in the spec URL. Reviewers should use md5sum for this task."

Is there anyone you might be able to ask to find out the reason for the
sysklogd-<sersion>rh.tar.gz source? It looks like notting used to maintain this
package around that time, perhaps we could ask him?

3. good. ok. 

4. a) b) c) d) good. ok. 
e) ok. I understand your reasoning, although I am not sure what problems
renaming the init file would cause. 
f) ok. 

5. You can read about the dist tag here:
basically this allows you to ship the same version in diffrent branches, and
have the fc6 version have a .fc6, and so on, so upgrades work. 

6. ok, none of them seem package related to me... 

Sorry for the breakage in syslog-ng Jose. ;( 

So, the only blocker I see left is point #2. Perhaps notting can enlighten us. 

Comment 9 Bill Nottingham 2007-02-21 04:12:26 UTC
2 - Too many patches, so it was maintained in CVS (upstream sysklogd imported on
a vendor branch.)

See sysklogd

Comment 10 Kevin Fenzi 2007-02-21 05:06:41 UTC
The differences between the upstream and the rh version seems to be around 26k
once you remove the sysklogd.spec and the redhat/Changelog files (which was last
updated in 2004). many of the remaning changes are pretty simple logical files
(like the files under the redhat/ dir, which are config, logrotate, etc, files).

For comparison, there is a ipv6 patch thats 21k.  

Peter: Would you be willing to split out the changes as logical patches and then
use the base upstream source?

Comment 11 Peter Vrabec 2007-02-21 12:15:31 UTC
2) We can do nothing. I don't want to use upstream sources, because upstream 
is dead and some other distros are using our sources. I'd like to rather merge 
patches with RH sources, but need to be sure the sysklogd is stable as much as 
possible. I can remove URL(it doesn't point to our sources), if you don't like 

5) There was a dist tag in spec file, wasn't it?

Comment 12 Kevin Fenzi 2007-02-21 23:50:07 UTC
2. ok. 

Checking out a copy from the above cvs, making a .tar.gz and diffing between
that version and whats shipped in your src rpm shows one diffrence... 
you have a sysklogd.spec file in your tar.gz, which doesn't exist in cvs. 
I assume thats generated from the, so it can be ignored. 

Can you add a comment to the spec before the Source line indicating how someone 
could check out the source from rhlinux cvs? something like: 

# The source for this package was pulled from cvs.
# Use the following commands to generate the tarball:
#  export
#  cvs login (hit return)
#  cvs co sysklogd
#  mv sysklogd sysklogd-%{version}rh
#  tar -czvf sysklogd-%{version}rh.tar.gz sysklogd-%{version}rh

5. There is indeed a dist tag. That was an old/outdated comment. 

I see no further blockers here. If you could make that comment change, we can
call this package APPROVED. 

Comment 13 Bill Nottingham 2007-02-21 23:58:30 UTC
For generating the tarball, see the tag-archive/create-archive sections in the

Comment 14 Kevin Fenzi 2007-02-24 03:27:38 UTC
Per the new review guidelines I am reassigning to me (reviewer). 
Please close this rawhide once you have made the change from comment #12 and 
pushed it out to rawhide/devel.

Comment 15 Peter Vrabec 2007-02-26 10:12:59 UTC
comment #12 is fixed in sysklogd-1.4.2-1.fc7, which is a new upstream(RH) 

Comment 16 Kevin Fenzi 2007-02-26 19:17:26 UTC
Yep. Looks good from here... I'll go ahead and close this review rawhide. 
Thanks again for all your patience. 

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