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

Fix runtime_solib_name for --incompatible_macos_set_install_name #23089

Closed
wants to merge 1 commit into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Jul 24, 2024

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.

Work towards #12370

@fmeum fmeum requested a review from lberki as a code owner July 24, 2024 14:04
@github-actions github-actions bot added team-Rules-CPP Issues for C++ rules awaiting-review PR is awaiting review from an assigned reviewer labels Jul 24, 2024
@fmeum fmeum requested review from comius and removed request for lberki July 24, 2024 14:06
@fmeum
Copy link
Collaborator Author

fmeum commented Jul 24, 2024

cc @Yannic @keith

@fmeum
Copy link
Collaborator Author

fmeum commented Jul 24, 2024

@bazel-io fork 7.3.0

@fmeum
Copy link
Collaborator Author

fmeum commented Jul 24, 2024

The follow-up is #23090

@fmeum
Copy link
Collaborator Author

fmeum commented Jul 24, 2024

The Darwin failure is a flake.

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
Copy link
Collaborator Author

fmeum commented Jul 24, 2024

@meteorcloudy The new macOS workers keep running into weird CI failures on this branch. Sometimes it's the Javac worker crashing, sometimes extraction failures such as https://buildkite.com/bazel/bazel-bazel-github-presubmit/builds/22557#0190e57f-a35c-460d-8a40-efef5a69e656. I'll avoid running new jobs for now in case CI is just overloaded.

@iancha1992
Copy link
Member

Just a friendly reminder, the 7.3.0RC1 is scheduled for next Monday, July 29th. Thanks!

@fmeum
Copy link
Collaborator Author

fmeum commented Aug 1, 2024

@bazel-io fork 7.4.0

@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 Aug 26, 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 Aug 28, 2024
@fmeum fmeum deleted the install-name-fix branch August 28, 2024 06:06
fmeum added a commit to fmeum/bazel that referenced this pull request Sep 19, 2024
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.

Work towards bazelbuild#12370

Closes bazelbuild#23089.

PiperOrigin-RevId: 668228562
Change-Id: I7524679bfe8c6b8b28c0cb04f46c0d22d0adbe99
github-merge-queue bot pushed a commit that referenced this pull request Sep 23, 2024
…l_name` (#23672)

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.

Work towards #12370

Closes #23089.

PiperOrigin-RevId: 668228562
Change-Id: I7524679bfe8c6b8b28c0cb04f46c0d22d0adbe99

Fixes #23185
@iancha1992
Copy link
Member

The changes in this PR have been included in Bazel 7.4.0 RC1. Please test out the release candidate and report any issues as soon as possible.
If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=7.4.0rc1. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants