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

Jump to bottom

picard: 2.2.1 -> 2.2.2 #70854

Merged
merged 1 commit into from Oct 20, 2019
Merged

picard: 2.2.1 -> 2.2.2 #70854

merged 1 commit into from Oct 20, 2019

Conversation

samdoshi
Copy link
Contributor

@samdoshi samdoshi commented Oct 9, 2019

Motivation for this change

Picard update from 2.2.1 to 2.2.2.

I haven't added qtmultimedia to enable to the experimental player.

Also acoustic fingerprinting is currently broken as Picard requires an updated version of chromaprint (this was also the case with 2.2.1). See #60137 for the chromaprint update.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @ehmry

jonringer
jonringer reviewed Oct 9, 2019
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

while running it, i still got this:

[nix-shell:/home/jon/.cache/nix-review/pr-70854]$ ./results/picard/bin/picard
W: 08:34:35,422 /nix/store/msx0qv3cmay9j805cxj1ypxfrixly9qr-picard-2.2.2/lib/python3.7/site-packages/picard/ui/playertoolbar.__init__:98: Internal player: unavailable, cannot import name 'QtMultimedia' from 'PyQt5' (/nix/store/whvma9712dk1csy9lhr158kf13n0wb03-python3.7-PyQt5.sip-4.19.18/lib/python3.7/site-packages/PyQt5/__init__.py)

@samdoshi
Copy link
Contributor Author

Hi, thanks for taking the time to review, that error is related to the experimental built-in player, the warning also exists with the current version:

$ nix-shell -p picard --run picard
Warning: config file 2.2.2.final0 was created by a more recent version of Picard (current is 2.2.1.final0)
W: 07:31:26,001 /nix/store/09zh7ygsg93157r8fdk4jgjp0l81l0mp-picard-2.2.1/lib/python3.7/site-packages/picard/ui/playertoolbar.__init__:98: Internal player: unavailable, cannot import name 'QtMultimedia' from 'PyQt5' (/nix/store/6hd6aqf18cw3gsmmfaypdn6ldd8ygk86-python3.7-PyQt5.sip-4.19.18/lib/python3.7/site-packages/PyQt5/__init__.py)

It is easy enough to get rid of the warning by adding qtmultimedia to the derivation:

index dc1118f47e0..fc49e91d414 100644
--- a/pkgs/applications/audio/picard/default.nix
+++ b/pkgs/applications/audio/picard/default.nix
@@ -13,10 +13,10 @@ in pythonPackages.buildPythonApplication rec {
     sha256 = "1iibkvwpj862wcrl0fmyi6qhcgx4q5ay63yr0zyg0bkqgcka0gpr";
   };
 
-  nativeBuildInputs = [ gettext qt5.wrapQtAppsHook qt5.qtbase ];
+  nativeBuildInputs = [ gettext qt5.wrapQtAppsHook qt5.qtbase qt5.qtmultimedia ];
 
   propagatedBuildInputs = with pythonPackages; [
-    pyqt5
+    pyqt5_with_qtmultimedia
     mutagen
     chromaprint
     discid

Then you'll be able to enable the player toolbar, however it doesn't work properly (no-audio, no progress bar). I can't tell if this is an issue with NixOS, or with Picard. Either way, it is explicitly an optional feature (e.g. Debian and Arch Linux both list the dependency on qtmultimedia as optional).

There is some info on the player toolbar here: https://tickets.metabrainz.org/browse/PICARD-1488

At some point I might try debugging the feature to get it to work, but so far it's not even producing anything in the debug logs, so it might be a while...

Lassulus
Lassulus approved these changes Oct 15, 2019
View reviewed changes
Copy link
Member

@Lassulus Lassulus left a comment

Choose a reason for hiding this comment

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

tested with nix-review, clicked around a little bit

c0bw3b
c0bw3b approved these changes Oct 20, 2019
View reviewed changes
Copy link
Contributor

@c0bw3b c0bw3b left a comment

Choose a reason for hiding this comment

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

Builds and runs.
chromaprint 1.4.3 now provides a compatible fpcalc

@c0bw3b c0bw3b merged commit f8ef6e5 into NixOS:master Oct 20, 2019
@samdoshi samdoshi deleted the picard branch April 9, 2020 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonringer jonringer jonringer left review comments

@Lassulus Lassulus Lassulus approved these changes

@c0bw3b c0bw3b c0bw3b approved these changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@samdoshi @Lassulus @jonringer @c0bw3b