Bug 541535

> rpmlint is silent.

Okay, lemme add some noise then. ;)


> # There are some MIT files but the effective license is GPLv2+
> License:	GPLv2+

The comment is confusing. What files do you refer to?

In case any source files applied a license other than GPLv2+, the guidelines would want you to make that clear.

http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Mixed_Source_Licensing_Scenario

All of the source files contain a GPLv2+ header, though. Only some autotools' scripts/files contain other headers, but we don't give them special treatment with regard to the licensing guidelines.


> raul-gcc44.patch
> ...
> +#include <stdio.h>

In C++ the proper header is <cstdio> though.


> %check
> pushd tests
>         export LD_PRELOAD=../src/.libs/lib%{name}.so

IMO, better would be this set-up:

export LD_LIBRARY_PATH=${RPM_BUILD_ROOT}%{_libdir}


> rpm -i /home/qa/tmp/rpm/RPMS/raul-devel-0.5.1-1.fc12.i686.rpm  \
> /home/qa/tmp/rpm/RPMS/raul-0.5.1-1.fc12.i686.rpm
> error: Failed dependencies:
>         liblo-devel is needed by raul-devel-0.5.1-1.fc12.i686

Uh, it requires another -devel package that wasn't needed for building it.


> $ rpm -qR raul-devel|grep devel
> boost-devel  
> glib2-devel  
> jack-audio-connection-kit-devel  
> liblo-devel  

> $ pkg-config --cflags raul
> -pthread -I/usr/include/glibmm-2.4 -I/usr/lib/glibmm-2.4/include
> -I/usr/include/sigc++-2.0 -I/usr/lib/sigc++-2.0/include
> -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include  
> $ pkg-config --libs raul
> -pthread -lraul -lglibmm-2.4 -lgobject-2.0 -lsigc-2.0 -lgthread-2.0
> -lglib-2.0 -ljack -lpthread -lrt 

"Requires: glibmm24-devel libsigc++20-devel" is missing in raul-devel. Only because the pkg-config file adds them explicitly.

Upstream might add proper "Requires" to raul.pc.in, in particular since some of these explicitly added libraries are not needed when building with libraul.


> $ grep mm include/raul/*
> AtomRDF.hpp:#include <redlandmm/Node.hpp>
> AtomRDF.hpp:#include <redlandmm/World.hpp>
> Command.hpp:#include <raul/Semaphore.hpp>
> Command.hpp:#include <boost/utility.hpp>
> Stateful.hpp:#include <redlandmm/Model.hpp>

$ sudo repoquery --whatprovides /usr/include\*/redlandmm/Node.hpp
$

Not in Fedora yet.

(In reply to comment #1)
> > rpmlint is silent.
> 
> Okay, lemme add some noise then. ;)
> 
> 

Thanks for getting your hands dirty :)

> > # There are some MIT files but the effective license is GPLv2+
> > License:	GPLv2+
> 
> The comment is confusing. What files do you refer to?
> 
> In case any source files applied a license other than GPLv2+, the guidelines
> would want you to make that clear.
> 
> http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Mixed_Source_Licensing_Scenario
> 
> All of the source files contain a GPLv2+ header, though. Only some autotools'
> scripts/files contain other headers, but we don't give them special treatment
> with regard to the licensing guidelines.
> 
> 

Ah, I probably was going thru the source files and saw the MIT headers in the autotools files and didn't pay attention what they actually are for. I'll remove the comment.

> > raul-gcc44.patch
> > ...
> > +#include <stdio.h>
> 
> In C++ the proper header is <cstdio> though.
> 

Yes. But it's not too big of a deal (Is it?). And upstream accepted and applied my <stdio.h> patch to the trunk.

> 
> > %check
> > pushd tests
> >         export LD_PRELOAD=../src/.libs/lib%{name}.so
> 
> IMO, better would be this set-up:
> 
> export LD_LIBRARY_PATH=${RPM_BUILD_ROOT}%{_libdir}
> 

Could you tell me what makes this better? Don't they serve the same purpose in this case? Does LD_PRELOAD have a potential of hiding errors or breaking thing?

> 
> > rpm -i /home/qa/tmp/rpm/RPMS/raul-devel-0.5.1-1.fc12.i686.rpm  \
> > /home/qa/tmp/rpm/RPMS/raul-0.5.1-1.fc12.i686.rpm
> > error: Failed dependencies:
> >         liblo-devel is needed by raul-devel-0.5.1-1.fc12.i686
> 
> Uh, it requires another -devel package that wasn't needed for building it.
> 

A "Requires" in the devel package does not necessarily mean that you need that package during building. Just check the header files that go into the devel package and you will understand what I mean :). You will see that some #include headers from liblo and some #include headers from:

> > boost-devel  
> > glib2-devel  
> > jack-audio-connection-kit-devel  
> > liblo-devel  
> 

So these requirements are for development purposes.

> > $ pkg-config --cflags raul
> > -pthread -I/usr/include/glibmm-2.4 -I/usr/lib/glibmm-2.4/include
> > -I/usr/include/sigc++-2.0 -I/usr/lib/sigc++-2.0/include
> > -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include  
> > $ pkg-config --libs raul
> > -pthread -lraul -lglibmm-2.4 -lgobject-2.0 -lsigc-2.0 -lgthread-2.0
> > -lglib-2.0 -ljack -lpthread -lrt 
> 
> "Requires: glibmm24-devel libsigc++20-devel" is missing in raul-devel. Only
> because the pkg-config file adds them explicitly.
> 
> Upstream might add proper "Requires" to raul.pc.in, in particular since some of
> these explicitly added libraries are not needed when building with libraul.
> 

