Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ardour: 6.9 -> 7.1 #196290

Merged
merged 6 commits into from
Nov 9, 2022
Merged

ardour: 6.9 -> 7.1 #196290

merged 6 commits into from
Nov 9, 2022

Conversation

anoadragon453
Copy link
Member

@anoadragon453 anoadragon453 commented Oct 16, 2022

Description of changes

Release notes for 7.1: https://ardour.org/whatsnew.html (long-term link: https://ardour.org/news/7.1.html)

Note: I retrieved the sha256 hash by setting it to "" and then taking the expected sum upon running the derivation. It doesn't match the same number of characters as the old hash though, so please double-check. I was confused between a base32 and base64 hashes. Have updated to use the hash keyword.

Things done

Tested:

  • Importing multiple songs and playing them via JACK
  • Messing around with the new clip launcher
  • Exporting a song to flac

Built on platform(s)

  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Oct 16, 2022
Copy link
Member

@magnetophon magnetophon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know about the release until I saw the notification for this PR.
Thanks for the happy surprise, works a charm!

I would like to keep 6.9 available for a while, both since 7.0 is less well tested (I already had a crash) and because 7.0 introduces a new file format.

Could you add an entry in all-packages.nix for both ardour6 and ardour7, and point ardour to ardour7?

@anoadragon453
Copy link
Member Author

@magnetophon Right, I think I've done the right thing? Let me know otherwise 😁

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Oct 16, 2022
@ofborg ofborg bot requested a review from magnetophon October 16, 2022 21:04
Copy link
Member

@magnetophon magnetophon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@magnetophon magnetophon added the 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package label Oct 17, 2022
@magnetophon
Copy link
Member

Oh, sorry, I just noticed it's 2 separate commits.
Could you squash them into one?

@anoadragon453
Copy link
Member Author

@magnetophon have squashed!

@ofborg ofborg bot requested a review from magnetophon October 17, 2022 07:47
Copy link
Member

@magnetophon magnetophon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@anoadragon453
Copy link
Member Author

(Just so you're aware, I don't have the ability to merge this, so someone else will have to.)

@magnetophon
Copy link
Member

Me neither! :)

@egasimus
Copy link
Contributor

Who else is looking forward to the new clip launcher feature? :-)

@magnetophon magnetophon added 8.has: package (update) This PR updates a package to a newer version and removed 8.has: package (new) This PR adds a new package labels Oct 18, 2022
@Vivien-lelouette
Copy link

I can't wait to try it out :)

@winterqt
Copy link
Member

@ofborg build ardour6

pkgs/applications/audio/ardour/ardour6.nix Outdated Show resolved Hide resolved
pkgs/applications/audio/ardour/ardour6.nix Outdated Show resolved Hide resolved
pkgs/top-level/all-packages.nix Show resolved Hide resolved
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
pkgs/applications/audio/ardour/default.nix Outdated Show resolved Hide resolved
@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Oct 20, 2022
@ofborg ofborg bot requested a review from magnetophon October 20, 2022 00:14
@magnetophon magnetophon mentioned this pull request Oct 25, 2022
13 tasks
@SuperSandro2000
Copy link
Member

Please use a web.archive.org link for unversioned downloads like https://web.archive.org/web/20221026200824/http://stuff.ardour.org/loops/ArdourBundledMedia.zip

@pyrotek45
Copy link

pyrotek45 commented Nov 3, 2022

7.1 should be out tomorrow (according to paul davis) , this release is quite buggy, it might be worth skipping to 7.1 anyways.

@anoadragon453
Copy link
Member Author

Thanks for the heads up @pyrotek45. I'll update this PR to point to 7.1 instead.

@magnetophon and others, do we still want to keep ardour 6.9 around now that 7.1 is out? Is 7.1 considered a stable enough release to drop 6.9?

@magnetophon
Copy link
Member

@anoadragon453 I've been running 7.1 for a little bit and while it seems better, I think it's good to leave 6.9 in for a couple of months, so people can finish any running projects they have in 6.9 and transfer to 7.x when they are done.
If others have strong feelings on removing 6.9, I'd be (reluctantly) OK with that as well.

