[Bug 541535] Review Request: raul - Realtime Audio Utility Library

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=541535





--- Comment #2 from Orcan 'oget' Ogetbil <oget.fedora at gmail.com>  2009-12-05 16:09:21 EDT ---
(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.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.