Bug 1007099

Summary: Review Request: ardour3 - Digital Audio Workstation
Product: [Fedora] Fedora Reporter: Nils Philippsen <nphilipp>
Component: Package ReviewAssignee: Brendan Jones <brendan.jones.it>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: brendan.jones.it, calamandrei, hdegoede, i, lightdot, mark, notting, nphilipp, oget.fedora, rdieter
Target Milestone: ---Flags: brendan.jones.it: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: ardour3-3.5.74-2.fc19 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-11-19 12:04:03 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 1022255    
Bug Blocks: 929044    
Attachments:
Description Flags
licensecheck.txt none

Description Nils Philippsen 2013-09-11 22:21:41 UTC
Spec URL: http://nphilipp.fedorapeople.org/review/ardour3/ardour3.spec
SRPM URL: http://nphilipp.fedorapeople.org/review/ardour3/ardour3-3.4-0.1.fc19.src.rpm
Description: Ardour is a multi-channel digital audio workstation, allowing users to record, edit, mix and master audio and MIDI projects. It is targeted at audio engineers, musicians, soundtrack editors and composers.
Fedora Account System Username: nphilipp

NB: this is version 3.x of the existing ardour package and supposed to be installable in parallel. We'll keep the old version around because while the new version can open ardour 2.x projects, some information may be lost during import and people should be able to edit their old projects without running afoul of this. Users should use the current version 3.x for new projects.

Comment 1 Nils Philippsen 2013-09-11 22:23:47 UTC
NB^2: I'll keep the release number as 0.x during the review and will bump it to 1 during import after the review is finished.

Comment 2 Nils Philippsen 2013-09-12 08:06:31 UTC
I've found out in a scratch build (koji doesn't work for me ATM) that the build dependencies are not complete, I'll upload a fixed spec file and SRPM soon.

Comment 3 Nils Philippsen 2013-09-12 20:00:35 UTC
Spec URL: http://nphilipp.fedorapeople.org/review/ardour3/ardour3.spec
SRPM URL: http://nphilipp.fedorapeople.org/review/ardour3/ardour3-3.4-0.2.fc19.src.rpm

This release lists all build dependencies and contains a patch to build the package on ARM.

Comment 4 Nils Philippsen 2013-09-12 20:02:17 UTC
FWIW, here's a scratch build against Rawhide from the above SRPM:

http://koji.fedoraproject.org/koji/taskinfo?taskID=5928568

Comment 5 Orcan Ogetbil 2013-09-24 04:04:46 UTC
Hi Nils, thanks for putting this together. 

From the koji logs I see that the Fedora %optflags are not passed to the compiler. I had a one liner fix/hack in my SPEC file prototype posted in the earlier bug (and in our email discussion), but I don't know if my fix would still apply to this version. It's been a while.

Comment 6 Nils Philippsen 2013-09-24 10:28:21 UTC
Thanks! I've set LINKFLAGS to %optflags as well for good measure. Interestingly, it seems as if you have to add "--optimize" to the "./waf configure" line for the env variables to have an effect.

Anyway, here are the current files and the corresponding scratch build:

Spec URL: http://nphilipp.fedorapeople.org/review/ardour3/ardour3.spec
SRPM URL: http://nphilipp.fedorapeople.org/review/ardour3/ardour3-3.4-0.3.fc19.src.rpm
Scratch Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5975191

Comment 7 Brendan Jones 2013-09-26 06:07:44 UTC
Couple of things from a quick first look:

Your %optflags are being superseded in the wscript.
'-O3', '-fomit-frame-pointer', '-ffast-math', '-fstrength-reduce', '-pipe',

A few inconsistent macros %_prefix v %{_prefix} in the waf configure line 

There are a number of libraries here which may require unbundling. Look in the libs directory of the source tree. Most are present in Ardour2 as well. 

