-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
CoreFoundation: use rpath to fix issues when using frameworks #27598
Conversation
@LnL7, thanks for your PR! By analyzing the history of the files in this pull request, we identified @pikajude, @edolstra, @Ericson2314, @LnL7 and @copumpkin to be potential reviewers. |
Waiting for the results of https://hydra.nixos.org/eval/1378812?compare=1378396#tabs-new to make sure nothing breaks. |
@LnL7 -- thanks for publishing; these changes got me unstuck by fixing a busted go1.8 binary installed from recent nixpkgs on macOS 10.12.6 earlier this weekend. |
That's great! Did you notice issues with any other packages? |
I'm also getting bitten by a busted go1.8. Is there a plan to merge this soon? Also, how can I apply this change locally and help test? |
I'll probably merge this to staging soon. You can test it by using my fork, eg.
|
Sounds good! |
…sed on NIX_COREFOUNDATION_RPATH
This will get propagated down to other libraries loaded because everything in nixpkgs references CF based on an rpath entry.
Awesome awesome awesome |
@@ -145,6 +145,10 @@ if [ "$NIX_@infixSalt@_DONT_SET_RPATH" != 1 ]; then | |||
fi | |||
done | |||
done | |||
|
|||
if [ -n "${NIX_COREFOUNDATION_RPATH:-}" ]; then |
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, I missed this before, but this should have probably done the @infixSalt@
thing, to support darwin->darwin cross compilation where the path may not be the same.
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 wrote this before the salts where added. Not sure if it really matters since it will only point to /System/Library/Frameworks
, similar to the dyld path with LD_DYLD_PATH
.
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, my bad @LnL7. I thought this was from the PR you just merged in the last 24 hours, and never double checked!
That it's analogous to LD_DYLD_PATH
makes perfect sense :)
Motivation for this change
Implementation of #24693
This allow a binary to decide what version of CoreFoundation is loaded based on it's rpath. This allows binaries to use the version from
/System/Library/Frameworks
and propagate that down to all dylibs, including libraries that are built against the pure version.I think keeping the setup hook from #22571 might still be useful, since a dylib that depends on frameworks won't work with the pure CF and hook might not get propagated all the way to the binary to set the rpath. /cc @NixOS/darwin-maintainers
Fixes #12346, #24124, #24127, #27353
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 -p nox --run "nox-review wip"
./result/bin/
)anki(python needs a wrapper)