https://bugzilla.redhat.com/show_bug.cgi?id=2137159
Bug ID: 2137159 Summary: Review Request: ardour7 - Digital Audio Workstation Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: nphilipp@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://nphilipp.fedorapeople.org/review/ardour7/ardour7.spec SRPM URL: https://nphilipp.fedorapeople.org/review/ardour7/ardour7-7.0.0-0.1.fc37.src.... 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 largely a touch up of the current ardour6 package adapted for the newly released version 7.0 of Ardour with some old cruft removed. - I'll keep the release number below 1 during the review.
https://bugzilla.redhat.com/show_bug.cgi?id=2137159
Nils Philippsen nphilipp@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |nphilipp@redhat.com
--- Comment #1 from Nils Philippsen nphilipp@redhat.com --- NB^2: The reason for having a separate package for this new version is so people can choose which version of Ardour to use for existing and new projects.
https://bugzilla.redhat.com/show_bug.cgi?id=2137159
Nils Philippsen nphilipp@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |upstream-release-monitoring | |@fedoraproject.org
--- Comment #2 from Nils Philippsen nphilipp@redhat.com --- *** Bug 2135048 has been marked as a duplicate of this bug. ***
https://bugzilla.redhat.com/show_bug.cgi?id=2137159
--- Comment #3 from Mads Kiilerich mads@kiilerich.com --- (In reply to Nils Philippsen from comment #1)
NB^2: The reason for having a separate package for this new version is so people can choose which version of Ardour to use for existing and new projects.
As mentioned in https://src.fedoraproject.org/rpms/ardour6/pull-request/5 (currently offline):
I think that putting the first part for the version number in the package name is a bad idea. The package name should just be "ardour".
* Ardour doesn't use semantic versioning. The step from 6.9 to 7.0 is not necessarily bigger than the step from 7.0 to 7.1 . Allowing side-by-side installation only when the first number changes will in general miss the point and just "work" randomly.
* That is not how we do in Fedora. Unless there are very good reasons, we package the latest and greatest version of end user software. We do for example not have two versions of for example OpenOffice, Firefox, GIMP, or Inkscape. It is unclear why this particular package needs it, and what problem this is solving.
* Ardour 7 can read (and upgrade) Ardour 6 projects. There is no particular good reason these two versions should be installed side-by-side. Supporting evidence: Upstream no longer makes Ardour 6 (easily) available for download.
* Having the version number in the package does that a trivial update to a new major version requires a new package review, for no good reason. That can potentially delay upgrades and put extra load on the review process.
* Having two versions side by side raise tricky upgrade questions. The general user experience should of course be that Ardour automatically gets upgraded to Ardour 7 ... or at least that it is installed automatically so it is available for launch, next to Ardour 6. This spec doesn't handle that at all.
https://bugzilla.redhat.com/show_bug.cgi?id=2137159
Guido Aulisi guido.aulisi@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |guido.aulisi@gmail.com Assignee|nobody@fedoraproject.org |guido.aulisi@gmail.com
--- Comment #4 from Guido Aulisi guido.aulisi@gmail.com --- Having separate packages was almost a must when versione 3 was released. It was really incompatible with version 2 and the migration of the session file had some bad bugs. I remember that some mixer settings were zeroed, and still now converting from 4 to 6 makes some stereo channels become mono!
https://bugzilla.redhat.com/show_bug.cgi?id=2137159
--- Comment #5 from Nils Philippsen nphilipp@redhat.com --- (In reply to Mads Kiilerich from comment #3)
(In reply to Nils Philippsen from comment #1)
NB^2: The reason for having a separate package for this new version is so people can choose which version of Ardour to use for existing and new projects.
As mentioned in https://src.fedoraproject.org/rpms/ardour6/pull-request/5 (currently offline):
I think that putting the first part for the version number in the package name is a bad idea. The package name should just be "ardour".
I think I remember we talked about this before, yeah. The naming is done this way according to the guidelines for the reasons I’ll go into below: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#multiple
- Ardour doesn't use semantic versioning. The step from 6.9 to 7.0 is not
necessarily bigger than the step from 7.0 to 7.1 . Allowing side-by-side installation only when the first number changes will in general miss the point and just "work" randomly.
Not using semantic versioning doesn’t preclude incompatible breaks between one major version and the next though…
- That is not how we do in Fedora. Unless there are very good reasons, we
package the latest and greatest version of end user software. We do for example not have two versions of for example OpenOffice, Firefox, GIMP, or Inkscape. It is unclear why this particular package needs it, and what problem this is solving.
…which is precisely what happened from version 2 to 3 and why we agreed on this scheme in a discussion on the (now largely defunct) fedora-music mailing list (i.e. this is why we did it that way in Fedora): Version 3 changed some internals in a way that it couldn’t fully interpret version 2 projects. So while version 3 would attempt to upgrade version 2 projects, any project utilizing the affected functionality would be converted effectively incurring data loss. Aside from the glitch Guido mentioned, another (minor) reason was significant changes in the UI which I believe was a hangup for some users going from version 3 to 4.
Anyway, the scheme was put in place so we would be free in the future to introduce new major versions with potentially breaking changes (functionality- or UI-wise) and give people a transition period in which they can adapt their workflows.
For reference, here's the discussion on the mailing list: https://lists.fedoraproject.org/pipermail/music/2015-May/002005.html
- Ardour 7 can read (and upgrade) Ardour 6 projects. There is no particular
good reason these two versions should be installed side-by-side. Supporting evidence: Upstream no longer makes Ardour 6 (easily) available for download.
This is all good in hindsight, but as I understand it, there are no guarantees that this will be the case for future major versions.
- Having the version number in the package does that a trivial update to a
new major version requires a new package review, for no good reason. That can potentially delay upgrades and put extra load on the review process.
Having to review a new major version as a package is obviously the downside of this scheme, but in my opinion it’s a relatively small price to pay (and nets us spec files with less accumulated cruft as a side effect).
- Having two versions side by side raise tricky upgrade questions. The
general user experience should of course be that Ardour automatically gets upgraded to Ardour 7 ... or at least that it is installed automatically so it is available for launch, next to Ardour 6. This spec doesn't handle that at all.
I’m open to improving things in that area. For instance, once things with version 7 have settled a little without breakage (say with 7.1 or 7.2), we can let it obsolete the ardour6 package. Making it install itself automatically side-by-side could be done as well, it's just a little bit more involved: we’d need a new ardour6 package and both it and ardour7 would obsolete the old ardour6 version-release (plus, ardour6 should then probably get a modified app icon so people can distinguish them in the list of apps).
https://bugzilla.redhat.com/show_bug.cgi?id=2137159
Guido Aulisi guido.aulisi@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Flags| |fedora-review+
--- Comment #6 from Guido Aulisi guido.aulisi@gmail.com --- Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = 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]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Header files in -devel subpackage, if present. [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. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "GNU General Public License, Version 2", "BSD 3-Clause License BSD 2-Clause License GNU General Public License, Version 3 GNU General Public License v2.0 or later", "*No copyright* [generated file]", "*No copyright* BSD 3-Clause License", "GNU General Public License v3.0 or later", "GNU General Public License v2.0 or later", "GNU General Public License v2.0 or later [obsolete FSF postal address (Mass Ave)]", "*No copyright* Do What The Fuck You Want To Public License, Version 2", "Public domain GNU General Public License v2.0 or later", "*No copyright* GNU General Public License v2.0 or later", "*No copyright* GNU General Public License, Version 2", "GNU General Public License", "MIT License", "GNU Library General Public License v2 or later", "*No copyright* GNU Library General Public License v2 or later", "GNU Lesser General Public License, Version 2.1", "GNU Lesser General Public License v2.1 or later [obsolete FSF postal address (Mass Ave)]", "NTP License", "GNU Lesser General Public License, Version 2.1 GNU General Public License, Version 2", "GNU Lesser General Public License", "GNU Lesser General Public License v2.1 or later", "GNU Library General Public License v2 or later [obsolete FSF postal address (Mass Ave)]", "GNU General Public License, Version 3", "Do What The Fuck You Want To Public License, Version 2", "zlib License", "*No copyright* GNU General Public License v2.0 or later [obsolete FSF postal address (Mass Ave)]", "Apple MIT License", "GNU Lesser General Public License v2.1 or later [obsolete FSF postal address (Temple Place)]", "BSD 3-Clause License GNU Lesser General Public License GNU General Public License", "*No copyright* GNU Lesser General Public License v2.1 or later", "Boost Software License 1.0", "BSD-4-Clause (University of California-Specific)", "BSD 4-Clause License BSD 2-clause NetBSD License", "GNU Lesser General Public License GNU General Public License v2.0 or later", "*No copyright* Public domain", "NTP License MIT License", "BSD 3-Clause License", "*No copyright* GNU General Public License v3.0 or later". 1877 files have unknown license. Detailed output of licensecheck in /home/guido/2137159-ardour7/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [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. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [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. [-]: 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. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [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 %license. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [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]: 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]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package must not depend on deprecated() packages. [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 does not use a name that already exists. [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]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 0 bytes in 0 files. [x]: Packages must not store files under /srv, /opt or /usr/local
===== SHOULD items =====
Generic: [x]: 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. [x]: Final provides and requires are sane (see attachments). [x]: Package functions as described. [x]: Latest version is packaged. [!]: Package does not include license text files separate from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. [x]: SourceX tarball generation or download is documented. Note: Package contains tarball without URL, check comments [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [x]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [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]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Fully versioned dependency in subpackages if applicable. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified.
===== EXTRA items =====
Generic: [!]: Large data in /usr/share should live in a noarch subpackage if package is arched. Note: Arch-ed rpms have a total of 31969280 bytes in /usr/share ardour7-7.0.0-0.1.fc38.x86_64.rpm:31969280 See:
https://fedoraproject.org/wiki/Packaging:ReviewGuidelines#Package_Review_Gui... [!]: Spec file according to URL is the same as in SRPM. Note: Spec file as given by url is not the same as in SRPM (see attached diff). See: (this test has no URL) [x]: Rpmlint is run on debuginfo package(s). Note: There are rpmlint messages (see attachment). [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment).
https://bugzilla.redhat.com/show_bug.cgi?id=2137159
--- Comment #7 from Guido Aulisi guido.aulisi@gmail.com --- Some items need extra investigation:
Package does not include license text files separate from upstream. This is clearly not true, but we need to ship GPLv3 license file
Large data in /usr/share/ardour7 Can we split it to a no arch sub package?
Spec file according to URL is the same as in SRPM. I think this is due to autospec
https://bugzilla.redhat.com/show_bug.cgi?id=2137159
--- Comment #8 from Nils Philippsen nphilipp@redhat.com --- Another case in point of having separate packages for different major versions is the changes in how tempo maps are implemented in 7.x vs. 6.x -- see https://tracker.ardour.org/view.php?id=9030#c26705
This was sort of a deliberate decision. The structure of the tempo map in 7.0 is radically different, and for now we opted to not correctly support 6.x sessions with the first tempo marker at 0.
However, we may be able to fix this at some point.
You should use 6.9 to continue working on such sessions.
(In reply to Guido Aulisi from comment #7)
Some items need extra investigation:
Package does not include license text files separate from upstream. This is clearly not true, but we need to ship GPLv3 license file
Yeah, that's just due to how the licenses of Ardour itself and bundled code are combined.
Large data in /usr/share/ardour7 Can we split it to a no arch sub package?
Sure!
Spec file according to URL is the same as in SRPM. I think this is due to autospec
Yes, it is.
https://bugzilla.redhat.com/show_bug.cgi?id=2137159
--- Comment #9 from Mads Kiilerich mads@kiilerich.com --- Just to make it clear:
My motivation for speaking up here is just to get good Ardour packaging, and to stay consistent with upstream "reality" so the packaging remains as good as possible.
Upstream considers all "major" and "minor" releases the same regarding new features, UI changes, risky refactorings ... and thus the risk of regressions. There are generally no low-risk "patch" releases (and thus never any updates that are suitable for existing Fedora releases). (7.1 might be an exception - it might come soon and be a "hotfix".)
*All* versions promise to be fully backwards compatible, so for example 7.0 can open back to (most) Ardour 2.0 sessions (and upgrade to the 7.x format).
"Major" .0 releases are *only* different in that they introduce a new file format, which remains forward compatible with all "minor" releases in that series (but can't be read by previous releases).
That means that there *could* be a potential use case for having 7.0 and 7.1 side-by-side and smoothly go back and forth if 7.1 in some way should be considered a regression. That use is *not* covered by the current scheme.
Having 6.9 and 7.0 side by side seems less relevant. While 7.0 can read (and update) everything 6.9 can read (and write), 6.9 can't read anything from 7.0 . There is thus not much use for having them installed side by side. I think it would be sufficient to pin / versionlock at the old release. But that use case *is* the one covered by the current scheme.
There is also a use case for making 7.0 available for preview, for example in the existing Fedora 36. With Fedora's 6 month release cycle, I don't think it is a big use case. I think that use case could be covered copr ... or by using binaries from upstream. But that use also seems to be a goal for the current scheme.
I would thus suggest to just let the "ardour" package move forward in rawhide, and generally not update it in released Fedora releases.
Other "compat" packages could be made on demand.
(In reply to Guido Aulisi from comment #4)
Having separate packages was almost a must when versione 3 was released. It was really incompatible with version 2 and the migration of the session file had some bad bugs.
Sure. No doubt there has been problems in the past where temporary desperate measures might have been justified.
It is hard to guess what bugs might show up in the future. I would guess that it is more likely that "minor" versions *without* session migrate introduce changes with problems with forward or backward compatibility.
I'm thus no big fan of letting previous unique experiences dictate current behaviour.
All software has bugs, and some releases turns out to be worse than others. One solution to that is to package *everything* in several versions. I understand that is kind of how Debian does it. But I prefer the Fedora way of having one canonical version of each package, where it is up to the packager to decide if it is sufficiently ready, and help out (or patch) to make it happen.
(In reply to Nils Philippsen from comment #5)
(In reply to Mads Kiilerich from comment #3)
I think that putting the first part for the version number in the package name is a bad idea. The package name should just be "ardour".
I think I remember we talked about this before, yeah.
(I don't think we have discussed it before. But others might have the same concern.)
The naming is done this way according to the guidelines for the reasons I’ll go into below: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#multiple
According to that: * One package SHOULD use the base name (with no version information). * The package version, which SHOULD include the periods present in the original version.
It should thus be for example ardour (with 7.0) and ardour6.9 . That would address my concern. (I still consider don't it relevant to have them side-by-side, but the 6.9 will not do any harm ;-) ).
Not using semantic versioning doesn’t preclude incompatible breaks between one major version and the next though…
Sure. But it also doesn't preclude incompatible breaks between two "minor" versions. Thus, if we want side-by-side packages, we must also be prepared for having for example 7.0 and 7.1 side by side.
For reference, here's the discussion on the mailing list: https://lists.fedoraproject.org/pipermail/music/2015-May/002005.html
Some comments to what is said there:
* "Installing "ardour" will always get you the latest available version" yes please! But that is not what we see now.
* "Every major version gets its own package" Ok. But 7.1 might in most regards be just as "major" as 7.0 was, so for consistency that one should get its own package too.
- Ardour 7 can read (and upgrade) Ardour 6 projects. There is no particular
good reason these two versions should be installed side-by-side. Supporting evidence: Upstream no longer makes Ardour 6 (easily) available for download.
This is all good in hindsight, but as I understand it, there are no guarantees that this will be the case for future major versions.
Nobody can guarantee that there never will be bugs or change of plans. But upstream explicitly state commitment to remain backwards compatible, going so far back that 7.0 can open (most) Ardour 2.0 sessions.
https://bugzilla.redhat.com/show_bug.cgi?id=2137159
--- Comment #10 from Mads Kiilerich mads@kiilerich.com --- Some review-review comments:
(In reply to Guido Aulisi from comment #6)
[x]: Package contains no bundled libraries without FPC exception.
If looking closely, there are some grey areas ...
[-]: If the package is a rename of another package, proper Obsoletes and Provides are present.
But the general expected user experience must be that this is an update to ardour6. It *is* thus a rename. After a version upgrade, people who had ardour6 installed, should have ardour7 installed instead. And if ardour7 gets introduced next to ardour6 in existing releases, I would expect it to be "advertised" (ie installed without replacing) if ardour 6 is installed. But that is tricky to achieve.
[x]: SourceX is a working URL.
(not really)
see attachment
That seems to have fallen out?
https://bugzilla.redhat.com/show_bug.cgi?id=2137159
--- Comment #11 from Mads Kiilerich mads@kiilerich.com --- (In reply to Nils Philippsen from comment #8)
Another case in point of having separate packages for different major versions is the changes in how tempo maps are implemented in 7.x vs. 6.x -- see https://tracker.ardour.org/view.php?id=9030#c26705
I don't know why paul commented like that. Reality is that it is a simple fix for a rare problem. You can apparently apply https://github.com/Ardour/ardour/commit/b195a04281cdd9352c010d3e5ea24ea03074... to fix it.
https://bugzilla.redhat.com/show_bug.cgi?id=2137159
--- Comment #12 from Gwyn Ciesla gwync@protonmail.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/ardour7
https://bugzilla.redhat.com/show_bug.cgi?id=2137159
Nils Philippsen nphilipp@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |NEXTRELEASE Status|ASSIGNED |CLOSED Last Closed| |2023-04-17 08:53:34
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com