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

Bazel: fix non-swift macOS builds #16632

Merged
merged 6 commits into from
May 31, 2024
Merged

Bazel: fix non-swift macOS builds #16632

merged 6 commits into from
May 31, 2024

Conversation

redsun82
Copy link
Contributor

@redsun82 redsun82 commented May 31, 2024

Now the swift-specific compilation options are contained in a transition applied only to swift targets.

A transition is required for the macOS -arch options, because we need all dependencies that are linked into swift targets to have those. While other options (-fno-rtti and -std=c++20) could work with a simple cc_{binary,library} attribute injection, it was preferred to use one mechanism only.

This is meant to be cleaned up in a later PR with respect to the TODO: the bazel transition used for Swift is inspired by what we use in the internal repo, but has some differences:

  • the internal binary is removed from the runfiles, as otherwise codeql_pkg_runfiles had a conflict on it
  • the transition works on cc_library as well
  • the transition forwards the CcInfo provider as well

Also added new triggers to local go CI to detect this kind of problem earlier next time.

One drawback of a transition-based approach is that bazel build //swift/... ceases to work, because it picks up the internal transition targets without the proper compile options. This also means that bazel test //swift/... now requires --build_tests_only.

This is meant to be cleaned up in a later PR with respect to the TODOs.
@redsun82 redsun82 requested review from a team as code owners May 31, 2024 09:51
@github-actions github-actions bot added the Swift label May 31, 2024
@redsun82 redsun82 requested a review from a team as a code owner May 31, 2024 10:14
@redsun82 redsun82 requested a review from criemen May 31, 2024 10:24
criemen
criemen previously approved these changes May 31, 2024
Copy link
Collaborator

@criemen criemen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, assuming that defining a select object and injecting that via _wrap_cc is too complicated/cumbersome, or doesn't work for some reason.

@criemen
Copy link
Collaborator

criemen commented May 31, 2024

(sorry about the conflict)

Copy link
Collaborator

@criemen criemen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reapproving

@redsun82 redsun82 merged commit 01c1acd into main May 31, 2024
20 of 21 checks passed
@redsun82 redsun82 deleted the redsun82/bazel-fix branch May 31, 2024 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants