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.

Sign up for GitHub

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

radeon-profile: init at 20161221 #24359

Merged
merged 1 commit into from Apr 24, 2017
Merged

radeon-profile: init at 20161221 #24359

merged 1 commit into from Apr 24, 2017

Conversation

rnhmjoj
Copy link
Contributor

@rnhmjoj rnhmjoj commented Mar 26, 2017

Motivation for this change

nixpkgs is missing a tool for controlling the GPU settings.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using (none)
  • Tested execution of all binary files
  • Fits CONTRIBUTING.md.

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Mar 28, 2017 •
edited

Apparently after I rebased to current master it stopped working.

ldd ./result/bin/radeon-profile  | grep found
	libQt5PrintSupport.so.5 => not found
	libQt5Widgets.so.5 => not found
	libQt5Gui.so.5 => not found
	libQt5Network.so.5 => not found
	libQt5Core.so.5 => not found

Any idea?

@ttuegel
Copy link
Member

ttuegel commented Mar 30, 2017

This package is not built from source, is that correct?

To make it work, you must at least use makeQtWrapper; see the Nixpkgs manual for details.

ttuegel
ttuegel reviewed Mar 30, 2017
View reviewed changes
pkgs/tools/misc/radeon-profile/default.nix Outdated
sha256 = "0zdmpc0rx6i0y32dcbz02whp95hpbmmbkmcp39f00byvjm5cprgg";
}) + "/radeon-profile";

installPhase = ''
Copy link
Member

Choose a reason for hiding this comment

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

To override a phase like this, the first line must be runHook preInstall and the last line must be runHook postInstall.

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Mar 30, 2017

This package is not built from source, is that correct?

No, it's actually built from source but the binary must be manually installed once built.

To override a phase like this, the first line must be runHook preInstall and the last line must be runHook postInstall.

Thanks: I moved the commands into postInstall

@rnhmjoj rnhmjoj force-pushed the radeon-profile branch 2 times, most recently from c9d0619 to b81476d Compare March 30, 2017 15:37
@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Apr 9, 2017

Merge?

@matthiasbeyer
Copy link
Contributor

Just tested this and it seems to work on my desktop machine. Please merge, I'd love to see this in nixpkgs!

bennofs
bennofs reviewed Apr 23, 2017
View reviewed changes
pkgs/tools/misc/radeon-profile/default.nix Outdated
@@ -0,0 +1,30 @@
{ stdenv, fetchFromGitHub, qt5, xorg }:
Copy link
Contributor

Choose a reason for hiding this comment

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

From the nixpkgs manual (http://nixos.org/nixpkgs/manual/#ssec-qt-applications):

Dependencies should be imported unqualified, i.e. qtbase not qt5.qtbase. Do not import a package set such as qt5 or libsForQt5 into your package; although it may work fine in the moment, it could well break at the next Qt update.

@bennofs
Copy link
Contributor

bennofs commented Apr 23, 2017

@ttuegel is makeQtWrapper required even if a package is build from source?

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Apr 23, 2017 •
edited

There are several packages built from source using makeQtWrapper. The program seems to run ok without it though.

@ttuegel
Copy link
Member

ttuegel commented Apr 24, 2017

@ttuegel is makeQtWrapper required even if a package is build from source?

Yes, absolutely. Especially if the package is built from source.

It will sometimes work without it, e.g. if you are running from KDE.

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Apr 24, 2017

Ok, added makeQtWrapper.

@bennofs
Copy link
Contributor

bennofs commented Apr 24, 2017

Thanks, looks good now!

@bennofs bennofs merged commit 30a9923 into NixOS:master Apr 24, 2017
@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Apr 24, 2017

Thank you

@rnhmjoj rnhmjoj deleted the radeon-profile branch September 12, 2017 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ttuegel ttuegel ttuegel left review comments

@bennofs bennofs bennofs left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@rnhmjoj @ttuegel @matthiasbeyer @bennofs