Skip to content

stdenv: load default inputs and functions before setup hooks #26844

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
Closed
wants to merge 1 commit into from
Closed

stdenv: load default inputs and functions before setup hooks #26844

wants to merge 1 commit into from
+112 −109

Conversation

ttuegel
Copy link
Member

@ttuegel ttuegel commented Jun 25, 2017

The default build inputs are loaded first so that functions and variables
defined in their setup hooks can be used in setup hooks defined by later build
inputs.

The stripHash function and the text substitution functions are defined before
any build inputs are loaded to allow their use in setup hooks.

The goal of both changes is to run setup hooks in an environment closer to the
actual build environment.

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 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

@ttuegel
stdenv: load default inputs and functions before setup hooks

Verified

This commit was signed with the committer’s verified signature.
ttuegel Thomas Tuegel
GPG key ID: 22CBF5249D4B4D59
Verified
Learn about vigilant mode
57802d2 
The default build inputs are loaded first so that functions and variables
defined in their setup hooks can be used in setup hooks defined by later build
inputs.

The stripHash function and the text substitution functions are defined before
any build inputs are loaded to allow their use in setup hooks.

The goal of both changes is to run setup hooks in an environment closer to the
actual build environment.
@ttuegel ttuegel added 0.kind: enhancement Add something new 1.severity: mass-rebuild This PR causes a large number of packages to rebuild labels Jun 25, 2017
@ttuegel ttuegel requested review from edolstra and vcunat June 25, 2017 14:17
@mention-bot
Copy link

@ttuegel, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @urkud and @vcunat to be potential reviewers.

@ttuegel
Copy link
Member Author

ttuegel commented Jun 25, 2017

Build fails with

xz -c -5 < "$inputfile" > archive.dir.tar.xz-t && mv archive.dir.tar.xz-t archive.dir.tar.xz
unxz: unrecognized option: 5
Makefile:2221: recipe for target 'archive.dir.tar.xz' failed
make[4]: *** [archive.dir.tar.xz] Error 1
make[4]: Leaving directory '/tmp/nix-build-gettext-0.19.8.drv-0/gettext-0.19.8/gettext-tools/misc'
Makefile:2026: recipe for target 'all-recursive' failed
make[3]: *** [all-recursive] Error 1
make[3]: Leaving directory '/tmp/nix-build-gettext-0.19.8.drv-0/gettext-0.19.8/gettext-tools'
Makefile:1892: recipe for target 'all' failed
make[2]: *** [all] Error 2
make[2]: Leaving directory '/tmp/nix-build-gettext-0.19.8.drv-0/gettext-0.19.8/gettext-tools'
Makefile:413: recipe for target 'all-recursive' failed
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory '/tmp/nix-build-gettext-0.19.8.drv-0/gettext-0.19.8'
Makefile:369: recipe for target 'all' failed
make: *** [all] Error 2
builder for ‘/nix/store/05mzxyj2cm73ixsrlv4f6swxvw7p5yv4-gettext-0.19.8.drv’ failed with exit code 2

which does not seem to be related to my changes?

@vcunat
Copy link
Member

vcunat commented Jun 26, 2017

Oh, I see now some problems that I didn't realize when I suggested the changes. I actually think it's a directly caused error. (there are probably different xz versions on $PATH at once at that point)

The problem here is the order in variables like $PATH. Currently we append in addToSearchPathWithCustomDelimiter, so the first input has the highest priority – in there we probably do want the first user input going first in $PATH and the last stdenv-induced package to go last. However, the setup hook actions seem to make most sense to get run in "reverse" order, at least in the sense that stdenv hooks are run before user hooks.

@ttuegel
Copy link
Member Author

ttuegel commented Jun 26, 2017

The problem here is the order in variables like $PATH. Currently we append in addToSearchPathWithCustomDelimiter, so the first input has the highest priority – in there we probably do want the first user input going first in $PATH and the last stdenv-induced package to go last. However, the setup hook actions seem to make most sense to get run in "reverse" order, at least in the sense that stdenv hooks are run before user hooks.

OK. In that case, I'm happy to continue stuffing my setup hooks into postHook.

@ttuegel ttuegel closed this Jun 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fpletz fpletz

@edolstra edolstra

@vcunat vcunat

Assignees
No one assigned
Labels
0.kind: enhancement Add something new 1.severity: mass-rebuild This PR causes a large number of packages to rebuild
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ttuegel @mention-bot @vcunat @fpletz