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

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

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

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.

@ttuegel
stdenv: load default inputs and functions before setup hooks
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 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 fpletz approved these changes

@edolstra edolstra Awaiting requested review from edolstra

@vcunat vcunat Awaiting requested review from vcunat

Assignees
No one assigned
Labels
0.kind: enhancement 1.severity: mass-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