-
-
Notifications
You must be signed in to change notification settings - Fork 14.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
darwin.stdenv: propagate sdkRoot via extraBuildInputs #308366
Conversation
This ensures it is properly dropped when cross-compiling.
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.
Couple questions because I've really not grokked all the cross setup yet.
extraNativeBuildInputs = extraNativeBuildInputs | ||
++ [ prevStage.darwin.apple_sdk.sdkRoot ]; | ||
extraBuildInputs = [ prevStage.darwin.CF prevStage.darwin.apple_sdk.sdkRoot ]; | ||
extraNativeBuildInputs = extraNativeBuildInputs; |
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.
Potentially use inherit?
@@ -41,28 +41,24 @@ runCommand "sdkroot-${sdkVersion}" { } '' | |||
|
|||
install -D '${../../../build-support/setup-hooks/role.bash}' "$out/nix-support/setup-hook" | |||
cat >> "$out/nix-support/setup-hook" <<-hook | |||
# |
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.
So we're dropping the exception when the hook isn't a build-time dependency and we're not doing a native compile?
Seems like a good thing to have it apply consistently.
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.
It needs to be depsHostTarget
(i.e., a buildInput
), but the hook wasn’t being run unless I took this out. The doc seems to suggest it should work, but maybe I was doing something wrong.
sdkRootHook() { | ||
# See ../../../build-support/setup-hooks/role.bash | ||
local role_post | ||
getHostRoleEnvHook | ||
|
||
# Only set the SDK root if one has not been set via this hook or some other means. | ||
if [[ ! \$NIX_CFLAGS_COMPILE =~ isysroot ]]; then | ||
local cflagsVar=NIX_CFLAGS_COMPILE\''${role_post} | ||
if [[ ! \''${!cflagsVar} =~ isysroot ]]; then | ||
export NIX_CFLAGS_COMPILE\''${role_post}+=' -isysroot $out/${sdkName}.sdk' |
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.
Don't we want to reuse the cflagsVar
here or can you not use an indirect reference with export?
# See ../../../build-support/setup-hooks/role.bash | ||
getTargetRole | ||
|
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.
# See ../../../build-support/setup-hooks/role.bash | |
getTargetRole |
If the below is correct I believe this no longer serves a purpose. Actually seems to be redundant even without the change from target to host offset because this sets role_post, which is unset right after.
Closed by #346043 |
Description of changes
This ensures it is properly dropped when cross-compiling.
I tested by building the Darwin bootstrap on aarch64-darwin and building psutil in the bootstrap to confirm the sdk root is being propagated correctly. It should have an SDK version of 11.0 (and did).
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.