Bug 659082 - Review Request: redland-bindings - language bindings for redland
Summary: Review Request: redland-bindings - language bindings for redland
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Orcan Ogetbil
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-12-01 21:18 UTC by Thomas Vander Stichele
Modified: 2011-03-20 18:52 UTC (History)
3 users (show)

Fixed In Version: redland-bindings-1.0.7.1-1.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-03-14 05:39:56 UTC
Type: ---
Embargoed:
oget.fedora: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Thomas Vander Stichele 2010-12-01 21:18:37 UTC
Spec URL: http://thomas.apestaart.org/download/pkg/fedora-14-i386-updates/redland-bindings-1.0.11.1-2.fc14/redland-bindings.spec
SRPM URL: http://thomas.apestaart.org/download/pkg/fedora-14-i386-updates/redland-bindings-1.0.11.1-2.fc14/redland-bindings-1.0.11.1-2.fc14.src.rpm
Description:

Redland is a library that provides a high-level interface for RDF
(Resource Description Framework) implemented in an object-based API.
It is modular and supports different RDF/XML parsers, storage
mechanisms and other elements. Redland is designed for applications
developers to provide RDF support in their applications as well as
for RDF developers to experiment with the technology.

This package provides Python, Perl, Ruby and PHP bindings for redland.

Comment 1 Orcan Ogetbil 2011-01-05 02:48:20 UTC
Hi Thomas, here are my findings from my review. It needs a little work:

* "rpmbuild -bs redland-bindings" fails with 
   $ rpmbuild -bs redland-bindings.spec 
   sh: php-config: command not found
   error: Macro %php_extdir has empty body
   sh: /usr/bin/ruby: No such file or directory
   sh: /usr/bin/ruby: No such file or directory

Please fix these lines by using the macros as defined in
   http://fedoraproject.org/wiki/Packaging:PHP#Other_Modules
   http://fedoraproject.org/wiki/Packaging:Ruby#Ruby_packages_with_binary_content.2Fshared_libraries
Otherwise these might cause an issue in koji.

* Patches should be explained and be submitted to upstream. What is their status?

* rpmlint says:
   python-redland.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/Redland.so Redland.so()(64bit)
   perl-redland.x86_64: W: private-shared-object-provides /usr/lib64/perl5/auto/RDF/Redland/CORE/CORE.so CORE.so()(64bit)
   perl-redland.x86_64: W: spurious-executable-perm /usr/share/doc/perl-redland-1.0.11.1/example.pl
   perl-redland.x86_64: W: doc-file-dependency /usr/share/doc/perl-redland-1.0.11.1/example.pl /usr/bin/perl
   php-redland.x86_64: W: unstripped-binary-or-object /usr/lib64/php/modules/redland.so
   php-redland.x86_64: W: spurious-executable-perm /usr/share/doc/php-redland-1.0.11.1/example.pl
   php-redland.x86_64: W: doc-file-dependency /usr/share/doc/php-redland-1.0.11.1/example.pl perl(RDF::Redland)
   php-redland.x86_64: W: doc-file-dependency /usr/share/doc/php-redland-1.0.11.1/example.pl /usr/bin/perl
As far as I can tell, these all need to be fixed.

* We prefer %global over %defined

! The .html files in the root directory of the tarball could be packaged (except INSTALL.html)

* There are many files under perl/ and php/ directories, which have the license text:
   # This package is Free Software or Open Source available under the                                                   
   # following licenses (these are alternatives):                                                                       
   #   1. GNU Lesser General Public License (LGPL)                                                                      
   #   2. GNU General Public License (GPL)                                                                              
   #   3. Mozilla Public License (MPL)
These do not state any license versions. It would be good to ask upstream.

On the other hand, for example, python/RDF.py says that it is LGPLv2+ or GPLv2+ or ASL 2.0 or MIT, which makes the licensing scenario even more complicated.

Please go through all the source files and document their respective licenses.

! The files AUTHORS COPYING COPYING.LIB ChangeLog LICENSE.txt NEWS README LICENSE-2.0.txt NOTICE
are being duplicated. This is okay with license files. However I'm not sure about the other ones. Maybe put everything in the main package and make all the subpackages require it?

* The subpackages have
   Requires:       redland = %{redland_version}
which won't be satisfied with the current redland version we have in F-14+. Don't we need >=

* Parallel make must be supported whenever possible. If it is not supported, this should be noted in the SPEC file as a comment.

* The following BuildRequires seem redundant to me:
   perl php openssl-devel mysql-devel postgresql-devel sqlite-devel db4-devel

* Python guidelines are not obeyed:
   - Need to use BuildRequires: python2-devel
   - The macros do not match the guidelines.
Please see http://fedoraproject.org/wiki/Packaging:Python

* Ruby package must indicate the Ruby ABI version it depends on. Please see
   http://fedoraproject.org/wiki/Packaging:Ruby

? What is this for:
   /usr/lib/rpm/brp-python-bytecompile

