Bug 210061 - Review Request: msyslog - A daemon for the syslog system log interface
Summary: Review Request: msyslog - A daemon for the syslog system log interface
Keywords:
Status: CLOSED CANTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2006-10-09 19:51 UTC by manuel wolfshant
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-12-06 10:07:36 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
spec file patch (2.90 KB, patch)
2006-10-09 22:18 UTC, Patrice Dumas
no flags Details | Diff
simpler version of msyslog.noBSD.patch (1.09 KB, patch)
2006-10-09 22:20 UTC, Patrice Dumas
no flags Details | Diff
modified, to keep Makefile.in untouched (917 bytes, patch)
2006-10-09 22:21 UTC, Patrice Dumas
no flags Details | Diff
fix the pid file and dir (600 bytes, patch)
2006-10-09 22:22 UTC, Patrice Dumas
no flags Details | Diff

Description manuel wolfshant 2006-10-09 19:51:21 UTC
Spec URL: http://wdl.lug.ro/linux/rpms/msyslog/msyslog.spec
SRPM URL: http://wdl.lug.ro/linux/rpms/msyslog/msyslog-v1.08g-0.10.src.rpm
Description: 
This project is intended as a whole revision of previous Secure Syslogd
project (wich is unsupported by now). It has all functionalities and some
more. The remaining things are Solaris support and Audit compatibility (on the
works).
The whole internal structure was redesigned to work with input and output
modules, standarizing interfaces to facilitate development for using special
devices and flexible configurations.
Current available output modules are classic, mysql, peo, pgsql, regex and
tcp. Available input modules are bsd, linux, unix, tcp and udp.

This is my third submission (second original) and I am still looking for a sponsor

Comment 1 manuel wolfshant 2006-10-09 19:57:46 UTC
rpmlint on the rpm will yield the following error:
E: msyslog shared-lib-without-dependency-information /lib/alat/libmsyslog.so.1.08g

As far as I can tell, libmsyslog.so.1.08g is actually built as a static library
and seems to be so by design.
If anyone can provide a method to make rpmlint happy, I would definitely appreciate.



Comment 2 Patrice Dumas 2006-10-09 22:16:50 UTC
* the version should be 1.08g, since it looks like a postrelease
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#head-18aa467fc6925455e44be682fd336667a17e8933

* the indenting is bad (not a blocker)

* initdir could be %{_sysconfdir}/rc.d/init.d, but both are
  arguable.

* -n %{name}-%{version} is unusefull, it is the default

* the patches and install section are complicated. I propose 
  simplified version and a corresponding spec file patch, I attach them.

* I also attach a patch to use user provided value for localstatedir
  and modify the pid file name.

* the daemon seems not to drop privileges without a patch posted
  on cvs. It should be applied and used in init, together with 
  the creation of a user and so on.

* libmsyslog.so.1.08g shouldn't be a static library, you should patch
  the Makefile.in or the like to have it compiled as a module.

* the stuff at the end of %post should rapidly be removed before
  somebody notice what you were trying to do ;-)

* libmysqlclient.so and libpq.so are dlopened in om_mysql.c and
  om_pgsql.c, that's bad. They should be linked.

Overall I am not convinced that this software is worth packaging in 
fedora. It seems to be unmaintained since 3 years, the build system is
very broken, there are non portable Makefiles, and dlopening the 
libraries seems very bad to me. You can try nevertheless if you 
like, but it will be a fair amount of work.

Comment 3 Patrice Dumas 2006-10-09 22:18:09 UTC
Created attachment 138089 [details]
spec file patch

Comment 4 Patrice Dumas 2006-10-09 22:20:13 UTC
Created attachment 138090 [details]
simpler version of msyslog.noBSD.patch

There is no reason to modify the package like what is done
in msyslog.noBSD.patch. Just fix the broken stuff, no 
need to remove files.

Comment 5 Patrice Dumas 2006-10-09 22:21:32 UTC
Created attachment 138091 [details]
modified, to keep Makefile.in untouched

Comment 6 Patrice Dumas 2006-10-09 22:22:22 UTC
Created attachment 138092 [details]
fix the pid file and dir

