Bug 189313 - Review Request: liblrdf
Review Request: liblrdf
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michael Schwendt
Fedora Package Reviews List
:
Depends On: 189309
Blocks: FE-ACCEPT 189315 190040
  Show dependency treegraph
 
Reported: 2006-04-18 22:11 EDT by Anthony Green
Modified: 2008-04-14 15:36 EDT (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-05-13 23:32:29 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Anthony Green 2006-04-18 22:11:44 EDT
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 07:18:57 EDT
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 07:41:52 EDT
* 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 08:31:24 EDT
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 07:10:19 EDT
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 18:52:25 EDT
> 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 17:27:15 EDT
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-13 23:32:29 EDT
(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 17:07:54 EDT
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 13:10:57 EDT
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 15:36:05 EDT
cvs done.

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