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

[WIP] clementine-qt5: init at 1.3.1 - call for testers #22366

Closed
wants to merge 1 commit into from

Conversation

peterhoeg
Copy link
Member

Motivation for this change

Running qt4 applications on a hidpi screen isn't always a great experience.

Clementine based on qt5 has been in the works for some time.

Known issue:

  • spotify support is broken

While clementine is supposedly cross platform, the port to qt5 only works on
linux for now but upstream is working on it.

I'd love to hear some feedback from testers before we actually merge this.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@peterhoeg
Copy link
Member Author

@ttuegel are you OK to merge this in its current incarnation?

Copy link
Member

@ttuegel ttuegel left a comment

Choose a reason for hiding this comment

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

This looks fine, except for the makeWrapper vs. makeQtWrapper issue.


runCommand "clementine-${version}" {
inherit free blob;
buildInputs = [ makeWrapper ] ++ gst_plugins; # for the setup-hooks
Copy link
Member

Choose a reason for hiding this comment

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

This should use makeQtWrapper here and below. Since it has KDE 5 dependencies, you also need ecm here for the setup hooks.

${optionalString withSpotify "--set CLEMENTINE_SPOTIFYBLOB \"$blob/libexec/clementine\""} \
--prefix GST_PLUGIN_SYSTEM_PATH : "$GST_PLUGIN_SYSTEM_PATH"

mkdir -p $out/share
Copy link
Member

Choose a reason for hiding this comment

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

Adding ecm to buildInputs and using makeQtWrapper should make this unnecessary.

@peterhoeg
Copy link
Member Author

@ttuegel, couple of things:

Now the qt4 and qt5 versions are both compiling and launching correctly at the latest version (1.3.1) but they are both crashing on playback with a gstreamer assertion (see clementine-player/Clementine#5753).

  1. Are you otherwise OK to merge this when that is fixed?
  2. Should we keep the qt4 as well or simply blow that away in favour of the qt5 version?

Also update 1.2.3 -> 1.3.1 for the qt4 version.

Currently crashes when trying to play any file which I guess is related to gstreamer.
@Ma27
Copy link
Member

Ma27 commented Mar 27, 2019

clementine is now at 1.3.1 and uses qt5, so I guess we can close this now.

@Ma27 Ma27 closed this Mar 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants