-
Notifications
You must be signed in to change notification settings - Fork 4.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
Add oso_prefix_is_pwd macOS feature #11671
Add oso_prefix_is_pwd macOS feature #11671
Conversation
With Xcode 11 Apple's linker introduced a new oso_prefix option which removes the given prefix from the path in debug info. This improves the goal of producing reproducible builds. More info: https://milen.me/writings/apple-linker-ld64-deterministic-builds-oso-prefix
If there's a better way to do this type of feature that requires a specific Xcode version I'm happy to change this, but I figured it being opt in is probably "good enough" (although it's possible we want to join it with |
flag_sets = [ | ||
flag_set( | ||
actions = all_link_actions + | ||
["objc-executable", "objc++-executable"], |
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.
will this flag apply to executables without any objc as well?
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.
what's the status of debug-prefix-map in bazel? since it requires that each compile invocation change based on the path, as opposed to just the link invocation, does it invalidate any bazel caches for that reason? otherwise, i haven't tested this linker flag myself but this look sound overall |
Both debug-prefix-map and this one are "hidden" in the clang wrapper, so the command line is stable, and the wrapper resolves and puts the arbitrary path in the actually invocation (rules_swift does something similar too) so they don't invalidate any caches. |
👍 |
+@allevato this seems like a useful thing to have, and I don't see it set up in internal toolchains. |
We are tracking this internally. |
Even if this was fixed internally that wouldn't apply to the public crosstool correct? |
Correct. Not at this time. This change seems fine. |
Is this aligned with how it would work with the internal crosstool? It doesn't sound like that's happening anymore but if there's a small change I could make to be more in line with that I'd be happy to! |
Yes, I think this is how it would be handled. wrapped_clang needs to set this because we don't know pwd until the compile/link is executing on the machine. |
@trybka I'll import this for your review. |
Thanks!! |
Expands the targeted actions to allow stripping the absolute build path for debug symbols from all libraries. The support in wrapped_clang is indifferent to whether we are linking a true objc or regular cpp library. When used in concert with `--features=oso_prefix_is_pwd --remote_download_outputs=all`, allows successful local debugging of executables/libraries built remotely. Probably also helps with sandboxed actions. Helps mitigate #6327 and kinda sorta #2537? See also #11671 Closes #13311. PiperOrigin-RevId: 404262861
With Xcode 11 Apple's linker introduced a new oso_prefix option which
removes the given prefix from the path in debug info. This improves the
goal of producing reproducible builds.
More info: https://milen.me/writings/apple-linker-ld64-deterministic-builds-oso-prefix
RELNOTES: Add opt in 'oso_prefix_is_pwd' feature for Apple builds