Bug 476247 - Review Request: log4cpp - logging library for c++
Review Request: log4cpp - logging library for c++
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: David Cantrell
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-12-12 12:34 EST by jmccann
Modified: 2015-03-23 14:31 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-02-05 19:11:03 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
jkeating: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description jmccann 2008-12-12 12:34:47 EST
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 12:38:57 EST
Taking review.
Comment 2 Jesse Keating 2008-12-12 14:21:55 EST
* 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 16:14:27 EST
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-12 19:13:33 EST
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-12 21:35:56 EST
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-12 23:19:18 EST
Updated SRPM and spec for pkgconfig change.
Comment 7 Jesse Keating 2008-12-13 14:39:08 EST
All right, looks good now.  Approving.
Comment 8 jmccann 2008-12-15 13:57:52 EST
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 15:12:44 EST
CVS Done
Comment 10 Steve Traylen 2009-09-23 09:55:10 EDT

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 17:40:43 EDT
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 00:39:14 EDT
cvs done.
Comment 13 Fedora Update System 2009-09-24 08:23:59 EDT
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 08:25:36 EDT
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 15:34:34 EDT
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 15:34:52 EDT
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 09:44:33 EDT
Package Change Request
======================
Package Name: log4cpp
New Branches: epel7
Owners: stevetraylen
Comment 19 Gwyn Ciesla 2015-03-23 14:31:01 EDT
Git done (by process-git-requests).

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