Oget: will exceptions be required for these or are they permitted? There are some new ones to relating to video editing which are projects in there own right (libtimecode, libtc from https://github.com/x42 for example). 

Please compile with windows-vst support for x86. This is done using fst which does not rely on proprietary Steinberg headers

Comment 8 Orcan Ogetbil 2013-09-26 13:12:16 UTC
The libs were already there when I got the ownership of ardour2. In ardour2, there is a command line option (which we are using) to scons to use system libs. But this option applies to only a subset of the bundled libs and also the ardour devs were discouraging its use (there was a statement that they will not support users using system libs). From what I can tell they rely on the behavior of the particular versions of these libs that they bundle. Since this is a fresh review, I would contact Paul or other ardour devs to ask the situation of these bundled libs, whether they are vanilla or patched, and how safe is it to unbundle them.

Same story with the compilation flags. ardour2, and now I see that also ardour3, insist on using certain flags such as -O3, which is supposed to have some performance gain over -O2. The down side is, -O3 is not reliable on bad code. In the case of ardour, I don't think we should worry about the code quality. I know that the language in the packaging guidelines is blurry when it comes to compilation flwolud be good to document this in the SPEC file

As for the vst, for ardour2 we need to recompile the whole project to enable vst support. Moreover, there was some README file where the devs tell to the packager that the vst enabled ardour should not be called "ardour", and it should go to another (sub)package. Since ardour2 did not support parallel installation easily, I did not provide two versions (with and without vst) simultaneously. I don't know how much of this applies to ardour3.

Comment 9 Brendan Jones 2013-09-27 14:46:34 UTC
Yeah, it looks like the situation with regards to windows vst hasn't changed. Apparently it is associated with a loss of stability and thus the separate executable.

However, native linux support is also missing ( --lxvst  all arches) 

Some of the libs I looked at are behind some of those in Fedora. Look I'm happy with the libs as long as the new ones get looked at, with maybe a query upstream.

We are probably better off with -O3 I agree.

Comment 10 Brendan Jones 2013-10-03 06:40:19 UTC
Nils, Ping? 

Also, I'm looking for a review of bug 1004231 - its really small.

Comment 11 Nils Philippsen 2013-10-09 15:48:48 UTC
Hey,

I've meanwhile talked to Paul Davis and he's open to the idea of using unbundled versions for rubberband, taglib, libltc and vamp-sdk. I'm more or less ready with the changes to the build scripts for the first two, libltc needs to be done but shouldn't be too hard, but vamp-(plugin-)sdk probably needs some (hopefully minor) code changes in ardour -- mostly renamed headers and disentangled C++ namespaces between host and plugin code -- I'll have to try it out.

Anyway, I've combed through the libs and this is what I found:

- ardour, backends, gtkmm2ext, panners, surfaces, timecode, vamp-plugins: internal to ardour
- audiographer, evoral, gtkmm2ext, libltc, midi++2, pbd: developed for ardour and apparently not used elsewhere. I'd treat these as internal as well -- libltc is the most borderline of these (packaged as tarball, package is in Debian/Ubuntu but I still can't find it being used elsewhere.
- clearlooks-newer: clearlooks gtk engine modified for ardour (-> internal)
- clearlooks-older: older version of the above, unused
- fst: only used for windows-vst support
- appleutility: only used on OS/X (Darwin)
- qm-dsp: externally maintained library, used for building vamp-plugins, the upstream version doesn't build shared objects
- libltc: externally maintained library, I can split this out

What do you think?

For the compiler flags, I'd like to be able to override them, even if we end up using -O3 and whatnot. I'll add --lxvst, but it's default on Linux anyway.

I may have some evenings in the next few weeks, but not immediately, so if bug 1004231 doesn't have a reviewer then, I guess I can give it a shot.

Comment 12 Brendan Jones 2013-10-09 23:40:55 UTC
Sounds good Nils. Let me know and I'll review libltc as well.

fst (bug 1015958) is already in the pipeline, albeit without a reviewer.

Comment 13 Orcan Ogetbil 2013-10-09 23:49:34 UTC
Thank you for your work Nils. I agree that unbundling only the externally maintained libraries is sufficient.

Comment 14 Nils Philippsen 2013-10-10 10:10:59 UTC
Hmm, given Paul's stance on Windows VSTs on Linux -- http://manual.ardour.org/working-with-plugins/windows-vst-support/ -- I'm not keen of having it, even if it were feasible to have a second binary supporting it: It's more prone to crashes, it only builds on 32bit ix86, and debugging it is a lot harder with the involvement of wine and so forth.

As soon as I find the time, I'll package libltc and submit it for review, and then work on unbundling it and vamp-(plugin-)sdk.

Comment 15 Nils Philippsen 2013-10-28 13:25:35 UTC
Spec URL: http://nphilipp.fedorapeople.org/review/ardour3/ardour3.spec
SRPM URL: http://nphilipp.fedorapeople.org/review/ardour3/ardour3-3.5.14-0.1.fc19.src.rpm

This package should address the issues mentioned (and updates to the current release 3.5.14). It doesn't have a "tweaked" icon to distinguish ardour 3.x from 2.x, though. I've talked to upstream about it but we didn't come to a conclusion what to do about it.

Comment 16 Lapo Calamandrei 2013-10-29 13:34:36 UTC
If you need a tweaked icon downstream I can provide one, not sure I have the time to upstream it though

Comment 17 Brendan Jones 2013-10-30 07:06:49 UTC
Created attachment 817309 [details]
licensecheck.txt

Comment 18 Brendan Jones 2013-10-30 07:07:54 UTC
Just a couple of issues:

 - Can you add Requires: lv2 rather than specifying the directory and specifiy /usr/lib64/lv2/reasonablesynth.lv2 explicitly? Or (even better) can you split the plugin into a separate lv2-reasonablesynth package? 

 - see attached licensecheck.txt. Can you review and just make sure we have covered all Licenses used?

 - rpmlint:

ardour3.x86_64: W: incoherent-version-in-changelog 3.5.14-1 ['3.5.14-0.1.fc20', '3.5.14-0.1']
ardour3.src:84: W: macro-in-comment %{version}

 - can you filter the ardour libraries from provides

Comment 19 Nils Philippsen 2013-10-30 10:41:41 UTC
(In reply to Lapo Calamandrei from comment #16)
> If you need a tweaked icon downstream I can provide one, not sure I have the
> time to upstream it though

That'd be great, even if only as a stand-in as long as upstream doesn't come up with a solution that they prefer. Thanks!

Comment 20 Nils Philippsen 2013-10-30 21:32:19 UTC
(In reply to Brendan Jones from comment #18)
> Just a couple of issues:
> 
>  - Can you add Requires: lv2 rather than specifying the directory and
> specifiy /usr/lib64/lv2/reasonablesynth.lv2 explicitly? Or (even better) can
> you split the plugin into a separate lv2-reasonablesynth package? 

Hmm, I'm not convinced either of them is the right thing to do. The synth is considered a part of ardour to be able to play back MIDI tracks always (MIDI tracks are a new feature of ardour v3.x). And lv2 is just a set of other plugins that aren't necessary to ardour, it's not a library.

>  - see attached licensecheck.txt. Can you review and just make sure we have
> covered all Licenses used?

Hmm, there are a couple of BSD-licensed source files (various versions), which seem to require including the license texts with the package, I'll contact upstream about the issue, meanwhile include them directly. Other than that, libltc is licensed under GPLv3+ however we don't build it and link against the packaged libltc instead. Not sure if this should be reflected in the package license tag, do you have an idea?

>  - rpmlint:
> 
> ardour3.x86_64: W: incoherent-version-in-changelog 3.5.14-1
> ['3.5.14-0.1.fc20', '3.5.14-0.1']

As I said, I'll bump this to -1 when importing into dist-git.

> ardour3.src:84: W: macro-in-comment %{version}

That's an artifact from that "in-between" version 3.5.14 -- upstream mis-tagged 3.5 and 3.5.14 is the subsequent "real" release of 3.5. A bit of a mess, but I like to keep the commented out line around for the next (hopefully correctly tagged ;-) version comes around. Anyhow, the version macro doesn't have any annoying side-effects when commented out (unlike e.g. %define) so it's not harmful.

>  - can you filter the ardour libraries from provides

Sure.

Comment 22 Brendan Jones 2013-10-30 22:46:54 UTC
(In reply to Nils Philippsen from comment #20)
> (In reply to Brendan Jones from comment #18)
> > Just a couple of issues:
> > 
> >  - Can you add Requires: lv2 rather than specifying the directory and
> > specifiy /usr/lib64/lv2/reasonablesynth.lv2 explicitly? Or (even better) can
> > you split the plugin into a separate lv2-reasonablesynth package? 

OK, agreed on not splitting a separate package, but I still think you need to Requires: lv2.
It owns the /usr/lib64/lv2 filesystem and all packages that drop plugins under it must require it. They are not binary plugins as such, but rather the metadata that describes the specification. 

> 
> Hmm, I'm not convinced either of them is the right thing to do. The synth is
> considered a part of ardour to be able to play back MIDI tracks always (MIDI
> tracks are a new feature of ardour v3.x). And lv2 is just a set of other
> plugins that aren't necessary to ardour, it's not a library.
> 
> >  - see attached licensecheck.txt. Can you review and just make sure we have
> > covered all Licenses used?
> 
> Hmm, there are a couple of BSD-licensed source files (various versions),
> which seem to require including the license texts with the package, I'll
> contact upstream about the issue, meanwhile include them directly. Other
> than that, libltc is licensed under GPLv3+ however we don't build it and
> link against the packaged libltc instead. Not sure if this should be
> reflected in the package license tag, do you have an idea?
> 
I think if you just follow the guidelines here we should be OK:

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/LicensingGuidelines#Multiple_Licensing_Scenarios

> 
> > ardour3.src:84: W: macro-in-comment %{version}
> 
> That's an artifact from that "in-between" version 3.5.14 -- upstream
> mis-tagged 3.5 and 3.5.14 is the subsequent "real" release of 3.5. A bit of
> a mess, but I like to keep the commented out line around for the next
> (hopefully correctly tagged ;-) version comes around. Anyhow, the version
> macro doesn't have any annoying side-effects when commented out (unlike e.g.
> %define) so it's not harmful.
> 

Just add an extra % to keep rpmlint happy

Comment 23 Nils Philippsen 2013-10-31 00:51:55 UTC
(In reply to Brendan Jones from comment #22)
> (In reply to Nils Philippsen from comment #20)
> > (In reply to Brendan Jones from comment #18)
> > > Just a couple of issues:
> > > 
> > >  - Can you add Requires: lv2 rather than specifying the directory and
> > > specifiy /usr/lib64/lv2/reasonablesynth.lv2 explicitly? Or (even better) can
> > > you split the plugin into a separate lv2-reasonablesynth package? 
> 
> OK, agreed on not splitting a separate package, but I still think you need
> to Requires: lv2.
> It owns the /usr/lib64/lv2 filesystem and all packages that drop plugins
> under it must require it. They are not binary plugins as such, but rather
> the metadata that describes the specification. 

Yet these are not required for ardour, or the included synth plugin to function, i.e. the ardour3 package should own the directory instead of needlessly pulling in the lv2 package:

http://fedoraproject.org/wiki/Packaging:Guidelines#The_directory_is_owned_by_a_package_which_is_not_required_for_your_package_to_function

Frankly I don't have a clue why there is an lv2 package at all, rather than shipping the *.ttl files in the -devel subpackage -- they don't seem to fulfill a runtime purpose.

> > Hmm, I'm not convinced either of them is the right thing to do. The synth is
> > considered a part of ardour to be able to play back MIDI tracks always (MIDI
> > tracks are a new feature of ardour v3.x). And lv2 is just a set of other
> > plugins that aren't necessary to ardour, it's not a library.
> > 
> > >  - see attached licensecheck.txt. Can you review and just make sure we have
> > > covered all Licenses used?
> > 
> > Hmm, there are a couple of BSD-licensed source files (various versions),
> > which seem to require including the license texts with the package, I'll
> > contact upstream about the issue, meanwhile include them directly. Other
> > than that, libltc is licensed under GPLv3+ however we don't build it and
> > link against the packaged libltc instead. Not sure if this should be
> > reflected in the package license tag, do you have an idea?
> > 
> I think if you just follow the guidelines here we should be OK:
> 
> https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/
> LicensingGuidelines#Multiple_Licensing_Scenarios

The "License" field covers the resulting binary (not source, not shared libraries used) and the versions of the BSD licenses used for these files are GPL compatible -> GPLv2 is sufficient.

> > > ardour3.src:84: W: macro-in-comment %{version}
> > 
> > That's an artifact from that "in-between" version 3.5.14 -- upstream
> > mis-tagged 3.5 and 3.5.14 is the subsequent "real" release of 3.5. A bit of 
> > a mess, but I like to keep the commented out line around for the next
> > (hopefully correctly tagged ;-) version comes around. Anyhow, the version
> > macro doesn't have any annoying side-effects when commented out (unlike e.g.
> > %define) so it's not harmful.
> > 
> 
> Just add an extra % to keep rpmlint happy

But rpmlint is happy :), the warning is because some macros have an effect even if commented out (e.g. %define). Quoting the ampersand doesn't really help me, so I'll rather remove the line:

Spec URL: http://nphilipp.fedorapeople.org/review/ardour3/ardour3.spec
SRPM URL: http://nphilipp.fedorapeople.org/review/ardour3/ardour3-3.5.14-0.3.fc19.src.rpm

Comment 24 Lapo Calamandrei 2013-10-31 08:45:11 UTC
I'm getting there with the icons, I'll attach them soon, The name of the ardour binary is ardour3 right?
To have different icons from ardour2 I think the best way is just playing with colors (red for 3, blue for 2), mostly to respect the application logo which the icon is derived from, so I'd need to have new icons for the 2 version as well, which anyway does not ship all the needed sizes (hires icons missing) and the 16x16 icon sports a dropshadow which shouldn't be there; so I'll file a bug for 2 as soon as I have the graphics ready.

Comment 25 Nils Philippsen 2013-10-31 09:16:08 UTC
(In reply to Lapo Calamandrei from comment #24)
> I'm getting there with the icons, I'll attach them soon, The name of the
> ardour binary is ardour3 right?

Right.

> To have different icons from ardour2 I think the best way is just playing
> with colors (red for 3, blue for 2), mostly to respect the application logo
> which the icon is derived from, so I'd need to have new icons for the 2
> version as well, which anyway does not ship all the needed sizes (hires
> icons missing) and the 16x16 icon sports a dropshadow which shouldn't be
> there; so I'll file a bug for 2 as soon as I have the graphics ready.

Well, if you're changing the icons for ardour 2.x, I'm not sure why we need new icons for 3.x -- upstream is quite happy with the logo, they just don't know what to do best to distinguish the 2 versions ;-).

Comment 26 Lapo Calamandrei 2013-11-01 17:41:09 UTC
Ardour (both 2.x and 3.x versions) doesn't ship a 256x256 app icon,  so,  for example,  you have ugly icons in gnome shell app picker.
Also the 16x16 icons has a drop shadow which should not be there, so it wrong wrt tango style (gnome,  xfce, unity) or oxygen (kde) icons.
Since some tweaks are needed I think it's a good opportunity to [defeat my lazyness and] fix all those issues :)

Comment 27 Brendan Jones 2013-11-02 12:42:33 UTC
Sorry for the delay - free time has been invade by my day job.

(In reply to Nils Philippsen from comment #23)
> http://fedoraproject.org/wiki/Packaging:
> Guidelines#The_directory_is_owned_by_a_package_which_is_not_required_for_your
> _package_to_function

OK

> Frankly I don't have a clue why there is an lv2 package at all, rather than
> shipping the *.ttl files in the -devel subpackage -- they don't seem to
> fulfill a runtime purpose.

I asked LV2 upstream about this. There are some hosts which require the spec ttl files to be present, but Ardour is not one of them. ingen is one for example.

Only other issue is that the filter results in this for all internal libraries, which is not what I expected:

Error: Package: ardour3-3.5.14-0.3.fc20.x86_64 (/ardour3-3.5.14-0.3.fc20.x86_64)
           Requires: libardourvampplugins.so.0()(64bit)

All other issues seem to be resolved. Thanks for packaging this.
Remove the filter before importing and this package is APPROVED

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[x] = Manual review needed



===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[ ]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "WTFPL WTFPL (v2)", "LGPL (v2.1 or later) (with incorrect FSF address)",
     "LGPL (v2 or later) (with incorrect FSF address)", "MPL (v1.1) LGPL
     (v2.1) (with incorrect FSF address)", "LGPL (v2.1) (with incorrect FSF
     address)", "ISC", "*No copyright* GPL (v2 or later)", "*No copyright*
     LGPL (v2.1 or later)", "*No copyright* GPL (v2 or later) (with incorrect
     FSF address)", "zlib/libpng", "LGPL (v3 or later)", "GPL (v2 or later)
     (with incorrect FSF address)", "BSD (2 clause)", "*No copyright* LGPL (v2
     or later) (with incorrect FSF address)", "GPL", "*No copyright* WTFPL",
     "GPL (v2 or later)", "MIT/X11 (BSD like)", "BSD (3 clause)", "*No
     copyright* LGPL (v2 or later)", "Unknown or generated", "BSD (4 clause)",
     "BSL (v1.0)", "GPL (v2)". 358 files have unknown license. Detailed output
     of licensecheck in /tmp/review-ardour3/licensecheck.txt
[x]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by: /usr/lib64/lv2(lv2)
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: The spec file handles locales properly.
[x]: Package consistently uses macros (instead of hard-coded directory names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[x]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[x]: Package contains systemd file(s) if in need.
[x]: update-desktop-database is invoked in %post and %postun if package
     contains desktop file(s) with a MimeType: entry.
     Note: desktop file(s) with MimeType entry in ardour3
[x]: gtk-update-icon-cache is invoked in %postun and %posttrans if package
     contains icons.
     Note: icons in ardour3
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 30720 bytes in 2 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the license(s)
     in its own file, then that file, containing the text of the license(s)
     for the package is included in %doc.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: %config files are marked noreplace or the reason is justified.
[x]: Each %files section contains %defattr if rpm < 4.4
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package contains desktop file if it is a GUI application.
[x]: Package installs a %{name}.desktop using desktop-file-install or desktop-
     file-validate if there is such a file.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: No %config files under /usr.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[ ]: Avoid bundling fonts in non-fonts packages.
     Note: Package contains font files
[ ]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[ ]: Final provides and requires are sane (see attachments).
[ ]: Package functions as described.
[ ]: Latest version is packaged.
[ ]: Package does not include license text files separate from upstream.
[ ]: Patches link to upstream bugs/comments/lists or are otherwise justified.
[ ]: SourceX tarball generation or download is documented.
     Note: Package contains tarball without URL, check comments
[ ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[ ]: Package should compile and build into binary rpms on all supported
     architectures.
[ ]: %check is present and all tests pass.
[ ]: Packages should try to preserve timestamps of original installed files.
[ ]: update-mime-database is invoked in %post and %postun if package stores
     mime configuration in /usr/share/mime/packages.
     Note: mimeinfo files in: ardour3
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: Dist tag is present (not strictly required in GL).
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
     Note: Arch-ed rpms have a total of 8755200 bytes in /usr/share
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: ardour3-3.5.14-0.3.fc20.x86_64.rpm
          ardour3-3.5.14-0.3.fc20.src.rpm
ardour3.x86_64: W: spelling-error %description -l en_US Ardour -> Armour, Ar dour, Ar-dour
ardour3.x86_64: W: spelling-error %description -l en_US multi -> mulch, mufti
ardour3.x86_64: W: incoherent-version-in-changelog 3.5.14-1 ['3.5.14-0.3.fc20', '3.5.14-0.3']
ardour3.x86_64: E: incorrect-fsf-address /usr/share/doc/ardour3/COPYING
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libardourcp.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libevoral.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libltc.so
ardour3.x86_64: W: dangling-relative-symlink /usr/share/ardour3/ArdourMono.ttf ../fonts/google-droid/DroidSansMono.ttf
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/surfaces/libardour_wiimote.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libtimecode.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libvamphost.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/surfaces/libardour_generic_midi.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libmidipp.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libtaglib.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/panners/libpan2in2out.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/backends/libjack_audiobackend.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/librubberband.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/surfaces/libardour_mcp.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libardour.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libqmdsp.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libaudiographer.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libgtkmm2ext.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libpbd.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/panners/libpanvbap.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/surfaces/libardour_osc.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/vamp/libardourvampplugins.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/panners/libpan1in2out.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libvampplugin.so
ardour3.src: W: spelling-error %description -l en_US Ardour -> Armour, Ar dour, Ar-dour
ardour3.src: W: spelling-error %description -l en_US multi -> mulch, mufti
ardour3.src: W: invalid-url Source0: Ardour3-3.5.14.tar.bz2
2 packages and 0 specfiles checked; 1 errors, 30 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint ardour3
ardour3.x86_64: W: spelling-error %description -l en_US Ardour -> Armour, Ar dour, Ar-dour
ardour3.x86_64: W: spelling-error %description -l en_US multi -> mulch, mufti
ardour3.x86_64: W: incoherent-version-in-changelog 3.5.14-1 ['3.5.14-0.1.fc20', '3.5.14-0.1']
ardour3.x86_64: E: incorrect-fsf-address /usr/share/doc/ardour3/COPYING
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libardourcp.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libevoral.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libltc.so
ardour3.x86_64: W: dangling-relative-symlink /usr/share/ardour3/ArdourMono.ttf ../fonts/google-droid/DroidSansMono.ttf
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/surfaces/libardour_wiimote.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libtimecode.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libvamphost.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/surfaces/libardour_generic_midi.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libmidipp.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libtaglib.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/panners/libpan2in2out.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/backends/libjack_audiobackend.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/librubberband.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/surfaces/libardour_mcp.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libardour.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libqmdsp.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libaudiographer.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libgtkmm2ext.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libpbd.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/panners/libpanvbap.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/surfaces/libardour_osc.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/vamp/libardourvampplugins.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/panners/libpan1in2out.so
ardour3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ardour3/libvampplugin.so
1 packages and 0 specfiles checked; 1 errors, 27 warnings.
# echo 'rpmlint-done:'



Requires
--------
ardour3 (rpmlib, GLIBC filtered):
    /bin/sh
    config(ardour3)
    google-droid-sans-mono-fonts
    libFLAC.so.8()(64bit)
    libX11.so.6()(64bit)
    libardour.so.3()(64bit)
    libardour_generic_midi.so.4()(64bit)
    libardour_mcp.so.4()(64bit)
    libardour_osc.so.4()(64bit)
    libardour_wiimote.so.4()(64bit)
    libardourcp.so.4()(64bit)
    libardourvampplugins.so.0()(64bit)
    libart_lgpl_2.so.2()(64bit)
    libasound.so.2()(64bit)
    libasound.so.2(ALSA_0.9)(64bit)
    libatk-1.0.so.0()(64bit)
    libatkmm-1.6.so.1()(64bit)
    libaubio.so.2()(64bit)
    libaudiographer.so.0()(64bit)
    libbluetooth.so.3()(64bit)
    libc.so.6()(64bit)
    libcairo.so.2()(64bit)
    libcairomm-1.0.so.1()(64bit)
    libcurl.so.4()(64bit)
    libcwiid.so.1()(64bit)
    libdl.so.2()(64bit)
    libevoral.so.0()(64bit)
    libfftw3.so.3()(64bit)
    libfftw3f.so.3()(64bit)
    libfontconfig.so.1()(64bit)
    libfreetype.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libgdk-x11-2.0.so.0()(64bit)
    libgdk_pixbuf-2.0.so.0()(64bit)
    libgdkmm-2.4.so.1()(64bit)
    libgio-2.0.so.0()(64bit)
    libgiomm-2.4.so.1()(64bit)
    libglib-2.0.so.0()(64bit)
    libglibmm-2.4.so.1()(64bit)
    libgnomecanvas-2.so.0()(64bit)
    libgnomecanvasmm-2.6.so.1()(64bit)
    libgobject-2.0.so.0()(64bit)
    libgthread-2.0.so.0()(64bit)
    libgtk-x11-2.0.so.0()(64bit)
    libgtkmm-2.4.so.1()(64bit)
    libgtkmm2ext.so.0()(64bit)
    libjack.so.0()(64bit)
    libjack_audiobackend.so.1()(64bit)
    liblilv-0.so.0()(64bit)
    liblo.so.7()(64bit)
    liblrdf.so.2()(64bit)
    libltc.so.1()(64bit)
    libm.so.6()(64bit)
    libmidipp.so.4()(64bit)
    libogg.so.0()(64bit)
    libpan1in2out.so.1()(64bit)
    libpan2in2out.so.1()(64bit)
    libpango-1.0.so.0()(64bit)
    libpangocairo-1.0.so.0()(64bit)
    libpangoft2-1.0.so.0()(64bit)
    libpangomm-1.4.so.1()(64bit)
    libpanvbap.so.1()(64bit)
    libpbd.so.4()(64bit)
    libpthread.so.0()(64bit)
    libqmdsp.so.0()(64bit)
    librubberband.so.4()(64bit)
    libsamplerate.so.0()(64bit)
    libsamplerate.so.0(libsamplerate.so.0.0)(64bit)
    libserd-0.so.0()(64bit)
    libsigc-2.0.so.0()(64bit)
    libsmf.so()(64bit)
    libsndfile.so.1()(64bit)
    libsndfile.so.1(libsndfile.so.1.0)(64bit)
    libsord-0.so.0()(64bit)
    libsratom-0.so.0()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.1)(64bit)
    libsuil-0.so.0()(64bit)
    libtaglib.so.0()(64bit)
    libtimecode.so.0()(64bit)
    libuuid.so.1()(64bit)
    libuuid.so.1(UUID_1.0)(64bit)
    libvamphost.so.0()(64bit)
    libvampplugin.so.0()(64bit)
    libxml2.so.2()(64bit)
    libxml2.so.2(LIBXML2_2.4.30)(64bit)
    libxml2.so.2(LIBXML2_2.6.0)(64bit)
    rtld(GNU_HASH)



Provides
--------
ardour3:
    ardour3
    ardour3(x86-64)
    config(ardour3)
    mimehandler(application/x-ardour3)



Unversioned so-files
--------------------
ardour3: /usr/lib64/ardour3/backends/libjack_audiobackend.so
ardour3: /usr/lib64/ardour3/engines/libclearlooks.so
ardour3: /usr/lib64/ardour3/libardour.so
ardour3: /usr/lib64/ardour3/libardourcp.so
ardour3: /usr/lib64/ardour3/libaudiographer.so
ardour3: /usr/lib64/ardour3/libevoral.so
ardour3: /usr/lib64/ardour3/libgtkmm2ext.so
ardour3: /usr/lib64/ardour3/libltc.so
ardour3: /usr/lib64/ardour3/libmidipp.so
ardour3: /usr/lib64/ardour3/libpbd.so
ardour3: /usr/lib64/ardour3/libqmdsp.so
ardour3: /usr/lib64/ardour3/librubberband.so
ardour3: /usr/lib64/ardour3/libsmf.so
ardour3: /usr/lib64/ardour3/libtaglib.so
ardour3: /usr/lib64/ardour3/libtimecode.so
ardour3: /usr/lib64/ardour3/libvamphost.so
ardour3: /usr/lib64/ardour3/libvampplugin.so
ardour3: /usr/lib64/ardour3/panners/libpan1in2out.so
ardour3: /usr/lib64/ardour3/panners/libpan2in2out.so
ardour3: /usr/lib64/ardour3/panners/libpanvbap.so
ardour3: /usr/lib64/ardour3/surfaces/libardour_generic_midi.so
ardour3: /usr/lib64/ardour3/surfaces/libardour_mcp.so
ardour3: /usr/lib64/ardour3/surfaces/libardour_osc.so
ardour3: /usr/lib64/ardour3/surfaces/libardour_wiimote.so
ardour3: /usr/lib64/ardour3/vamp/libardourvampplugins.so
ardour3: /usr/lib64/lv2/reasonablesynth.lv2/reasonablesynth.so

Generated by fedora-review 0.5.0 (920221d) last change: 2013-08-30
Command line :/usr/bin/fedora-review -n ardour3 -m fedora-20-x86_64 -L ./rpm
Buildroot used: fedora-20-x86_64
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: Java, Python, SugarActivity, Perl, R, PHP, Ruby
Disabled flags: EPEL5, EXARCH, DISTTAG

Comment 28 Nils Philippsen 2013-11-02 21:01:47 UTC
New Package SCM Request
=======================
Package Name: ardour3
Short Description: Digital Audio Workstation
Owners: nphilipp
Branches: f18 f19 f20
InitialCC:

Comment 29 Nils Philippsen 2013-11-02 21:05:51 UTC
(In reply to Brendan Jones from comment #27)
> Sorry for the delay - free time has been invade by my day job.

No worries ;-).

> (In reply to Nils Philippsen from comment #23)
> > Frankly I don't have a clue why there is an lv2 package at all, rather than
> > shipping the *.ttl files in the -devel subpackage -- they don't seem to
> > fulfill a runtime purpose.
> 
> I asked LV2 upstream about this. There are some hosts which require the spec
> ttl files to be present, but Ardour is not one of them. ingen is one for
> example.

Ahh interesting. I'll try to keep an eye on this when dealing with other lv2 plugins.
 
> Only other issue is that the filter results in this for all internal
> libraries, which is not what I expected:
> 
> Error: Package: ardour3-3.5.14-0.3.fc20.x86_64
> (/ardour3-3.5.14-0.3.fc20.x86_64)
>            Requires: libardourvampplugins.so.0()(64bit)
>
> All other issues seem to be resolved. Thanks for packaging this.
> Remove the filter before importing and this package is APPROVED

Oh. I've added a filter for the requirements that are satisfied by internal libraries. That should do the trick without polluting the RPM deps unduly.

Thanks for reviewing!

Comment 30 Gwyn Ciesla 2013-11-04 12:51:58 UTC
Git done (by process-git-requests).

Comment 31 Fedora Update System 2013-11-04 18:44:38 UTC
ardour3-3.5.14-1.fc18, libltc-1.1.2-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/FEDORA-2013-20148/ardour3-3.5.14-1.fc18,libltc-1.1.2-1.fc18

Comment 32 Fedora Update System 2013-11-04 18:45:49 UTC
ardour3-3.5.14-1.fc20, libltc-1.1.2-1.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/FEDORA-2013-20094/ardour3-3.5.14-1.fc20,libltc-1.1.2-1.fc20

Comment 33 Fedora Update System 2013-11-04 18:46:08 UTC
ardour3-3.5.14-1.fc19, libltc-1.1.2-1.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/FEDORA-2013-20198/ardour3-3.5.14-1.fc19,libltc-1.1.2-1.fc19

Comment 34 Fedora Update System 2013-11-05 02:49:22 UTC
ardour3-3.5.14-1.fc18, libltc-1.1.2-1.fc18 has been pushed to the Fedora 18 testing repository.

Comment 35 Brendan Jones 2013-11-06 05:38:39 UTC
Ardour 3.5.74 released today which includes your unbundling and optflags changes.

Comment 36 Fedora Update System 2013-11-19 21:49:12 UTC
ardour3-3.5.74-2.fc20, libltc-1.1.3-1.fc20 has been pushed to the Fedora 20 stable repository.

Comment 37 Fedora Update System 2013-11-21 04:34:25 UTC
ardour3-3.5.74-2.fc18, libltc-1.1.3-1.fc18 has been pushed to the Fedora 18 stable repository.

Comment 38 Fedora Update System 2013-11-21 04:42:01 UTC
ardour3-3.5.74-2.fc19, libltc-1.1.3-1.fc19 has been pushed to the Fedora 19 stable repository.