Bug 470914

Summary: Review Request: slv2 - An LV2 host library
Product: [Fedora] Fedora Reporter: Anthony Green <green>
Component: Package ReviewAssignee: Michael Schwendt <bugs.michael>
Status: CLOSED DUPLICATE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: bugs.michael, fedora-package-review, notting, oget.fedora
Target Milestone: ---Flags: bugs.michael: fedora‑review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-03-26 13:59:55 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On: 470913    
Bug Blocks: 201449    

Description Anthony Green 2008-11-10 15:48:38 EST
Spec URL: http://people.redhat.com/green/Fedora/slv2.spec
SRPM URL: http://people.redhat.com/green/Fedora/slv2-0.6.0-1.fc10.src.rpm
Description: 
SLV2 is a library for LV2 hosts intended to make using LV2 Plugins as simple
as possible (without sacrificing capabilities).


Ardour and other Fedora packages will BuildRequire this once it has been approved.  This is the second time I've submitted it.  I dropped the ball the first time around.  It depends on my recent lv2core submission.
Comment 1 Michael Schwendt 2009-01-03 06:59:33 EST
> License:        LGPLv2+

Web page says "Licensed under the GPL v2 or later for now".
File COPYING contains the GPL v2.
Only a few source files contain a LGPL header.
This suggests the project is:

 => License: GPLv2+


> Summary: An LV2 host library

Suggest dropping the "An ".


> %description    devel
> slv2-devel contains the headers and development libraries for slv2.

Suggest
"This package contains the headers and development libraries for SLV2."
for consistency and to avoid repeating the pkg name.


> %files
> %doc AUTHORS COPYING README
> %defattr(-,root,root,-)

%defattr ought to be moved one line up.


> %{_libdir}/*.a

https://fedoraproject.org/wiki/Packaging:Guidelines#Exclusion_of_Static_Libraries


* The slv2.pc pkg-config file adds a redundant -L/usr/lib -lrdf from redland.pc


* 0.6.2 is available (still marked unstable, though)


* Run-time warning (in src/world.c) about Redland librdf not being new enough:

  $ lv2_list 
  Warning: Unable to create "trees" RDF storage.
  Performance can be improved by upgrading librdf.


* src/world.c contains hardcoded /usr/lib and /usr/local/lib paths
also on 64-bit platforms!


* The only real blockers:
  * licence
  * static lib
  * hardcoded lib paths
Comment 2 Anthony Green 2009-01-07 11:50:20 EST
Thanks for taking the time to review this.

(In reply to comment #1)
> > License:        LGPLv2+
> 
> Web page says "Licensed under the GPL v2 or later for now".
> File COPYING contains the GPL v2.
> Only a few source files contain a LGPL header.
> This suggests the project is:
> 
>  => License: GPLv2+


Fixed.

> 
> 
> > Summary: An LV2 host library
> 
> Suggest dropping the "An ".
> 

Fixed.

> 
> > %description    devel
> > slv2-devel contains the headers and development libraries for slv2.
> 
> Suggest
> "This package contains the headers and development libraries for SLV2."
> for consistency and to avoid repeating the pkg name.
> 

Fixed.

> 
> > %files
> > %doc AUTHORS COPYING README
> > %defattr(-,root,root,-)
> 
> %defattr ought to be moved one line up.
> 

Fixed.


> 
> > %{_libdir}/*.a
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#Exclusion_of_Static_Libraries
> 
> 

Fixed.


> * The slv2.pc pkg-config file adds a redundant -L/usr/lib -lrdf from redland.pc
> 

Not fixed yet.


> 
> * 0.6.2 is available (still marked unstable, though)
> 

I haven't updated version.


> 
> * Run-time warning (in src/world.c) about Redland librdf not being new enough:
> 
>   $ lv2_list 
>   Warning: Unable to create "trees" RDF storage.
>   Performance can be improved by upgrading librdf.
> 

I'll look at librdf.


> 
> * src/world.c contains hardcoded /usr/lib and /usr/local/lib paths
> also on 64-bit platforms!
> 

Fixed with perl in %prep section.


> 
> * The only real blockers:
>   * licence
>   * static lib
>   * hardcoded lib paths


Thanks.  New versions here:
Spec URL: http://people.redhat.com/green/Fedora/slv2.spec
SRPM URL: http://people.redhat.com/green/Fedora/slv2-0.6.0-2.fc10.src.rpm
Comment 3 Michael Schwendt 2009-01-08 07:37:32 EST
> perl in %prep section.

Works with this release, but is fragile. It bears the risk of replacing /usr/lib64 with /usr/lib6464 in future upgrades, for example. You could avoid that by handling the slash after /usr/lib and /usr/local appropriately in the match and replacement (if not using a more complex sed/perl expr).

Typically, a patch (and optionally a guard in %prep) is safer.



* slv2-devel is missing

  Requires: lv2core-devel redland-devel

for include statements in lv2_ui.h and world.h
That's easy to add in pkg cvs, though.


* APPROVED
Comment 4 Orcan Ogetbil 2009-03-16 00:03:53 EDT
ping? no cvs request is made for the last 2 months. Is there a problem? Can I co-maintain this package?
Comment 5 Orcan Ogetbil 2009-03-16 00:46:37 EDT
A response needs to be made according to [1] within a week. Otherwise, the bug needs to be closed and a new one will be opened.

[1] https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
Comment 6 Michael Schwendt 2009-03-16 05:56:29 EDT
All http://people.redhat.com/green URLs give 404 Not Found
Comment 7 Orcan Ogetbil 2009-03-21 03:21:40 EDT
Please backport this patch from the upstream trunk:
   http://dev.drobilla.net/changeset/1948
from
   http://dev.drobilla.net/ticket/341

The compilation of slv2 does not fail without it but some plugins which need slv2 as a dependency need this patch, otherwise they won't compile.
Comment 8 Orcan Ogetbil 2009-03-23 02:10:48 EDT
Let's wait a couple days more. We'll close the bug if we don't get any response and then I'll open a new one.

Michael, would you like to re-review this package in case we don't get a reply? And could you attach or email me the SPEC file if you still have it in your archives? Thanks!
Comment 10 Orcan Ogetbil 2009-03-26 13:59:55 EDT
Closing the bug. 

I opened a new one at bug 492398.

*** This bug has been marked as a duplicate of bug 491421 ***
Comment 11 Orcan Ogetbil 2009-03-26 14:01:51 EDT
Sorry put the wrong number. The correct one is this one:

*** This bug has been marked as a duplicate of bug 492398 ***