Bug 476247 - Review Request: log4cpp - logging library for c++
Summary: Review Request: log4cpp - logging library for c++
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: David Cantrell
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-12-12 17:34 UTC by jmccann
Modified: 2015-03-23 18:31 UTC (History)
5 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-02-06 00:11:03 UTC
Type: ---
Embargoed:
jkeating: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description jmccann 2008-12-12 17:34:47 UTC
Spec URL: http://people.fedoraproject.org/~mccann/packages/log4cpp.spec
SRPM URL: http://people.fedoraproject.org/~mccann/packages/log4cpp-1.0-1.fc10.src.rpm
Description: 

A library of C++ classes for flexible logging to files, syslog, IDSA and
other destinations. It is modeled after the Log for Java library
(http://www.log4j.org), staying as close to their API as is reasonable.

Comment 1 Jesse Keating 2008-12-12 17:38:57 UTC
Taking review.

Comment 2 Jesse Keating 2008-12-12 19:21:55 UTC
* Name is OK
* spec matches name
* ERROR: "LGPL" is not a valid license name, please see http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#GPL_and_LGPL
* No pre-built binaries found
* Files follow FHS
* Changelog is sane
* Buildroot tag is sane
* Buildroot properly prepped for %install
* No unnecessary requires
* Summary/Description mostly OK, -devel ends in a "." which rpmlint doesn't like.
* Docs packaged
* No static libs
* No rpath
* Consistent use of macros
* smp flags used for make
* WARNING Patches missing bug or comment http://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment
* License review:
  src/AppendersFactory.cpp No license header
  src/BufferingAppender.cpp No license header
  src/DummyThreads.cpp No license header
  src/FactoryParams.cpp No license header
  src/LayoutsFactory.cpp No license header
  src/LevelEvaluator.cpp "
  src/Localtime.cpp "
  src/Localtime.hh "
  src/MSThreads.cpp "
  src/NTEventLogAppender.cpp "
  src/OmniThreads.cpp "
  src/PassThroughLayout.cpp "
  src/PThreads.cpp "
  src/snprintf.c Licensed under "Frontier Artistic License", how does this interact with the other source?  Where is the text of this license?
  src/TriggeringEventEvaluatorFactory.cpp No license header
  tests/testbench.cpp "
  tests/testCategory.cpp "
  tests/testConfig.cpp "
  tests/testErrorCollision.cpp "
  tests/testFilter.cpp "
  tests/testFixedContextCategory.cpp "
  tests/testmain.cpp "
  tests/testNDC.cpp "
  tests/testNTEventLog.cpp "
  tests/testPattern.cpp "
  tests/testPriority.cpp "
  tests/testProperties.cpp "
  tests/testPropertyConfig.cpp "
* Upstream source matches
* Package builds in mock
* ldconfig called correctly
* defatter set correctly
* Might consider a %doc package given the large number of doc files
* ERROR a .pc file is shipped but pkgconfig is not Required.
* rpmlint output:
  log4cpp.i386: W: file-not-utf8 /usr/share/doc/log4cpp-1.0/ChangeLog
  log4cpp.i386: W: file-not-utf8 /usr/share/doc/log4cpp-1.0/THANKS
  log4cpp.i386: W: invalid-license LGPL
  log4cpp.src: W: invalid-license LGPL
  log4cpp-debuginfo.i386: W: invalid-license LGPL
  log4cpp-devel.i386: W: no-documentation
  log4cpp-devel.i386: W: summary-ended-with-dot Header files, libraries and development documentation for log4cpp.
  log4cpp-devel.i386: W: invalid-license LGPL

** SUMMARY **
License issues; LGPL is not valid, many packages missing headers, one file under a license that references a license file that doesn't exist

Summary could nuke '.'

pkgconfig is required since a .pc file is shipped.

comments and/or links missing for the patches.

Comment 3 jmccann 2008-12-12 21:14:27 UTC
Updated to address comments

Spec URL: http://people.fedoraproject.org/~mccann/packages/log4cpp.spec
SRPM URL: 
http://people.fedoraproject.org/~mccann/packages/log4cpp-1.0-1.fc10.src.rpm

My understanding (IANAL) is that the Frontier Artistic License derives from the Artistic License and is compatible with the GPL (and LGPL).

Comment 4 Jesse Keating 2008-12-13 00:13:33 UTC
You're still missing the LICENSE.txt file mentioned in the header for the src/snprintf.c file, as well as adding the license itself to the License: field in the spec.

Also curious if the patches have been submitted upstream (or if they're suitable for upstream).

Only the -devel subpackage should Require pkgconfig since that's where the .pc file is shipped.

Comment 5 jmccann 2008-12-13 02:35:56 UTC
No, my understanding is that once the snprintf code (artistic license) is used in LGPL code the derived work is entirely LGPL.  We do this all the time with such liberally licensed code.  So the spec file header is correct.

I also don't see a requirement in the license conditions to ship LICENSE.txt. Since the upstream code doesn't include it, I'm not sure it is a good idea to add it on our own.  Especially since the entire work is LGPL.

I'll fix the pkgconfig part.

Comment 6 jmccann 2008-12-13 04:19:18 UTC
Updated SRPM and spec for pkgconfig change.

Comment 7 Jesse Keating 2008-12-13 19:39:08 UTC
All right, looks good now.  Approving.

Comment 8 jmccann 2008-12-15 18:57:52 UTC
New Package CVS Request
=======================
Package Name: log4cpp
Short Description: C++ logging library
Owners: mccann
Branches: F-10
InitialCC:

Comment 9 Dennis Gilmore 2008-12-15 20:12:44 UTC
CVS Done

Comment 10 Steve Traylen 2009-09-23 13:55:10 UTC

Package Change Request
======================
Package Name: log4cpp
New Branches: EL-4 EL-5
Owners: mccann stevetraylen


I contacted the maintainer "William Jon McCann" a couple of weeks 
back requesting that he maintain log4cpp within EPEL as well.
As such no response and as I understand it I am now at liberty to
co-maintain the EPEL branches.

Comment 11 jmccann 2009-09-23 21:40:43 UTC
Sorry for the no-response - seriously overloaded.  Feel free to maintain or own the package as you see fit.  Thanks.

Comment 12 Kevin Fenzi 2009-09-24 04:39:14 UTC
cvs done.

Comment 13 Fedora Update System 2009-09-24 12:23:59 UTC
log4cpp-1.0-4.el4 has been submitted as an update for Fedora EPEL 4.
http://admin.fedoraproject.org/updates/log4cpp-1.0-4.el4

Comment 14 Fedora Update System 2009-09-24 12:25:36 UTC
log4cpp-1.0-4.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/log4cpp-1.0-4.el5

Comment 15 Fedora Update System 2009-10-16 19:34:34 UTC
log4cpp-1.0-4.el4 has been pushed to the Fedora EPEL 4 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 16 Fedora Update System 2009-10-16 19:34:52 UTC
log4cpp-1.0-4.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 18 Steve Traylen 2015-03-23 13:44:33 UTC
Package Change Request
======================
Package Name: log4cpp
New Branches: epel7
Owners: stevetraylen

Comment 19 Gwyn Ciesla 2015-03-23 18:31:01 UTC
Git done (by process-git-requests).


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