This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 789386 - Review Request: lilv - An LV2 Resource Description Framework Library
Review Request: lilv - An LV2 Resource Description Framework Library
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
16
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Orcan Ogetbil
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 797418
  Show dependency treegraph
 
Reported: 2012-02-10 11:42 EST by Brendan Jones
Modified: 2012-04-11 23:38 EDT (History)
3 users (show)

See Also:
Fixed In Version: lilv-0.5.0-3.fc17
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-03-23 20:43:03 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
oget.fedora: fedora‑review+
petersen: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Brendan Jones 2012-02-10 11:42:25 EST
lilv is a library to make the use of LV2 audio plugins as simple as possible 
for applications. Lilv is the successor to SLV2, rewritten to be significantly 
faster and have minimal dependencies. 

SRPM: http://bsjones.fedorapeople.org/lv2/lilv-0.5.0-1.fc16.src.rpm
SPEC: http://bsjones.fedorapeople.org/lv2/lilv.spec

rpmlint /home/bsjones/rpmbuild/SRPMS/lilv-0.5.0-1.fc16.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
Comment 1 Orcan Ogetbil 2012-02-25 22:51:21 EST
I started reviewing this one. Here are my initial findings:

! The specfile in th link above does not match the specfile that comes inside the SRPM. I started the review with the latter.

* Missing BR: lv2core-devel sord-devel (You BR sord instead, which is insufficient). I suggest using mock which will tell you the BRs you need to add.

* The directory /etc/bash_completion.d/ remains unowned. We can either own this directory, or add Requires: bash_completion. But looking at the multiple owners of the directory /etc/bash_completion.d/ I can say that we should also own this directory.

* You can pass "--bindings" to "./waf configure" to build the Python bindings. Note that you will then also need BR: swig python-devel (Just BR: python is not sufficient)

* Fedora specific compilation flags are not honored. You will need to export CFLAGS instead, as the source is written in C.

! Since you build the unit test via --test, you should probably run the test in %check.

* %{_mandir}/man1/* should go into the main package, as these are manpages for stuff that goes to /usr/bin.

* License tag should be MIT.

! Buildroot tag is no longer required. Does no harm though.
Comment 2 Brendan Jones 2012-02-27 00:27:30 EST
Thanks for the help Orcan.

I've addressed all of your points. The tests are currently failing - an error in the test suite that has been addressed upstream but is commented out for now.

SRPM: http://bsjones.fedorapeople.org/lv2/lilv-0.5.0-2.fc16.src.rpm
SPEC: http://bsjones.fedorapeople.org/lv2/lilv.spec
Comment 3 Orcan Ogetbil 2012-02-28 22:04:17 EST
Thank you for the update. Here is the rest of the review:

* Missing BR: swig

! BR: python is redundant. Does no harm though.

* BR: boost-devel and glib2-devel are redundant. It would be good to remove them so that we don't have to drag in unrelated packages during a build.

? Could you backport the fixes from the trunk and run the tests?

* I am not sure we need to divide the Python modules in two subpackages. At the very least the noarch package should require the arch specific one, since lilv.py has
   import _lilv
referring to _lilv.so. Can _lilv.so be used without lilv.py? If not, they should probably go to the same package (feel free to disagree :)).

- rpmlint says:
   lilv.x86_64: W: non-conffile-in-etc /etc/bash_completion.d/lilv
   lilv.x86_64: W: no-manual-page-for-binary lilv-bench
   python-lilv.noarch: W: no-documentation
These can be ignored.

* /usr/share/man/man3/* should go to the devel package as this is development documentation.
Comment 4 Brendan Jones 2012-03-03 17:21:59 EST
Thanks Orcan, Addressed all your comments and worked with upstream to bring the test suite up to date.

(Sure its possible to use the python lib outside the bindings but I can't see why you'd want to - merged the packages)

SRPM: http://bsjones.fedorapeople.org/lv2/lilv-0.5.0-2.fc16.src.rpm
SPEC: http://bsjones.fedorapeople.org/lv2/lilv.spec

Scratch: http://koji.fedoraproject.org/koji/taskinfo?taskID=3849804
Comment 5 Brendan Jones 2012-03-03 17:22:52 EST
Disregard SRPM link in previous post, corrected here:

SRPM: http://bsjones.fedorapeople.org/lv2/lilv-0.5.0-3.fc16.src.rpm
SPEC: http://bsjones.fedorapeople.org/lv2/lilv.spec
Comment 6 Orcan Ogetbil 2012-03-04 13:14:15 EST
This is good now. Thanks for packaging this.

---------------------------------------
This package (lilv) is APPROVED by oget
---------------------------------------
Comment 7 Brendan Jones 2012-03-05 01:41:41 EST
Thank you for the review

New Package SCM Request
=======================
Package Name: lilv
Short Description: An LV2 Resource Description Framework Library
Owners: bsjones
Branches: f15 f16 f17
InitialCC:
Comment 8 Jens Petersen 2012-03-06 02:19:16 EST
Git done (by process-git-requests).
Comment 9 Fedora Update System 2012-03-06 06:25:29 EST
lilv-0.5.0-3.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/lilv-0.5.0-3.fc16
Comment 10 Fedora Update System 2012-03-06 06:26:01 EST
lilv-0.5.0-3.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/lilv-0.5.0-3.fc17
Comment 11 Fedora Update System 2012-03-07 02:20:29 EST
Package lilv-0.5.0-3.fc17:
* should fix your issue,
* was pushed to the Fedora 17 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing lilv-0.5.0-3.fc17'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-3143/lilv-0.5.0-3.fc17
then log in and leave karma (feedback).
Comment 12 Fedora Update System 2012-03-23 20:43:03 EDT
lilv-0.5.0-3.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 13 Fedora Update System 2012-04-11 23:38:39 EDT
lilv-0.5.0-3.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.

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