Skip to content

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

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

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.

Sorry, something went wrong.

@vrthra vrthra added 0.kind: enhancement Add something new 2.status: work-in-progress This PR isn't done 8.has: package (new) This PR adds a new package labels Feb 11, 2017
@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?

@peterhoeg peterhoeg force-pushed the u/clementine branch 2 times, most recently from d41fe33 to dc059fd Compare July 21, 2017 02:58
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.
@samueldr samueldr added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 11, 2018
@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
Labels
0.kind: enhancement Add something new 2.status: merge conflict This PR has merge conflicts with the target branch 2.status: work-in-progress This PR isn't done 8.has: package (new) This PR adds a new package 9.needs: community feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants