Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: lv2-mdala-plugins - LV2 port of the MDA VST plugins
https://bugzilla.redhat.com/show_bug.cgi?id=750139
Summary: Review Request: lv2-mdala-plugins - LV2 port of the MDA VST plugins Product: Fedora Version: 16 Platform: Unspecified OS/Version: Unspecified Status: NEW Severity: unspecified Priority: unspecified Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: brendan.jones.it@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: ---
lv2-mda-plugins is a LV2 port of the MDA VST plugins
This project exists only in SVN and has done for quite some time. It is still under active development by one of the co-authors of the LV2 standard.
SPEC: http://bsjones.fedorapeople.org/lv2-mdala-plugins.spec SRPM: http://bsjones.fedorapeople.org/lv2-mdala-plugins-0-0.1.svn3580.fc16.src.rpm
rpmlint /var/lib/mock/fedora-16-x86_64/result/*.rpm lv2-mdala-plugins.src: W: spelling-error %description -l en_US phaser -> phase, phrase, phases lv2-mdala-plugins.src: W: spelling-error %description -l en_US reverb -> revere, revers, revert lv2-mdala-plugins.src: W: invalid-url Source0: lv2-mdala-plugins-0-svn3580.tar.bz2 lv2-mdala-plugins.x86_64: W: spelling-error %description -l en_US phaser -> phase, phrase, phases lv2-mdala-plugins.x86_64: W: spelling-error %description -l en_US reverb -> revere, revers, revert 3 packages and 0 specfiles checked; 0 errors, 5 warnings.
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=750139
--- Comment #1 from Brendan Jones brendan.jones.it@gmail.com 2011-11-14 23:43:18 EST --- A minor update
SPEC: http://bsjones.fedorapeople.org/lv2-mdala-plugins.spec SRPM: http://bsjones.fedorapeople.org/lv2-mdala-plugins-0-0.2.svn3580.fc16.src.rpm
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=750139
Jerry James loganjerry@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |loganjerry@gmail.com AssignedTo|nobody@fedoraproject.org |loganjerry@gmail.com Flag| |fedora-review?
--- Comment #2 from Jerry James loganjerry@gmail.com 2011-11-23 17:46:38 EST --- I'll take this review. Thanks for swapping with me. Now I get to learn what LV2, MDA, and VST stand for. :-)
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=750139
--- Comment #3 from Jerry James loganjerry@gmail.com 2011-11-23 18:14:07 EST --- Here are a few preliminary comments. First, the BuildRoot tag is not necessary on Fedora. Second, it appears that README does not need to have its line endings converted, and neither README nor COPYING need to be converted to UTF-8. Third, it looks like the source files are not being compiled with RPM_OPT_FLAGS. Is that right? If I'm reading the docs right, I think you want to configure like this:
CFLAGS="$RPM_OPT_FLAGS" ./waf configure --prefix=%{_prefix} --libdir=%{_libdir}
Fourth, I'm not familiar with waf. Is it wise to use the shipped version, instead of the version in the Fedora waf package?
+: OK -: must be fixed =: should be fixed (at your discretion) N: not applicable
MUST: [+] rpmlint output: lv2-mdala-plugins.x86_64: W: spelling-error %description -l en_US phaser -> phase, phrase, phases lv2-mdala-plugins.x86_64: W: spelling-error %description -l en_US reverb -> revere, revers, revert lv2-mdala-plugins.spec: W: invalid-url Source0: lv2-mdala-plugins-0-svn3580.tar.bz2 2 packages and 1 specfiles checked; 0 errors, 3 warnings. [+] follows package naming guidelines [+] spec file base name matches package name [=] package meets the packaging guidelines: with the possible exception of the RPM_OPT_FLAGS issue noted above [+] package uses a Fedora approved license [+] license field matches the actual license [=] license file is included in %doc: the license file actually included is the GPL v2 license. See if upstream will update that to the GPL v3 license text. [+] spec file is in American English [+] spec file is legible [+] sources match upstream [+] package builds on at least one primary arch (tried x86_64) [N] appropriate use of ExcludeArch [+] all build requirements in BuildRequires [N] spec file handles locales properly [N] ldconfig in %post and %postun [+] no bundled copies of system libraries [N] no relocatable packages [+] package owns all directories that it creates [+] no files listed twice in %files [+] proper permissions on files [+] consistent use of macros [+] code or permissible content [N] large documentation in -doc [+] no runtime dependencies in %doc [N] header files in -devel [N] static libraries in -static [N] .so in -devel [N] -devel requires main package [+] package contains no libtool archives [N] package contains a desktop file, uses desktop-file-install [+] package does not own files/dirs owned by other packages [+] all filenames in UTF-8
SHOULD: [=] query upstream for license text: see above [N] description and summary contains available translations [+] package builds in mock: tried fedora-rawhide-i386 [+] package builds on all supported arches: tried i386 and x86_64 [=] package functions as described: did not test [+] sane scriptlets [N] subpackages require the main package [N] placement of pkgconfig files [N] file dependencies versus package dependencies [N] package contains man pages for binaries/scripts
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=750139
--- Comment #4 from Brendan Jones brendan.jones.it@gmail.com 2011-11-24 03:08:16 EST --- Hi Jerry, thanks for the review!
I've removed the README form the encoding fix and dropped the buildroot tag.
RPM_OPT_FLAGS and %{optflags} are interchangeable [1]
From what I know the waf is not so concerned with backwards compatibility as
other build tools (such as cmake) and recommends shipping the waf script. There are no clear guidelines on this - the version of waf used here is actually earlier than the one in f16, so I will leave as is.
I have contacted the author with regards to the incorrect license file, stay tuned.
thanks
Brendan
[1] http://fedoraproject.org/wiki/Packaging:Guidelines#Using_.25.7Bbuildroot.7D_...
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=750139
--- Comment #5 from Jerry James loganjerry@gmail.com 2011-11-24 13:41:01 EST --- (In reply to comment #4)
RPM_OPT_FLAGS and %{optflags} are interchangeable [1]
Errr, yeah, I knew that. I don't know how I overlooked that %{optflags}. Must be time to go see the eye doctor again...
I have contacted the author with regards to the incorrect license file, stay tuned.
OK, thanks.
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=750139
--- Comment #6 from Brendan Jones brendan.jones.it@gmail.com 2011-11-27 08:48:08 EST --- The License file has been updated in SVN however the author has expressed some misgivings about packaging pre-release, so I will respect his wishes. This review can be put on hold until further notice, ( a release is imminent however)
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=750139
--- Comment #7 from Brendan Jones brendan.jones.it@gmail.com 2011-12-09 15:55:25 EST --- Hi Jerry,
this may be on hold for a while. I have this other LV@ plugin if interested bug 760270
cheers
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=750139
--- Comment #8 from Brendan Jones brendan.jones.it@gmail.com 2012-05-16 10:34:04 EDT --- OK - upstream has officially released this:
SPEC: http://bsjones.fedorapeople.org/lv2-mdala-plugins.spec SRPM: http://bsjones.fedorapeople.org/lv2-mdala-plugins-1.0.0.fc17.src.rpm
ta
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=750139
--- Comment #9 from Jerry James loganjerry@gmail.com 2012-05-16 15:10:28 EDT --- I'm getting an HTTP 404 on that SRPM URL.
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=750139
--- Comment #10 from Brendan Jones brendan.jones.it@gmail.com 2012-05-16 15:47:59 EDT --- My apologies:
SPEC: http://bsjones.fedorapeople.org/lv2-mdala-plugins.spec SRPM: http://bsjones.fedorapeople.org/lv2-mdala-plugins-1.0.0-1.fc17.src.rpm
thanks
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=750139
--- Comment #11 from Jerry James loganjerry@gmail.com 2012-05-16 17:40:38 EDT --- No problem. It's not like I've never done that. :-)
There is no %changelog entry for the new release.
I couldn't tell if the right CFLAGS were being used for compilation, so I added -v to the waf build invocation. They aren't. The compiler is being invoked like this (plus some -I and -D options):
/usr/lib/ccache/g++ -O0 -g -fshow-column -fPIC <filename>.cpp
The file COPYING is ASCII with Unix line endings, so the sed and iconv invocations in %prep create an exact duplicate of the original file.
rpm -q --provides lv2-mdala-plugins shows a bunch of .so files (Ambience.so, Bandisto.so, etc.). Is that correct, or should those be filtered out?
Everything else looks okay.
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=750139
--- Comment #12 from Brendan Jones brendan.jones.it@gmail.com 2012-05-18 05:27:17 EDT --- SPEC: http://bsjones.fedorapeople.org/lv2-mdala-plugins.spec SRPM: http://bsjones.fedorapeople.org/lv2-mdala-plugins-1.0.0-2.fc17.src.rpm
Thanks again. Changelog updated.
I have updated the wscript to honour Fedora coimpiler flags and removed the line encoding script and left the verbose flag in for good measure
The .so's are actually the plugin binaries themselves and are required
https://bugzilla.redhat.com/show_bug.cgi?id=750139
Jerry James loganjerry@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #13 from Jerry James loganjerry@gmail.com --- Sorry for the delay. It's crunch time at work.
That takes care of the last of my concerns, so this package is APPROVED.
https://bugzilla.redhat.com/show_bug.cgi?id=750139
--- Comment #14 from Jerry James loganjerry@gmail.com --- Brendan: ping.
https://bugzilla.redhat.com/show_bug.cgi?id=750139
--- Comment #15 from Jerry James loganjerry@gmail.com --- Brendan, are you out there?
https://bugzilla.redhat.com/show_bug.cgi?id=750139
--- Comment #16 from Brendan Jones brendan.jones.it@gmail.com --- Sorry James, been a bit crazy mine end workwise - will jump on this ASAP. Thanks for the review!
https://bugzilla.redhat.com/show_bug.cgi?id=750139
Brendan Jones brendan.jones.it@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #17 from Brendan Jones brendan.jones.it@gmail.com ---
Thanks for the review!
New Package SCM Request ======================= Package Name: lv2-mdala-plugins Short Description: LV2 port of the MDA VST plugins Owners: bsjones Branches: f16 f17 f18 InitialCC:
https://bugzilla.redhat.com/show_bug.cgi?id=750139
--- Comment #18 from Jason Tibbitts tibbs@math.uh.edu --- Git done (by process-git-requests).
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=750139
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=750139
--- Comment #19 from Fedora Update System updates@fedoraproject.org --- lv2-mdala-plugins-1.0.0-2.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/lv2-mdala-plugins-1.0.0-2.fc18
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=750139
--- Comment #20 from Fedora Update System updates@fedoraproject.org --- lv2-mdala-plugins-1.0.0-2.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/lv2-mdala-plugins-1.0.0-2.fc17
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=750139
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=750139
--- Comment #21 from Fedora Update System updates@fedoraproject.org --- lv2-mdala-plugins-1.0.0-2.fc18 has been pushed to the Fedora 18 testing repository.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=750139
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |CURRENTRELEASE Last Closed| |2013-01-01 23:57:22
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=750139
--- Comment #22 from Fedora Update System updates@fedoraproject.org --- lv2-mdala-plugins-1.0.0-2.fc17 has been pushed to the Fedora 17 stable repository.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=750139
--- Comment #23 from Fedora Update System updates@fedoraproject.org --- lv2-mdala-plugins-1.0.0-2.fc18 has been pushed to the Fedora 18 stable repository.
package-review@lists.fedoraproject.org