Should I remove these entries from the .pc file:
-lglibmm-2.4 -lgobject-2.0 -lsigc-2.0 lgthread-2.0 -lglib-2.0 -ljack
I don't think they are really required.

> 
> > $ grep mm include/raul/*
> > AtomRDF.hpp:#include <redlandmm/Node.hpp>
> > AtomRDF.hpp:#include <redlandmm/World.hpp>
> > Command.hpp:#include <raul/Semaphore.hpp>
> > Command.hpp:#include <boost/utility.hpp>
> > Stateful.hpp:#include <redlandmm/Model.hpp>
> 
> $ sudo repoquery --whatprovides /usr/include\*/redlandmm/Node.hpp
> $
> 
> Not in Fedora yet.  

Exactly. That's why I didn't add BR: redlandmm-devel (or whatever it is called) to the Requires of the devel package. I will add it once this package is in Fedora. For the time being this won't break anything. I don't know of any software that uses redlandmm feature of raul. In particular, redlandmm needs redland >= 1.0.8 or higher. But even in rawhide we still have 1.0.7. I talked to the maintainer and got the response that it is being worked on. I guess the progress is a little slow.

> LD_LIBRARY_PATH vs. LD_PRELOAD

The benefit would be that you would test run-time linking of the test-suite programs with the actual shared libs installed into the package buildroot. It's much more a real-world test scenario than forcefully making available a .so that won't even be put into the run-time library package.


> A "Requires" in the devel package does not necessarily mean
> that you need that package during building.

I found it surprising enough to mention it. :-)  Upstream could include a check for liblo in its build framework. Afterall, it's a requirement of the API.


> Should I remove these entries from the .pc file:

Raises the question whether you would like the increased maintenance requirements?

[pkg-config is kind of unflexible in that area. For any inter-dependency on other .pc files that is added in a .pc file's "Requires" line, it propagates and accumulates the cflags and ldflags. Good for the cflags (to find headers in customised trees outside standard search paths). However, when linking shared-only, we don't need to relink against shared libs a base library is linked with already (as opposed to static linkage). So, as an affect, shared library ldflags pile up even if an API doesn't depend on any of the libs.]
And in this case the author even specified all those cflags and ldflags manually instead of using dependencies.


> I don't know of any software that uses redlandmm feature of raul.

The cleaner work-around would be to %exclude those headers then and effectively disable that part of the API that could not be used. We can't ship an API that is defunctional because of missing headers.

I asked these questions to the author via email. Let us wait, then I will proceed with what comes out.

The author replied and actually released a new version today, with the excessive linkage removed. So here we go:

Spec URL: http://oget.fedorapeople.org/review/raul.spec
SRPM URL: http://oget.fedorapeople.org/review/raul-0.6.0-1.fc12.src.rpm

ChangeLog - 0.6.0-1
- Update to 0.6.0. Build system uses waf now.
- Drop upstreamed gcc44 patch
- Removed irrelevant license comment
- Change LD_PRELOAD to LD_LIBRARY_PATH in %%check
- Exclude the headers that require redlandmm

Something is wrong with the new -debuginfo package now. It's missing the sources.

That doesn't happen here. Koji's build has the source files in debuginfo package too:
   http://koji.fedoraproject.org/koji/taskinfo?taskID=1939511

How did you build the package?

* A plain rpmbuild shell function on a F-12 update-testing machine that only adds stdin/stderr output logging. Somehow buildroot/builddir paths appeared in the -debuginfo below /usr/src/debug. Though, I cannot reproduce it anymore. :-) 

* There is a mixture of $RPM_BUILD_ROOT and %buildroot in the spec.
https://fedoraproject.org/wiki/Packaging/Guidelines#macros

* x86_64 build hardcodes /usr/lib in raul.pc

* raul.pc is still somewhat "unclean" beyond that, which bears a risk that you will have fun with it in the future: It hardcodes header/library search paths instead of setting a dependency on "gthread-2.0" and "glib-2.0". It relinks with gthread+glib2+rt, which libraul is linked with already.


* If you want me to give another update a look, feel free to request that.

If you think you can deal with those items in cvs: APPROVED

Thanks Michael, I'll do the trivial fixes. About linkage in raul.pc, I already informed upstream about it. I'll go with their decision. I am sure they will update those paths accordingly in time, or the software will get obsolete anyway.

New Package CVS Request
=======================
Package Name: raul
Short Description: Realtime Audio Utility Library
Owners: oget
Branches: F-11 F-12
InitialCC:

CVS done (by process-cvs-requests.py).

raul-0.6.0-2.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/raul-0.6.0-2.fc12

raul-0.6.0-2.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/raul-0.6.0-2.fc11

raul-0.6.0-2.fc12 has been pushed to the Fedora 12 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 raul'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F12/FEDORA-2010-1378

raul-0.6.0-2.fc11 has been pushed to the Fedora 11 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 raul'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2010-1396

raul-0.6.0-2.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

raul-0.6.0-2.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.