[Bug 479835] New: Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

Summary: Review Request: log4c - an application message logging library

https://bugzilla.redhat.com/show_bug.cgi?id=479835

Summary: Review Request: log4c - an application message logging
                    library
           Product: Fedora
           Version: rawhide
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: medium
          Priority: low
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail], [e-mail]
   Estimated Hours: 0.0
    Classification: Fedora

Spec URL: http://www.alexhudson.com/fedora/log4c/log4c.spec
SRPM URL: http://www.alexhudson.com/fedora/log4c/log4c-1.2.1-1.fc10.src.rpm
Description:
Log4c is a C language library for flexible logging to files, syslog and other 
destinations. It is modeled after the Log for Java library (log4j), 
staying as close to their API as is reasonable.

This is similar to log4cpp which already exists in Fedora; and is in use by a
number of applications that I would like to package.

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

--- Comment #1 from Alex Hudson (Fedora Address)   2009-01-13 09:29:00 EDT ---
Ah, I also meant to add, this is the first package that I have attempted to
submit, so I would need a sponsor :)

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

manuel wolfshant  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |177841

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

Kevin Fenzi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|[e-mail]    |[e-mail]
               Flag|                            |fedora-review?

--- Comment #2 from Kevin Fenzi   2009-01-25 15:25:52 EDT ---
Hey Alex, I would be happy to look at this and bug 479951 and sponsor you.

Look for a full review later today.

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

--- Comment #3 from Kevin Fenzi   2009-01-25 15:50:57 EDT ---
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name. 
See below - Spec has consistant macro usage. 
OK - Meets Packaging Guidelines. 
OK - License (LGPLv2+)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
ca5412b7515d8901714ab7892323adb6  log4c-1.2.1.tar.gz
ca5412b7515d8901714ab7892323adb6  log4c-1.2.1.tar.gz.orig
See below - 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 has rm -rf RPM_BUILD_ROOT at top of %install

OK - Headers/static libs in -devel subpackage. 
OK - Spec has needed ldconfig in post and postun
OK - .so files in -devel subpackage.
OK - -devel package Requires: %{name} = %{version}-%{release}
See below - .la files are removed.

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. 
OK - Package obey's FHS standard (except for 2 exceptions)
See below - No rpmlint output. 
OK - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock. 
OK - Should build on all supported archs
OK - Should have dist tag
OK - Should package latest version
OK - Should not use file requires outside of /etc, /bin, /sbin, /usr/bin, or
/usr/sbin

Issues:

1. No need for a %check section if there is nothing in it is there?

2. Please change the Source url to match the sourceurl guidelines for
sourceforge
projects? http://fedoraproject.org/wiki/Packaging/SourceURL

3. Please use a consistent type of macro... either $RPM_BUILD_ROOT or
%{buildroot}, 
but not both. As a side note, I don't personally think it's worth it to
macroize
things like mv or mkdir, but thats up to you.

4. Please remove .la files, don't %exclude them. excluding has weird effects
sometimes.

5. Looks like you might be missing a BuildRequires... from the build.log:

checking for EXPAT - version >= 1.95.1...
no
*** Could not run EXPAT test program, checking why...
*** The test program failed to compile or link. See the file config.log for the
*** exact error that occured. This usually means EXPAT was incorrectly
installed
*** or that you have moved EXPAT since it was installed. In the latter case,
you
*** may want to edit the expat-config script: ''
*** Log4C will still run without EXPAT--it uses some bundled
*** lex/yacc code to parse the configuration file

6. rpmlint says:

log4c.x86_64: W: shared-lib-calls-exit /usr/lib64/liblog4c.so.3.1.0
exit@GLIBC_2.2.5
log4c-devel.x86_64: W: no-documentation

You might bring up the first to upstream, the second can be ignored.

7. Since your devel subpackage puts a file under /usr/share/aclocal, you 
probibly need to add a Requires: automake to the devel subpackage.

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

--- Comment #4 from Alex Hudson (Fedora Address)   2009-01-26 09:56:02 EDT ---
Kevin,

Thanks for the detailed comments, much appreciated.

Updated SRPM: http://www.alexhudson.com/fedora/log4c/log4c-1.2.1-2.fc10.src.rpm
SPEC: http://www.alexhudson.com/fedora/log4c/log4c.spec

1: The %check was there to remind me to tie in the built-in tests; however,
they fail to run cleanly at the moment - indeed, with the current compiler
flags, one crashes very nicely. I've removed this entirely for now; I suspect
the tests needs to be reworked upstream before being useful here.

2, 3, 4: Thanks!

5: Well, it was optional, but the config parsing seems much more robust using
Expat instead of the built-in grammar, so I've changed it to use that

6: Yes, indeed - I'll follow this up with the log4c guys