Comment 2 Thomas Vander Stichele 2011-01-18 18:19:38 UTC
Thanks for the review.  Looking at it I might have submitted the up-to-F11-compatible version of my spec and I should probably do a F14-and-up-only.  Let me do that and get back to you.

Comment 3 Thomas Vander Stichele 2011-01-22 20:19:57 UTC
Hi Orcan,

I went through all your comments.  Some of them were indeed fixed in a -3, but not all.

I addressed almost all of them, except a few.  Notes on the ones that need it:

 * patches: I commented with a link to upstream bug tracker
 * rpmlint: warnings are fixed, except for
redland-bindings.x86_64: E: no-binary
I guess a docs only package is not appreciated by rpm, but doing a noarch doc package here is a little overkill IMO.
 * licensing: I'm not sure what you want me to do.  Where do you want me to document them ? I can ask upstream what the situation is, but that will probably take time.  I don't think the package itself should block on it, what do you think ? For now I added MPLv1.0 as a license.

-4 is here:

http://thomas.apestaart.org/download/pkg/fedora-14-i386-updates/redland-bindings-1.0.11.1-4.fc14/

Comment 4 Thomas Vander Stichele 2011-01-22 20:32:29 UTC
http://bugs.librdf.org/mantis/view.php?id=413 for the licensing issue.

Comment 5 Orcan Ogetbil 2011-01-28 15:46:58 UTC
Thanks for the update. It is now in much better shape. I still have a few questions though:

? Do we really need these:
   %{?!pybasever:%{expand:%%define pybasever %(%{__python} -c "import sys ; print sys.version[:3]")}}
   
   %if "%{pybasever}" == "2.3"
   Requires:       python-abi = 2.3
   %endif

Python-2.3 is a bit too old. I don't even remember what Fedora version came with it. This is not a blocker but I am curious why you have this in the specfile.

! php_extdir doesn't match the guideline
   http://fedoraproject.org/wiki/Packaging:PHP#PECL_Modules

? How about instead of
   %attr(664,root,root) %doc perl/example.pl
just
   %doc perl/example.pl

? Why is this file a ghost?
   %ghost %{python_sitearch}/RDF.pyo

! I am still not sure how to handle the common %doc files. I will send an email to the packaging list. In case we need a common package, we will probably need a versioned requires, such as
   Requires:       redland-bindings = %{version}-%{release}
in the subpackages.

Comment 6 Orcan Ogetbil 2011-01-28 16:57:36 UTC
Here is the discussion in the packaging list:
   http://lists.fedoraproject.org/pipermail/packaging/2011-January/007593.html


