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

stdenv setup: Run setup hooks and other processing after accumulating deps #31723

Merged
merged 1 commit into from Nov 16, 2017

Conversation

Ericson2314
Copy link
Member

Motivation for this change

Dependencies are now just accumulated in findInputs, and and their setup hooks and other side-effectual stuff run after. Some packages, along with work by @sjmackenzie IIRC, used findInputs separately. I'm a bit dubious of this in general, but this makes it better by having findInputs just, you know, find the inputs. In any event, I find the separation of concerns more readable, especially once I complicate the propagation logic in my yet-to-be-merged cross work.

This is a part of #31414 that currently actually works, heh.

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

… deps

I find the separation of concerns, accumulating, then processing, easier
to follow. Also, with my yet-to-be-merged cross work, the accumulation
part will become more complex.
@Ericson2314 Ericson2314 added this to Needed for binutils-wrapper in Cross compilation Nov 16, 2017
@Ericson2314
Copy link
Member Author

@nixborg build

@nixborg
Copy link

nixborg commented Nov 16, 2017

Jobset created at https://hydra.mayflower.de/jobset/nixos/pr-31723

Copy link
Contributor

@orivej orivej left a comment

Choose a reason for hiding this comment

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

The code looks good, let's wait for the hydra and merge.

@Ericson2314 Ericson2314 merged commit b26038f into NixOS:staging Nov 16, 2017
@Ericson2314 Ericson2314 deleted the stdenv-accum-them-setup branch November 16, 2017 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Cross compilation
Needed for binutils-wrapper
Development

Successfully merging this pull request may close these issues.

None yet

4 participants