7: I'm going to disagree with you here ;) The .m4 file is there as a
convenience for automake-using developers; you don't need to use automake to
compile against log4c. E.g, I have a project here using CMake to build against
log4c - it doesn't need that macro at all.

Thanks again!

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

--- Comment #5 from Kevin Fenzi   2009-01-27 00:28:50 EDT ---
1. ok. You can always add a comment reminding yourself to add the check when
it's working.

2, 3, 4: All look good.

5. ok. Sounds good.

6. Sounds good.

7. Sadly, thats not going to pass guidelines:

MUST: A package must own all directories that it creates. If it does not create
a directory that it uses, then it should require a package which does create
that directory.

I think your choices are:

- Requires: automake

- Split the .m4 file into a subpackage (log4c-automake?) and Requires: automake
there.

- Move the .m4 file over to being a %doc file.

- Don't ship the .m4 file at all.

Thoughts?

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

--- Comment #6 from Mamoru Tasaka   2009-01-27 03:53:36 EDT ---
About automake .m4 file:

My recognition is that we have not reached consensus yet about
whether every packaging shipping aclocal .m4 files should have
"Requires: automake" or not. c.f.
http://www.redhat.com/archives/fedora-packaging/2007-October/msg00046.html

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

--- Comment #7 from Alex Hudson (Fedora Address)   2009-01-27 04:29:27 EDT ---
Guys,

Thanks very much for the comments - I have a better appreciation of the
automake issue now; I had just thought about it in terms of "what dependencies
are needed to develop?" instead of the technical directory ownership issue.

It does seem on the face of it that moving /usr/share/aclocal to filesystem
would be a good idea, or at least some new automake-macros package which can
then be required and not bring in the entire automake system. However, that's
obviously something which needs agreement project-wide.

So in that context, I suspect the best solution right now is Requires:
automake. It solves the guideline issue, and it's consistent with other -devel
packages. I can put a comment in the spec. about keeping a weather-eye out for
changes to that, but I imagine there would be some mass bug-posting if that
changed anyway.

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

Kevin Fenzi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|177841                      |
               Flag|fedora-review?              |fedora-review+

--- Comment #8 from Kevin Fenzi   2009-01-28 01:11:09 EDT ---
ok, that was the last blocker item I had... so with the addition of the
Requires, this package is APPROVED.

I will sponsor you... continue the process from: 
http://fedoraproject.org/wiki/PackageMaintainers/Join#Get_a_Fedora_Account

Please feel free to contact me here, via email, or on irc if you have any
questions at all.

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

Alex Hudson (Fedora Address)  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-cvs?

--- Comment #9 from Alex Hudson (Fedora Address)   2009-01-29 03:04:04 EDT ---
New Package CVS Request
=======================
Package Name: log4c
Short Description: flexible application message logging library 
Owners: alexh
Branches: F-9 F-10
InitialCC:

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

Kevin Fenzi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-cvs?                 |fedora-cvs+

--- Comment #10 from Kevin Fenzi   2009-01-30 01:37:04 EDT ---
cvs done.

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

Kevin Fenzi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |RAWHIDE

--- Comment #11 from Kevin Fenzi   2009-05-11 02:38:20 EDT ---
This appears to have been imported and built. 
Closing now.

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

Steve Traylen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
               Flag|fedora-cvs+                 |fedora-cvs?

--- Comment #12 from Steve Traylen   2009-12-14 03:11:09 EDT ---
Package Change Request
======================
Package Name: log4c
New Branches: EL-4 EL-5
Owners: stevetraylen

I sent this mail a week ago to Alex requesting and EPEL branch
with no response. I am now going ahead and requesting the branch myself.

Steve

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

Kevin Fenzi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-cvs?                 |fedora-cvs+

--- Comment #13 from Kevin Fenzi   2009-12-14 12:50:36 EDT ---
cvs done.

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

--- Comment #14 from Fedora Update System   2009-12-14 14:02:51 EDT ---
log4c-1.2.1-7.el4 has been submitted as an update for Fedora EPEL 4.
http://admin.fedoraproject.org/updates/log4c-1.2.1-7.el4

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

--- Comment #15 from Fedora Update System   2009-12-14 14:05:16 EDT ---
log4c-1.2.1-7.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/log4c-1.2.1-7.el5

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

--- Comment #16 from Fedora Update System   2010-01-05 17:29:17 EDT ---
log4c-1.2.1-7.el5 has been pushed to the Fedora EPEL 5 stable repository.  If
problems still persist, please make note of it in this bug report.

[Bug 479835] Review Request: log4c - an application message logging library

[Red Hat Bugzilla]

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=479835

--- Comment #17 from Fedora Update System   2010-01-05 17:31:30 EDT ---
log4c-1.2.1-7.el4 has been pushed to the Fedora EPEL 4 stable repository.  If
problems still persist, please make note of it in this bug report.