(In reply to comment #3)
>  * licensing: I'm not sure what you want me to do.  Where do you want me to
> document them ? I can ask upstream what the situation is, but that will
> probably take time.  I don't think the package itself should block on it, what
> do you think ? For now I added MPLv1.0 as a license.
> 

Until upstream makes things clear, you will need to go into each source file and find out its license, and indicate it as a comment in the specfile. as an example:


# python/RDF.py is LGPLv2+ or GPLv2+ or ASL 2.0 or MIT
# python/example.py is LGPLv2 or GPLv2 or MPLv1.1
# perl/* are LGPL+ or GPL+ or MPLv?  (please check this, I did not see GPL or LGPL or MPL versions in the headers)
# ... (list all other files with different licenses)
# the rest of the code is licensed with ...

%package -n perl-redland
License: LGPL+ or GPL+ or MPL

%package -n python-redland
License: (LGPLv2+ or GPLv2+ or ASL 2.0 or MIT) and (LGPLv2 or GPLv2 or MPLv1.1) and ...

etc.

Comment 7 Orcan Ogetbil 2011-01-29 13:22:25 UTC
According to the packaging list, it is best to make a -common subpackage, and put the common files in there. All the other packages will require the -common subpackage. The license of the -common subpackage should be what upstream claims as the license of their software.

Note that there are multiple license files. If a particular license file does not apply to a particular subpackage, then we can't put that license file into the -common package. (LGPLv2+ or ASL 2.0 or MPLv1.0 ?)

Comment 8 Thomas Vander Stichele 2011-02-07 19:04:43 UTC
(In reply to comment #5)
> Thanks for the update. It is now in much better shape. I still have a few
> questions though:
> 
> ? Do we really need these:
>    %{?!pybasever:%{expand:%%define pybasever %(%{__python} -c "import sys ;
> print sys.version[:3]")}}
> 
>    %if "%{pybasever}" == "2.3"
>    Requires:       python-abi = 2.3
>    %endif
> 
> Python-2.3 is a bit too old. I don't even remember what Fedora version came
> with it. This is not a blocker but I am curious why you have this in the
> specfile.

ok, removing.

> 
> ! php_extdir doesn't match the guideline
>    http://fedoraproject.org/wiki/Packaging:PHP#PECL_Modules
> 

ok, changed.


> ? How about instead of
>    %attr(664,root,root) %doc perl/example.pl
> just
>    %doc perl/example.pl


by default it is executable, so rpmlint complains.

> 
> ? Why is this file a ghost?
>    %ghost %{python_sitearch}/RDF.pyo
>

removed.
 
> ! I am still not sure how to handle the common %doc files. I will send an email
> to the packaging list. In case we need a common package, we will probably need
> a versioned requires, such as
>    Requires:       redland-bindings = %{version}-%{release}
> in the subpackages.

I don't see why it needs to be versioned ? If this package only contains docs/license info then it's not that important IMO.

FWIW rpmlint still errors about a package without any binaries.


The licensing situation is cleared up in a commit, so I adapted the license field too.  Will build new binaries.

Comment 9 Thomas Vander Stichele 2011-02-08 11:06:47 UTC
New version at
http://thomas.apestaart.org/download/pkg/fedora-14-i386-updates/redland-bindings-1.0.11.1-5.fc14/

I also took the time to integrate the commited versions of my patches because upstream commited fixes - should make it easier to update for the next release.  As a result the package now needs autotools to build, which is temporary.

I think this resolves all problems you had with the review, so I hope you can approve this one!

Comment 10 Orcan Ogetbil 2011-03-01 03:38:18 UTC
Sorry for the delay. This looks good. In the packaging list, we were advised to name the common package "%{name}-common", whereas you simply named it %{name}. This should not be an issue. Moreover, you could also apply the license clarification patch. 

However these are not blockers. I leave them up to you to keep as is or to change.

---------------------------------------------------
This package (redland-bindings) is APPROVED by oget
---------------------------------------------------

Comment 11 Thomas Vander Stichele 2011-03-04 11:30:55 UTC
rebuilt a new version doing the -common rename.

The license clarification patch I choose to wait until the next release, because extracting patches from git is painful.

Comment 12 Thomas Vander Stichele 2011-03-04 11:32:35 UTC
New Package SCM Request
=======================
Package Name: redland-bindings
Short Description: Redland RDF Application Framework API Bindings
Owners: thomasvs
Branches: f13 f14 f15 el5 el6
InitialCC: oget

Comment 13 Jason Tibbitts 2011-03-04 13:06:22 UTC
Git done (by process-git-requests).

Comment 14 Fedora Update System 2011-03-04 17:09:09 UTC
Package redland-bindings-1.0.11.1-6.fc15:
* should fix your issue,
* was pushed to the Fedora 15 updates-testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing redland-bindings-1.0.11.1-6.fc15'
as soon as you are able to, then reboot.
Please go to the following url:
https://admin.fedoraproject.org/updates/redland-bindings-1.0.11.1-6.fc15
then log in and leave karma (feedback).

Comment 15 Fedora Update System 2011-03-04 17:16:41 UTC
Package redland-bindings-1.0.11.1-6.fc14:
* should fix your issue,
* was pushed to the Fedora 14 updates-testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing redland-bindings-1.0.11.1-6.fc14'
as soon as you are able to, then reboot.
Please go to the following url:
https://admin.fedoraproject.org/updates/redland-bindings-1.0.11.1-6.fc14
then log in and leave karma (feedback).

Comment 16 Fedora Update System 2011-03-04 19:21:36 UTC
Package redland-bindings-1.0.7.1-1.el6:
* should fix your issue,
* was pushed to the Fedora EPEL 6 updates-testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing redland-bindings-1.0.7.1-1.el6'
as soon as you are able to, then reboot.
Please go to the following url:
https://admin.fedoraproject.org/updates/redland-bindings-1.0.7.1-1.el6
then log in and leave karma (feedback).

Comment 17 Fedora Update System 2011-03-04 19:43:12 UTC
Package redland-bindings-1.0.7.1-1.fc13:
* should fix your issue,
* was pushed to the Fedora 13 updates-testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing redland-bindings-1.0.7.1-1.fc13'
as soon as you are able to, then reboot.
Please go to the following url:
https://admin.fedoraproject.org/updates/redland-bindings-1.0.7.1-1.fc13
then log in and leave karma (feedback).

Comment 18 Fedora Update System 2011-03-05 02:26:26 UTC
redland-bindings-1.0.11.1-6.fc15 has been pushed to the Fedora 15 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update redland-bindings'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/redland-bindings-1.0.11.1-6.fc15

Comment 19 Fedora Update System 2011-03-14 05:39:51 UTC
redland-bindings-1.0.11.1-6.fc15 has been pushed to the Fedora 15 stable repository.

Comment 20 Fedora Update System 2011-03-14 10:21:31 UTC
redland-bindings-1.0.7.1-1.fc13 has been pushed to the Fedora 13 stable repository.

Comment 21 Fedora Update System 2011-03-14 10:22:34 UTC
redland-bindings-1.0.11.1-6.fc14 has been pushed to the Fedora 14 stable repository.

Comment 22 Fedora Update System 2011-03-20 18:52:51 UTC
redland-bindings-1.0.7.1-1.el6 has been pushed to the Fedora EPEL 6 stable repository.


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