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

[7.2.0] Add layering_check support for macOS #22572

Merged
merged 1 commit into from
May 28, 2024

Conversation

bazel-io
Copy link
Member

There were 2 things with the previous implementation that needed to be improved here:

  1. Apple Clang has a bug where it doesn't pass module compiler flags to the underlying -cc1 invocation, so we have to manually pass them directly to that invocation with -Xclang
  2. The previous search script was too aggressive and slow for macOS. The macOS SDK has tons of files that aren't headers, and tons of symlinks pointing to other files within the SDK. This adds a fork in the script to run a version that works with Apple SDKs. The time difference on my machine is 41s->6s. 6s is still pretty long so if desired we can put this behavior behind an env var for users to opt in with.

I've added a hermetic version of this to the apple_support toolchain, but similar to the Linux setup here the modulemap file includes absolute paths.

Closes #22259.

This reverts commit 1f1b4fd.

Closes #22475.

PiperOrigin-RevId: 637969838
Change-Id: I7d4940a820e3741836239493222ba8d06c4d70e4

Commit 6abdd2a

@bazel-io bazel-io requested a review from a team as a code owner May 28, 2024 18:52
@bazel-io bazel-io added team-Rules-CPP Issues for C++ rules awaiting-review PR is awaiting review from an assigned reviewer labels May 28, 2024
@keertk keertk requested a review from Wyverald May 28, 2024 18:55
@iancha1992 iancha1992 enabled auto-merge May 28, 2024 18:56
@Wyverald
Copy link
Member

Need to cherry-pick de0a37c first

@iancha1992
Copy link
Member

Need to cherry-pick de0a37c first

Sent #22575 @Wyverald

@Wyverald
Copy link
Member

Is there a way for us to force bazel-io to rebase the PR on top of branch? Or do we need to send a PR ourselves?

There were 2 things with the previous implementation that needed to be improved here:

1. Apple Clang has a bug where it doesn't pass module compiler flags to the underlying -cc1 invocation, so we have to manually pass them directly to that invocation with -Xclang
2. The previous search script was too aggressive and slow for macOS. The macOS SDK has tons of files that aren't headers, and tons of symlinks pointing to other files within the SDK. This adds a fork in the script to run a version that works with Apple SDKs. The time difference on my machine is 41s->6s. 6s is still pretty long so if desired we can put this behavior behind an env var for users to opt in with.

I've added a hermetic version of this to the apple_support toolchain, but similar to the Linux setup here the modulemap file includes absolute paths.

Closes bazelbuild#22259.

This reverts commit 1f1b4fd.

Closes bazelbuild#22475.

PiperOrigin-RevId: 637969838
Change-Id: I7d4940a820e3741836239493222ba8d06c4d70e4
@iancha1992
Copy link
Member

iancha1992 commented May 28, 2024

Is there a way for us to force bazel-io to rebase the PR on top of branch? Or do we need to send a PR ourselves?

for now, I'll force push this one. But we'll come up with a mechanism to rebase and foce-push to bazel-io's branches to make it easier. @Wyverald

@iancha1992 iancha1992 added this pull request to the merge queue May 28, 2024
Merged via the queue into bazelbuild:release-7.2.0 with commit bc346fb May 28, 2024
32 checks passed
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label May 28, 2024
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.

None yet

4 participants