Release notes for 7.1: https://ardour.org/whatsnew.html
(long-term link: https://ardour.org/news/7.1.html)

Additionally create an 'ardour6' package for those that may wish to
stay on a more stable release. To potentially be removed once 7.x
stabilises.
Also fix a small formatting error in the import statements.
@anoadragon453 anoadragon453 changed the title ardour: 6.9 -> 7.0 ardour: 6.9 -> 7.1 Nov 6, 2022
@magnetophon
Copy link
Member

@anoadragon453 That doesn't build for me, I get:

Automatic merge went well; stopped before committing as requested
$ nix-env --option system x86_64-linux -f /home/bart/.cache/nixpkgs-review/pr-196290-5/nixpkgs -qaP --xml --out-path --show-trace --meta
1 package added:
ardour_6 (init at 6.9)

1 package updated:
ardour (6.9 → 7.1)

$ nix --experimental-features nix-command build --no-link --keep-going --option build-use-sandbox relaxed -f /home/bart/.cache/nixpkgs-review/pr-196290-5/build.nix
error: build of '/nix/store/f96dn18w8scqs3imsk5ccb27201cf9k6-ardour-7.1.drv' on 'ssh://nixBuild@builder' failed: builder for '/nix/store/f96dn18w8scqs3imsk5ccb27201cf9k6-ardour-7.1.drv' failed with exit code 1;
       last 10 log lines:
       > removed '/nix/store/wmcsp68ckzidh5b1sxfx17bcpx9ss6qp-ardour-7.1/share/icons/hicolor/256x256/apps/ardour7.png'
       > 'gtk2_ardour/resources/Ardour-icon_256px.png' -> '/nix/store/wmcsp68ckzidh5b1sxfx17bcpx9ss6qp-ardour-7.1/share/icons/hicolor/256x256/apps/ardour7.png'
       > removed '/nix/store/wmcsp68ckzidh5b1sxfx17bcpx9ss6qp-ardour-7.1/share/icons/hicolor/512x512/apps/ardour7.png'
       > 'gtk2_ardour/resources/Ardour-icon_512px.png' -> '/nix/store/wmcsp68ckzidh5b1sxfx17bcpx9ss6qp-ardour-7.1/share/icons/hicolor/512x512/apps/ardour7.png'
       > install: creating directory '/nix/store/wmcsp68ckzidh5b1sxfx17bcpx9ss6qp-ardour-7.1/share/man'
       > install: creating directory '/nix/store/wmcsp68ckzidh5b1sxfx17bcpx9ss6qp-ardour-7.1/share/man/man1'
       > 'ardour.1' -> '/nix/store/wmcsp68ckzidh5b1sxfx17bcpx9ss6qp-ardour-7.1/share/man/man1/ardour.1'
       > 'MIDI Beats'  'MIDI Chords'  'MIDI Progressions'
       > install: creating directory '/nix/store/wmcsp68ckzidh5b1sxfx17bcpx9ss6qp-ardour-7.1/share/media'
       > install: omitting directory '/nix/store/vzmx96hjs7k1czfb0c8akpfzj8wbxac6-source'
       For full logs, run 'nix log /nix/store/f96dn18w8scqs3imsk5ccb27201cf9k6-ardour-7.1.drv'.

@anoadragon453
Copy link
Member Author

@magnetophon Mmm yes, because 69c3b46 is still included. Happy to drop it if we think we should add the bundled content in a separate PR (#196290 (comment)).

@magnetophon
Copy link
Member

@magnetophon Mmm yes, because 69c3b46 is still included. Happy to drop it if we think we should add the bundled content in a separate PR (#196290 (comment)).

The Ardour devs worked hard to be able to include this content, I think we should try to present the users the full version.
@cillianderoiste @mitchmindtree What do you think?

@anoadragon453
Copy link
Member Author

Note that I'm happy to include it -- but will need some guidance on writing the nix needed to do so :)

@magnetophon
Copy link
Member

Note that I'm happy to include it -- but will need some guidance on writing the nix needed to do so :)

I just got it to install by replacing the install command by:

    mkdir -p "$out/share/media"
    cp -rp "${bundledContent}"/* "$out/share/media"

The media are in $out/share/media, but they do not show up in ardour yet. :/

@anoadragon453
Copy link
Member Author

To be explicit: I believe this is ready to be merged.

@magnetophon
Copy link
Member

@anoadragon453 For me, the bundled content does not show up in ardour.

@xlambein
Copy link
Contributor

xlambein commented Nov 9, 2022

@magnetophon I just tried it out, it does work.

image

Copy link
Member

@magnetophon magnetophon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it works here too.
Thanks a lot for your patience and persistence @anoadragon453!

@SuperSandro2000 SuperSandro2000 merged commit ef527a4 into NixOS:master Nov 9, 2022
rtimush pushed a commit to rtimush/nixpkgs that referenced this pull request Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 8.has: package (update) This PR updates a package to a newer version 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants