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

nix: fix stdenv.system check -- should be hostPlatform #39666

Merged
merged 1 commit into from May 1, 2018
Merged

nix: fix stdenv.system check -- should be hostPlatform #39666

merged 1 commit into from May 1, 2018

Conversation

dtzWill
Copy link
Member

@dtzWill dtzWill commented Apr 29, 2018

(stdenv.system is the system of the builder, for ... reasons)

Fixes cross from or to armv6l.

  • 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.

@dtzWill
nix: fix stdenv.system check -- should be hostPlatform
6e8118b 
(stdenv.system is the system running the build, apparently)
@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: nix

Partial log (click to expand)

  /nix/store/zjgm3wna02y1rmsmnyqn923kdwlyk7jj-libatomic_ops-7.6.4
copying path '/nix/store/nh08vz3fw5j5g3xhhzsl5hxy14f6ygvd-busybox-1.28.3' from 'https://cache.nixos.org'...
copying path '/nix/store/qbggnz69ddby4azz24n4scfr3d4g5p0s-nix-2.0.1-man' from 'https://cache.nixos.org'...
copying path '/nix/store/y6dkzf0099c14fx424pz342q2c0r7vkr-brotli-1.0.3-lib' from 'https://cache.nixos.org'...
copying path '/nix/store/zjgm3wna02y1rmsmnyqn923kdwlyk7jj-libatomic_ops-7.6.4' from 'https://cache.nixos.org'...
copying path '/nix/store/asi9sck26ycrqjsc8xp5fq65sk7d437r-aws-sdk-cpp-1.4.33' from 'https://cache.nixos.org'...
copying path '/nix/store/bfh5siq8xm6nxr5dcgvcnds0z79h4xsk-boehm-gc-7.6.4' from 'https://cache.nixos.org'...
copying path '/nix/store/fsvixkxm9vxq28xm6csqyrq93dx9a8va-libsodium-1.0.16' from 'https://cache.nixos.org'...
copying path '/nix/store/vs0xgljqgak368jrylalsl4b73p74ajx-nix-2.0.1' from 'https://cache.nixos.org'...
/nix/store/vs0xgljqgak368jrylalsl4b73p74ajx-nix-2.0.1

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: nix

Partial log (click to expand)

/nix/store/jp9am3c1zkg79zp197wk47229p7ch47d-nix-2.0.1

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: nix

Partial log (click to expand)

running test tests/fetchGit.sh... [SKIP]
running test tests/fetchMercurial.sh... [SKIP]
running test tests/signing.sh... [PASS]
running test tests/run.sh... [PASS]
running test tests/brotli.sh... [PASS]
running test tests/pure-eval.sh... [PASS]
running test tests/check.sh... [PASS]
running test tests/plugins.sh... [PASS]
All tests succeeded
/nix/store/vsy8i8llzadg8dafbv1h2fsbj25rx89w-nix-2.0.1

@dezgeg
Copy link
Contributor

dezgeg commented Apr 29, 2018

Sounds like something that could be fixed in stdenv as well, given that isx86_64 et al already refer to hostPlatform ones.

@dtzWill
Copy link
Member Author

dtzWill commented Apr 30, 2018

Sounds like something that could be fixed in stdenv as well, given that isx86_64 et al already refer to hostPlatform ones.

I think leaving it this way was done "on purpose" to accommodate something; I agree that it seems like a stdenv bug to have stdenv.isFoo not describe the system in stdenv.system, but I figured I'd save that battle for a different day :D.

cc @Ericson2314 who may know more.

@Ericson2314
Copy link
Member

Yes because stdenv is a derivation, stdenv.system must be the build platform. It's unfortunate.

@Ericson2314
Copy link
Member

Ericson2314 commented May 1, 2018 •
edited

See the checklist in #27069

@dtzWill
Copy link
Member Author

dtzWill commented May 1, 2018

Okay, thanks! Just wanted to be sure, and thanks for the reference :).

@dtzWill dtzWill merged commit 47b2513 into NixOS:master May 1, 2018
@dtzWill dtzWill deleted the fix/stdenv-system-nix branch May 1, 2018 01:52
@dezgeg
Copy link
Contributor

dezgeg commented May 1, 2018

Well, there is no real requirement of having the system attribute in the builder to be same as the attribute that's reachable via stdenv.system.

@Ericson2314
Copy link
Member

Indeed, but people would probably assume that e.g. stdenv.isLinux matches stdenv.system, and that's not true.

@dezgeg
Copy link
Contributor

dezgeg commented May 15, 2018

Uh, I'm confused now. stdenv.isLinux currently does not match stdenv.system. I proposed to make them match.

@Ericson2314
Copy link
Member

Ericson2314 commented May 16, 2018 •
edited

@dezgeg Ohhh you are saying that we do a passthru (or similar) to hide the stdenv.system the builder sees? sounds good to me. 👍. Any way we can completely remove stdenv.system except for the derivation? I'd like that even more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@dtzWill @GrahamcOfBorg @dezgeg @Ericson2314