-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
Get rid of most @rpath nonsense on Darwin #30150
Conversation
Do you know why the problem of rpaths causing cyclic references between Go outputs does not exist on Linux? |
Because of the |
Yeah, we could have replicated that, but given that the rpaths are almost entirely unnecessary on macOS, I'd rather just scrap them altogether than replicate the more complicated solution on a platform that doesn't need it. |
How does dynamic library linkage work on darwin in general? (Just asking, I'm curious.) Do the binaries contain absolute paths to the .dylib files? If so, why does the concept of RPATH exist in the file format in the first place? |
Yes, and the libraries contain absolute paths to themselves, so that when programs (and other libraries) are linked to a library, they refer to it by its builtin absolute path. Linux ld loader also supports loading libraries by an absolute path (this is documented in |
What @orivej said, but to answer your "why" question, it's to support relocateable libraries. We often distribute .app "files" that are basically bundles of dependencies in a folder, and in those cases you don't want your dependencies to expect to be in any particular place. So basically, you can add |
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 great! Hope to do the same thing on Linux too, someday.
# TODO: This really wants to be in stdenv/darwin but we don't have hostPlatform | ||
# there (yet?) so it goes here until then. | ||
preHook = preHook+ lib.optionalString buildPlatform.isDarwin '' | ||
export NIX_BUILD_DONT_SET_RPATH=1 |
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 the rpath flags for CoreFoundation are only added when this is not set. It probably should be moved outside of the conditional.
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.
Hmm, sorry, I don't understand. Where is the CF rpath code?
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.
8a71869
to
2bbf4fd
Compare
This requires some small changes in the stdenv, then working around the weird choice LLVM made to hardcode @rpath in its install name, and then lets us remove a ton of annoying workaround hacks in many of our Go packages. With any luck this will mean less hackery going forward.
2bbf4fd
to
b426c85
Compare
@Ericson2314 if you want it on Linux, is there a ticket for that? I'd be curious to hear some discussion of what's involved and see what the community thinks of it |
There's #24844 |
Thanks! |
I'm going to pick this into release (via |
Released in cfc55fe. |
Cool, thanks @orivej |
This requires some small changes in the stdenv, then working around the weird choice LLVM made to hardcode
@rpath
in its install name, and then lets us remove a ton of annoying workaround hacks in many of our Go packages. With any luck this will mean less hackery going forward.Motivation for this change
I got sick of seeing people getting weird rpath-related errors when packaging Go programs.
Fixes #18131
@acowley this should fix the Go issue you encountered earlier today
Things left to do
rustc
) to make sure it still runs now that I took out therpath
install name in libLLVM. I expect it to but it might have an extraneousrpath
load command or two in it.rpath
crap from other LLVM versions too, since they'll probably break without the stdenv supportThings done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)