Bug 798998 - Review Request: libcdr - a library for import of Corel Draw drawings
Review Request: libcdr - a library for import of Corel Draw drawings
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michael Schwendt
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-01 08:59 EST by David Tardon
Modified: 2012-03-22 08:34 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-03-22 02:08:16 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
bugs.michael: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description David Tardon 2012-03-01 08:59:55 EST
Spec URL: http://dtardon.fedorapeople.org/libcdr.spec
SRPM URL: http://dtardon.fedorapeople.org/libcdr-0.0.3-1.fc16.src.rpm
Description:

Libcdr is library providing ability to interpret and import Corel Draw drawings into various applications.

It will be a dependency of libreoffice 3.6 .
Comment 1 Michael Schwendt 2012-03-10 06:00:00 EST
> %package doc
> Summary: Documentation of %{name} API
> Group: Documentation
> Requires: %{name} = %{version}-%{release}

What in the -doc package requires the base library package to be _installed_? Nothing. So, please keep Documentation packages free from dependency bloat.
Comment 2 David Tardon 2012-03-10 08:47:49 EST
Right. I corrected that.

Spec URL: http://dtardon.fedorapeople.org/libcdr.spec
SRPM URL: http://dtardon.fedorapeople.org/libcdr-0.0.3-2.fc16.src.rpm
Comment 3 Michael Schwendt 2012-03-11 12:11:54 EDT
Thanks.  Meanwhile, I've had a look at things other than just the spec file:


* The licensing is not clear yet. Spec says:

> License: GPL+ or LGPLv2+ or MPLv1.1

That sounds like MPL tri-license, but the file contents disagree:

1) COPYING.GPL is the GPLv2 not GPL+

2) src/conv/raw/cdr2raw.cpp is LGPLv2+ only (!) and linking with libcdr

3) src/conv/svg/cdr2xhtml.cpp : MPLv1.1 tri-license header

4) src/lib/* :
   
   some files explicitly mention "MPL 1.1 / GPLv2+ / LGPLv2+",
   which is the tri-license

   CDRDocument.h : LGPLv2+ only (!)
   libcdr.h : LGPLv2+ only (!)

   All others contain the MPLv1.1 tri-license header with explicit GPLv2+/LGPLv2+ option at the bottom.
  

The two files with LGPLv2+ header belong to the libcdr API,

  $ rpmls -p libcdr-devel-0.0.3-2.fc17.x86_64.rpm|grep \.h
  -rw-r--r--  /usr/include/libcdr-0.0/libcdr/CDRDocument.h
  -rw-r--r--  /usr/include/libcdr-0.0/libcdr/CDRStringVector.h
  -rw-r--r--  /usr/include/libcdr-0.0/libcdr/libcdr.h

and /usr/bin/cdr2raw is in the separate -tools package.

There is no statement that those files are dual-/multi-licensed in any way. That makes the library LGPLv2+ licensed IMO. License clarification by the developers would be helpful.


* I assume you are aware of these few items which are not needed in spec files anymore:

https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag
https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions
https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean


* There's an old packaging trick to ship subpackage documentation in the subpackage's own versioned docdir. Currently:

$ rpmls -p libcdr-doc-0.0.3-2.fc17.noarch.rpm | grep ^d
drwxr-xr-x  /usr/share/doc/libcdr
drwxr-xr-x  /usr/share/doc/libcdr-doc-0.0.3
drwxr-xr-x  /usr/share/doc/libcdr/html

--- libcdr.spec.ORIG	2012-03-11 16:23:18.395893248 +0100
+++ libcdr.spec	2012-03-11 17:04:52.849667449 +0100
@@ -63,6 +63,7 @@
 rm -rf %{buildroot}
 make install DESTDIR=%{buildroot}
 rm -f %{buildroot}/%{_libdir}/*.la
+rm -rf _tmpdoc && mkdir _tmpdoc && mv %{buildroot}%{_docdir}/%{name}/html _tmpdoc
 
 
 %clean
@@ -90,9 +91,7 @@
 
 %files doc
 %defattr(-,root,root,-)
-%doc COPYING.*
-%dir %{_docdir}/%{name}
-%{_docdir}/%{name}/html
+%doc _tmpdoc/html COPYING.*
 
 
 %files tools
Comment 4 David Tardon 2012-03-13 06:41:14 EDT
(In reply to comment #3)
> Thanks.  Meanwhile, I've had a look at things other than just the spec file:
> 
> 
> * The licensing is not clear yet. Spec says:

I am looking into it.

> * I assume you are aware of these few items which are not needed in spec files
> anymore:
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag
> https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions
> https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean

Yes, I am. The last time I tried to make a package without them rpmlint issued warnings about it, so I just put them back and have not tried to do it ever again. If rmplint has been fixed in the meanwhile, fine, I will remove them eventually.

> * There's an old packaging trick to ship subpackage documentation in the
> subpackage's own versioned docdir.

I do not care either way...
Comment 5 David Tardon 2012-03-19 03:03:17 EDT
The licensing issues have been addressed in new upstream release.

Spec URL: http://dtardon.fedorapeople.org/libcdr.spec
SRPM URL: http://dtardon.fedorapeople.org/libcdr-0.0.5-1.fc16.src.rpm
Comment 6 Michael Schwendt 2012-03-20 07:19:41 EDT
* Release 0.0.5 fixes the licensing issues and explicitly acknowledges the tri-license in the README even.


* New file src/lib/CDRColorProfiles.h uses the compatible zlib/libpng "no acknowledgement" license.


* Please add  V=1  to the Make invocation for more verbose build output.


* The pkgconfig file is worse in release 0.0.5. It explicitly adds a dependency on libraries that are linked with libcdr already. In 0.0.5 it has added a dependency on lcms2 and zlib in "Requires" and/or "Libs". Whereas libwpd* and libwpg headers are included from within libcdr headers at least (and the pkgconfig dependency can help with getting their custom include path right), relinking with lcms2 and zlib would not be necessary.


* APPROVED
Comment 7 David Tardon 2012-03-20 07:57:48 EDT
New Package SCM Request
=======================
Package Name: libcdr
Short Description: A library for import of Corel Draw drawings
Owners: dtardon
Branches: 
InitialCC: caolanm
Comment 8 Gwyn Ciesla 2012-03-20 08:51:14 EDT
Git done (by process-git-requests).
Comment 9 Gwyn Ciesla 2012-03-22 08:34:28 EDT
Not sure why the flag was re-set.

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