-
-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Unbreak stdenv #27650
Unbreak stdenv #27650
Conversation
@Ericson2314, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @Profpatsch and @vcunat to be potential reviewers. |
Unfortunately |
pkgs/stdenv/generic/setup.sh
Outdated
[[ "$#" -gt 0 ]] || return 0 | ||
printf '%s\n' "$@" | ||
} | ||
|
||
printWords() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you have a duplicated definition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Err I meant to keep printLines
, but I guess it's now unused so there's no point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, haha, just realized my find & replace renamed the old one.
…files We cannot switch to line-delimited yet, because certain Nix commands do not read in the entire file, but just the first line.
This is a temporary measure until this impurity is removed from Nix.
This is invalid before bash-4.2, affecting bash used impurely in nix-shell on MacOS.
1df0b2f
to
820e402
Compare
@@ -211,7 +211,7 @@ stdenv.mkDerivation ({ | |||
configureFlags="${concatStringsSep " " defaultConfigureFlags} $configureFlags" | |||
|
|||
# nativePkgs defined in stdenv/setup.hs | |||
for p in "''${!nativePkgs[@]}"; do | |||
for p in "''${nativePkgs[@]}"; do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was that bang?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The keys of the map, instead of the values. (With arrays, really int maps, it gives you the indices.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's okay but haven't tested
Ok it builds and I tested |
Sure! |
Motivation for this change
Solves the
nix-env
and Darwinnix-shell
issues. No evals so far! After this I'll merge with master, basically with-Xours
.Things done
Please check what applies. Note that these are not hard requirements but mereley serve as information for reviewers.
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell
on Darwin -- old version workednix-shell -p nox --run "nox-review wip"
./result/bin/
)