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

wrap-qt-apps-hook.sh: wrap binaries in ./sbin/ #74990

Merged
merged 1 commit into from Dec 8, 2019

Conversation

@bjornfor
Copy link
Contributor

@bjornfor bjornfor commented Dec 4, 2019

Motivation for this change

wrap-qt-apps-hook.sh runs before move-sbin.sh, so programs in ./sbin are
currently not wrapped. Fix it.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 @

wrap-qt-apps-hook.sh runs before move-sbin.sh, so programs in ./sbin are
currently not wrapped. Fix it.
@flokli
Copy link
Contributor

@flokli flokli commented Dec 4, 2019

Can we change the order of how it's run?

@FRidh
Copy link
Member

@FRidh FRidh commented Dec 4, 2019

How I wish we had no explicit order for phases and hooks and instead used something like multiple dispatch to determine the ordering.

@worldofpeace
Copy link
Member

@worldofpeace worldofpeace commented Dec 4, 2019

We have the same issue with wrapGAppsHook because of ordering. I've just taken to having people not install things in sbin.

I think there won't be a problem with things becoming double wrapped hopefully (because checking for elf interpreter).

@ttuegel
ttuegel approved these changes Dec 8, 2019
@bjornfor bjornfor changed the base branch from master to staging Dec 8, 2019
@bjornfor bjornfor merged commit 9e144d5 into NixOS:staging Dec 8, 2019
11 of 13 checks passed
11 of 13 checks passed
grahamcofborg-eval Beginning Evaluations
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@bjornfor bjornfor deleted the bjornfor:wrap-qt-apps-hook-in-sbin branch Dec 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.