xz: set CONFIG_SHELL to /bin/sh, fix retained reference to bootstrap #34916
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Using CONFIG_SHELL since that's what is checked first. Don't force the shell to use, let xz test what it finds and come to its own conclusions about suitability. As-is our stdenv fails to build with Nix 2.0 due to references in xz's scripts to bootstrap tools' "sh". Try `nix-build -A xz --check` and look at what configure detects for the POSIX-compat shell: it needs to be /bin/sh, and if it instead is a path with the word "bootstrap" then you've reproduced.
GrahamcOfBorg
added
10.rebuild-darwin-stdenv
|
Success on aarch64-linux (full log) Partial log (click to expand)
|
|
Failure on x86_64-linux (full log) Partial log (click to expand)
|
|
Failure on x86_64-darwin (full log) Partial log (click to expand)
|
|
Can't reproduce nixpkgs 2e4aded I see no references to "bootstrap". Probably this only occurs for musl libc? I see similar fixes in nixpkgs though |
|
@danbst did you build those paths yourself (using nix 1.12/2) or did you fetch them from a cache? You'll need to build yourself to see this. Don't believe it has anything to do with musl other than we don't rebuild bootstrap tools often and so things like this can sneak in unnoticed. I say this because before filing this PR I tested using nixpkgs master to confirm it wasn't. ... Although it'd be good to see someone else reproduce to be sure it's not dtz-specific :). Thanks for the link to the commit, I thought I remembered this being an issue. Hmm. |
|
Yes, I've tried cached and local results: |
|
Hmm, thanks. 1.12 might not be new enough, including corresponding NixOS changes made for 2.0 that move to using a busybox-based /bin/sh instead of a more complete bash. (Although to be honest I don't quite follow how that change caused this exactly but it seems much too related to be a coincidence :)). Can someone already running Nix 2.0 using NixOS master give this a go? |
|
Alright, another attempt to reproduce this. You should see same result running locally, otherwise something impure is happening (well yes, My NixOS is 18.03.pre.e860b65, not quite master though. Which nixos commits are you talking about? |
|
@danbst mostly I was referring to 700e21d. Your nixpkgs seems plenty new. Thanks for the invocation goodness! That command produces no output for me since the |
|
Ok, I'll try to update to that commit, 2.0 stuff should be so cool. In meanwhile I've stopped my nix-daemon and launched 2.0 one inside nix-shell - xz still builds for me. |
|
Thank you for your heroic efforts. In an attempt to help I asked on IRC and @lheckemann was kind enough to try with latest Nix/NixOS. I believe he found that --check failed at least sometimes but that it still used /bin/sh not the bootstrap shell as I'm experiencing. Meanwhile I'm still observing this behavior, currently using:
I'm really not sure why I'd be seeing different behavior than you guys, but if no one else can reproduce ("until" they can) then that's good news for everyone :). Well, except me since I'm not sure I'll be able to let this go until I sort out what's going on. |
|
This was with
building nixpkgs b1ba405; the result was that xz is apparently not deterministic (seems to vary depending on whether the sandbox is used or not. I also tried with the sandbox off, which produced the same output as the binary cache). |
|
I have now which is similar to @lheckemann and @dtzWill Though I see now that This kind of irreproducibility makes me sad, because Nix is all about reproducibility... |
|
@dtzWill can you post your error log? |
|
Allright, I see some progress. I've tried |
|
So, I understand like this: nix builds xz locally, but doesn't replace the derivation in store with newly built one (because those should be identical, why bother replacing?). So @dtzWill and I both have different packages with same name |
|
Still don't get why previous "fix" ( CONFIG_SHELL unset, but SHELL is not. Why doesn't it influence build result? Probably because when |
danbst
approved these changes
Feb 13, 2018
|
I have the same confusion, although I like your explanation. |
3 tasks
|
FWIW this was merged as part of the musl PR, closing for now. Thanks everyone! |
|
I've dig deeper here. So here's what happens. Both with and without sandbox SHELL is initialized to some bash. Now, what happens when CONFIG_SHELL is unset?
Probably another instance of what #34628 should fix, but I'm not sure |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
stdenv should be buildable cleanly with new Nix!
I ran into this while working on the musl PR (#34645)
but this is reproducible on master with Nix 2
(AFAIK not with Nix 1.11).
Please review and test. And yes this will require rebuilding all the things.
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)Using CONFIG_SHELL since that's what is checked first.
Don't force the shell to use, let xz test what it finds
and come to its own conclusions about suitability.
As-is our stdenv fails to build with Nix 2.0 due to references
in xz's scripts to bootstrap tools' "sh".
Try
nix-build -A xz --checkand look at what configure detectsfor the POSIX-compat shell: it needs to be /bin/sh, and if
it instead is a path with the word "bootstrap" then you've reproduced.