Comment 7 Patrice Dumas 2006-10-09 22:23:53 UTC
(In reply to comment #5)
> Created an attachment (id=138091) [edit]
> modified, to keep Makefile.in untouched
> 

This is a modified version of msyslog.x86_64v2.patch


Comment 8 Patrice Dumas 2006-10-09 22:25:02 UTC
Also most of the man pages should be prefixed, for example
by msyslogd.

Comment 9 manuel wolfshant 2006-10-09 23:50:43 UTC
I will look at your suggestions in the morning. 16hrs of continuous work lead to
dumb typo errors :(
Thanks a lot for your efforts and suggestions, Patrice. What I liked most was
your fix for x86_64. I had no idea about the make INSTALL_LIBDIR option, I guess
that's my major lesson for today.

Fast comments:
- I have kept the version as defined by the developer. Actually during the first
iterations I was using 1.08g (hence the setup -n), but switched back to the
original after a talk with a developer friend (while trying to convince him to
help me fix the dlopens). The funny part is that for the devel tree (1.9) the
developer changed the convention used for naming the tar file :)
- I have removed the BSD part on purpose. We do not support/build for openBSD,
do we ?
- About the uglyness in %post: I know. Unfortunately I have not found any other
way of getting rid of the dlopen effect without modifying the source files.
- With one exception, all the man pages already have distinctive names (either
starting with im_, om_ or with msyslog) and do not seem to clash with anything.
Why do you suggest renaming them? Just to be able to locate where do they belong
to from a first glance?



Comment 10 Patrice Dumas 2006-10-10 07:03:44 UTC
(In reply to comment #9)

> Thanks a lot for your efforts and suggestions, Patrice. What I liked most was
> your fix for x86_64. I had no idea about the make INSTALL_LIBDIR option, I guess
> that's my major lesson for today.

It is not something standard at all... In fact after noticing
that make variables where used for all the installed files, I removed
the DESTDIR patch and found the relevant make variables by looking
at the code each time I got an install error.

There is nothing standard in the build system of that package...
All seems to be hand made, but unfortunately some parts (like linking
libraries in a portable way) are better done with tools like libtool,
and the result is quite messy.

> Fast comments:
> - I have kept the version as defined by the developer. Actually during the first
> iterations I was using 1.08g (hence the setup -n), but switched back to the
> original after a talk with a developer friend (while trying to convince him to
> help me fix the dlopens). The funny part is that for the devel tree (1.9) the
> developer changed the convention used for naming the tar file :)

It is not very clear in the guidelines, but if the version is
numeric it is much easier to have an increasing ascii version, 
if it has letters (except for an increasing alphabetical postfix)
ensuring that the version increase in the ascii sense becomes 
harder to achieve. The 'internal' version is 1.08g, so it is certainly
better to use that, even though the tarball is named otherwise. 

> - I have removed the BSD part on purpose. We do not support/build for openBSD,
> do we ?

We don't but we shouldn't modify unused upstream code except when
it breaks something.

> - About the uglyness in %post: I know. Unfortunately I have not found any other
> way of getting rid of the dlopen effect without modifying the source files.

You'll have to :/. There is nothing in the guideline formally forbidding
dlopening although the library is always used, but I don't think you'll 
find any other fedora contributor who would accept the package with such
a wrong behaviour. Fortunately the way things are packaged forbids such 
misbehaviour by hidding the .so in -devel packages...

> - With one exception, all the man pages already have distinctive names (either
> starting with im_, om_ or with msyslog) and do not seem to clash with anything.
> Why do you suggest renaming them? Just to be able to locate where do they belong
> to from a first glance?

A man page like im_tcp is both too general, (a clash may appear
with another package for which the name is relevant), and hard
to find. I personnally wouldn't make that issue a blocker, but 
other reviewers may be stricter.


Comment 11 manuel wolfshant 2006-12-06 10:07:36 UTC
I am closing this because I neither have the time to edit the sources in order
to clean /remove the dlopen stuff not the interest to use this software any
more, as I have switched to syslog-ng.


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