Skip to content
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

Flip --incompatible_macos_set_install_name #23090

Closed
wants to merge 3 commits into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Jul 24, 2024

This requires adding the set_install_name feature to the Unix toolchain on macOS only. The legacy install name patcher is kept around for as long as the flag can be unflipped.

RELNOTES[INC]: With the default Unix toolchain on macOS, binaries now use @rpath to find their .dylib dependencies. This is required to fix issues where tools run during the build couldn't find their dynamic dependencies.

Closes #12370

The `runtime_solib_name` link variable had an incorrect value in the case of a transitive dynamic library. Since non-transitive ("nodeps") dynamic libraries are no longer used on macOS, the `--incompatible_macos_set_install_name` flag didn't have any positive effect.

This PR is intentionally limited to the fix so that it can be cherry-picked into Bazel 7, where it can make the incompatible flag work with the `apple_support` toolchain. A follow-up PR will add the feature to the Unix toolchain and flip the incompatible flag for Bazel 8.
@fmeum fmeum added the incompatible-change Incompatible/breaking change label Jul 24, 2024
@fmeum fmeum marked this pull request as ready for review August 28, 2024 06:54
@fmeum fmeum requested a review from lberki as a code owner August 28, 2024 06:54
@fmeum fmeum requested a review from comius August 28, 2024 06:54
@github-actions github-actions bot added team-Rules-CPP Issues for C++ rules awaiting-review PR is awaiting review from an assigned reviewer labels Aug 28, 2024
@fmeum fmeum removed the request for review from lberki August 28, 2024 06:54
@comius comius added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Sep 23, 2024
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Sep 25, 2024
@fmeum fmeum deleted the fix-toolchain branch September 25, 2024 20:44
keith pushed a commit to keith/bazel that referenced this pull request Oct 3, 2024
This requires adding the `set_install_name` feature to the Unix toolchain on macOS only. The legacy install name patcher is kept around for as long as the flag can be unflipped.

RELNOTES[INC]: With the default Unix toolchain on macOS, binaries now use `@rpath` to find their `.dylib` dependencies. This is required to fix issues where tools run during the build couldn't find their dynamic dependencies.

Closes bazelbuild#12370

Closes bazelbuild#23090.

PiperOrigin-RevId: 678780800
Change-Id: I13a1bb993c13fb2f9d7b358c2881a7ccdafe66cc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
incompatible-change Incompatible/breaking change team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

incompatible_macos_set_install_name: Set -install_name when linking dynamic libraries on macOS
2 participants