Restore cross-patch-shebangs branch #49608
Merged
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
|
cc @LnL7 |
matthewbauer
force-pushed
the
cross-patch-shebangs-2
branch
from
11edcdd
to
21a236d
Compare
|
@GrahamcOfBorg build stdenv |
|
Success on aarch64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
|
Timed out, unknown build status on x86_64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
|
Timed out, unknown build status on x86_64-darwin (full log) Attempted: stdenv Partial log (click to expand)
|
Ericson2314
reviewed
Nov 1, 2018
| @@ -278,8 +278,8 @@ in rec { | |||
| # enables patchShebangs above. Unfortunately, patchShebangs ignores our $SHELL setting | |||
| # and instead goes by $PATH, which happens to contain bootstrapTools. So it goes and | |||
| # patches our shebangs back to point at bootstrapTools. This makes sure bash comes first. | |||
There was a problem hiding this comment.
Edit this comment to match the new reality, too? I also feel like if we can use your disallowRequisites patch with this, we can skip pkgs.bash in either list, which would be best. I'd be very down to collaborate getting all the stdenv deps to use strictDeps :).
There was a problem hiding this comment.
Yeah that sounds good! It might have to go in another PR for now - I'm more worried about fixing cross compilation right now.
One thing these are all going to need though is bash in the buildInputs. I think there's quite a few of those that don't have it right now.
I think we could also do an override here that does something like:
stdenv = super.stdenv // { mkDerivation = args: super.stdenv.mkDerivation (args // { strictDeps = true; }); };
To avoid individually setting all of these.
There was a problem hiding this comment.
Well I do want to audit those packages anyways. I think I've decided this PR and the disallowRequites one are good as is, and then all the per-package cleanup can happen in a 3rd PR.
This reverts commit 9c4b11e.
In strictDeps=false, autoPatchshebangs should use --build (corresponding to PATH) to lookup commands. This restores the previous behavior of patchshebangs so that we don’t break stuff that isn’t careful in the buildInputs vs. nativeBuildInputs distinction. Unfortunately this won’t work under cross compilation.
patch shebangs needs to be in build inputs for it to get into HOST_PATH.
matthewbauer
force-pushed
the
cross-patch-shebangs-2
branch
from
21a236d
to
068db20
Compare
matthewbauer
force-pushed
the
cross-patch-shebangs-2
branch
from
068db20
to
2f2e635
Compare
13 tasks
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.
Motivation for this change
This was merged, then reverted. This should address the issues that originally came up.