Bug 189313 - Review Request: liblrdf
Summary: Review Request: liblrdf
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Michael Schwendt
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Keywords:
Depends On: 189309
Blocks: FE-ACCEPT 189315 190040
TreeView+ depends on / blocked
 
Reported: 2006-04-19 02:11 UTC by Anthony Green
Modified: 2008-04-14 19:36 UTC (History)
1 user (show)

(edit)
Clone Of:
(edit)
Last Closed: 2006-05-14 03:32:29 UTC
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Anthony Green 2006-04-19 02:11:44 UTC
Spec URL: http://people.redhat.com/green/FE/FC5/liblrdf.spec
SRPM URL: http://people.redhat.com/green/FE/FC5/liblrdf-0.4.0-2.src.rpm
Description: 
This is a library to make it easy to manipulate RDF files describing
LADSPA plugins.

Ardour is dependent on this package.  The spec file is based on a old planet ccrma spec file.

Comment 1 Anthony Green 2006-04-23 11:18:57 UTC
I've just updates the bits based on recent experience and expected feedback...

Spec URL: http://people.redhat.com/green/FE/FC5/liblrdf.spec
SRPM URL: http://people.redhat.com/green/FE/FC5/liblrdf-0.4.0-3.src.rpm



Comment 2 Michael Schwendt 2006-04-23 11:41:52 UTC
* Until raptor-devel will be good, this one BuildRequires: libxslt-devel

* Run rpmlint on the binary rpms:

Lots of output, in particular because the included "examples" %doc
directory contains arch-dependent files (it MUST NOT), including
unfinished libtool based executables in the hidden .libs directory,
object files, and dependency meta data files in the hidden .deps
directory.

* rpmqfcheck.pl /home/qa/tmp/rpm/RPMS/liblrdf-0.4.0-3.i386.rpm
Orphaned dir: /usr/share/ladspa
Orphaned dir: /usr/share/ladspa/rdf

* Source0 would be directly downloadable if in the form:
http://download.sourceforge.net/...
or
http://dl.sf.net/...

* Static libraries should not be included.

* Noticable compiler warnings:

showdefaults.c:42: warning: format '%d' expects type 'int', but argument 3 has
type 'long unsigned int'
setting_test.c:43: warning: format '%d' expects type 'int', but argument 3 has
type 'long unsigned int'
lrdf.c:596: warning: pointer targets in passing argument 1 of 'raptor_new_uri'
differ in signedness
lrdf.c:597: warning: pointer targets in passing argument 1 of 'raptor_new_uri'
differ in signedness


Comment 3 Anthony Green 2006-04-23 12:31:24 UTC
Thanks for looking at this.  Updated bits here:

Spec URL: http://people.redhat.com/green/FE/FC5/liblrdf.spec
SRPM URL: http://people.redhat.com/green/FE/FC5/liblrdf-0.4.0-4.src.rpm


(In reply to comment #2)
> * Until raptor-devel will be good, this one BuildRequires: libxslt-devel

I've submitted a fixed raptor spec file.

> * Run rpmlint on the binary rpms:
> 
> Lots of output, in particular because the included "examples" %doc
> directory contains arch-dependent files (it MUST NOT), including
> unfinished libtool based executables in the hidden .libs directory,
> object files, and dependency meta data files in the hidden .deps
> directory.

I've removed examples from the doc list, and added a README.fedora file to
point people at the SRPM for example source code.

> * rpmqfcheck.pl /home/qa/tmp/rpm/RPMS/liblrdf-0.4.0-3.i386.rpm
> Orphaned dir: /usr/share/ladspa
> Orphaned dir: /usr/share/ladspa/rdf

I'm not sure who should own these directories.  Perhaps this package should own
/usr/share/ladspa/rdf, and ladspa could own /usr/share/ladspa - although there's
no real need to install the ladspa package when using ladspa plugins.  Suggestions?

> * Source0 would be directly downloadable if in the form:
> http://download.sourceforge.net/...
> or
> http://dl.sf.net/...

Fixed.
 
> * Static libraries should not be included.

Fixed.  Configured with --disable-static.
 
> * Noticable compiler warnings:
> 
> showdefaults.c:42: warning: format '%d' expects type 'int', but argument 3 has
> type 'long unsigned int'
> setting_test.c:43: warning: format '%d' expects type 'int', but argument 3 has
> type 'long unsigned int'

This is from the example directory, which is ignored now.

> lrdf.c:596: warning: pointer targets in passing argument 1 of 'raptor_new_uri'
> differ in signedness
> lrdf.c:597: warning: pointer targets in passing argument 1 of 'raptor_new_uri'
> differ in signedness

I will push this upstream rather than try to handle it here.  It's a
signed-vs-unsigned char thing.






Comment 4 Anthony Green 2006-04-24 11:10:19 UTC
I've cleaned up the spec file based raptor changes.  Updated bits here...

Spec URL: http://people.redhat.com/green/FE/FC5/liblrdf.spec
SRPM URL: http://people.redhat.com/green/FE/FC5/liblrdf-0.4.0-5.src.rpm



Comment 5 Michael Schwendt 2006-04-26 22:52:25 UTC
> Perhaps this package should own
> /usr/share/ladspa/rdf, and ladspa could own /usr/share/ladspa 

Common practice among packagers seems to be to _either_ require
a package which owns the directories _or_ to own the directories
yourself.

Here, however, the file is installed from within the "examples"
directory for rdf_uri load tests and is not required at run-time.
At build-time it is not adjusted to be in sync with the plugins
found in the "ladspa" package. In liblrdf there's also no run-time
dependency on the LADSPA_RDF_PATH /usr/share/ladspa/rdf


Comment 6 Michael Schwendt 2006-05-13 21:27:15 UTC
Confirming bottom of comment 5. /usr/share/ladspa/rdf/ladspa.rdfs
is a useless example for a classification of ladspa plugins. Inside
Hydrogen it is recognised without any of the classified ladspa
plugins being available. The file should not be included in the
package. (Do we have any ladspa package in the queue which comes
with an rdf file?)

APPROVED if you fix that remaining issue.


Comment 7 Anthony Green 2006-05-14 03:32:29 UTC
(In reply to comment #6)
> (Do we have any ladspa package in the queue which comes
> with an rdf file?)

swh-plugins does, and it's already in Extras.
I've also submitted CAPS, which does as well.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190045
 
> APPROVED if you fix that remaining issue.

Done.  Thanks!


Comment 8 Lubomir Kundrak 2008-04-11 21:07:54 UTC
I'd be very thiankful if you could request and maintain a EPEL-5 branch for this
package. In case you don't want or can't do that, let me know and I'll do that.

Thanks!


Comment 9 Lubomir Kundrak 2008-04-14 17:10:57 UTC
Maintainer is OK with the change as per previous conversation with him.

Package Change Request
======================
Package Name: liblrdf
New Branches: EL-5
Owners for new branch: green,lkundrak
cvsextras commits for new branch: yes

Comment 10 Kevin Fenzi 2008-04-14 19:36:05 UTC
cvs done.


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