-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
apple_sdk_11_0.stdenv: replace extraBuildInputs with CF from 11.0 SDK #234863
Conversation
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.
Is there a convenient test case where I can observe the wrong CF ending up in a build?
@@ -61,7 +61,9 @@ let | |||
mkStdenv = stdenv: | |||
if stdenv.isAarch64 then stdenv | |||
else | |||
|
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.
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 pushed an updated commit without the newline. I’m not sure how it ended up there, but it’s gone.
This test case demonstrates the issue. It’s mostly theoretical, but the idea is to avoid mixing SDKs at all to save having to troubleshoot cryptic build failures in the future. Given
|
This can result in swift-corefoundation being included even when a derivation uses `apple_sdk_11_0.stdenv` directly, which may result in hard to debug compiler errors (unless you just happen to know cryptic compiler errors means the 10.12 and 11.0 SDKs have been mixed).
02c796e
to
28fe4ee
Compare
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.
LGTM
Description of changes
This can result in swift-corefoundation being included even when a derivation uses
apple_sdk_11_0.stdenv
directly, which may result in hard to debug compiler errors (unless you just happen to know cryptic compiler errors means the 10.12 and 11.0 SDKs have been mixed).Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)