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
cc-wrapper, cc-wrapper-old: Simplify shell logic #29550
cc-wrapper, cc-wrapper-old: Simplify shell logic #29550
Conversation
@Ericson2314, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @gridaphobe and @vcunat to be potential reviewers. |
shell = if shell == "" then stdenv.shell else | ||
if builtins.isAttrs shell then (shell + shell.shellPath) | ||
else shell; | ||
shell = shell + lib.optionalString (stdenv ? shellPath) stdenv.shellPath; |
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.
Maybe shell = shell + (stdenv.shellPath or "");
?
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.
Good call. Thanks!
@@ -86,7 +86,8 @@ stdenv.mkDerivation { | |||
|
|||
preferLocalBuild = true; | |||
|
|||
inherit cc shell libc_bin libc_dev libc_lib binutils_bin coreutils_bin; | |||
inherit cc libc_bin libc_dev libc_lib binutils_bin coreutils_bin; | |||
shell = getBin shell + stdenv.lib.optionalString (stdenv ? shellPath) stdenv.shellPath; |
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.
Ditto
ad01e6f
to
28d4824
Compare
28d4824
to
6c74ee6
Compare
Motivation for this change
crossDrv
is now the default so we don't need to worry about that in build != host builds.shell is the build time shell, so
wrapCCCross
doesn't need to worry, as build == host.shell.shellPath
will always be appended where useful.Complicated
shell == ""
logic served no purpose.Things done
Not everything relevant builds currently, but I don't think any hashes in practice changed either (though hypothetical overrides would for the better).
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)