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.

Sign up for GitHub

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

Jump to bottom

Update DEFAULT_MACOS_CPU to match host #13440

Closed
wants to merge 1 commit into from
Closed

Update DEFAULT_MACOS_CPU to match host #13440

wants to merge 1 commit into from

Conversation

keith
Copy link
Member

@keith keith commented May 6, 2021

With Apple Silicon machines host tools that use the apple_binary, or
apple fragment to fetch the CPU would be built for the wrong
architecture, even though the C++ logic would correctly identify the
host CPU. This changes this hardcoded value to correctly determine Apple
Silicon as arm64, and otherwise fallback to x86_64.

Fixes #12671

@keith
Update DEFAULT_MACOS_CPU to match host
2b66486 
With Apple Silicon machines host tools that use the apple_binary, or
apple fragment to fetch the CPU would be built for the wrong
architecture, even though the C++ logic would correctly identify the
host CPU. This changes this hardcoded value to correctly determine Apple
Silicon as arm64, and otherwise fallback to x86_64.

Fixes bazelbuild#12671
@keith keith requested a review from lberki as a code owner May 6, 2021 21:45
@google-cla google-cla bot added the cla: yes label May 6, 2021
@keith keith mentioned this pull request May 6, 2021
Closed
@keith
Copy link
Member Author

keith commented May 6, 2021

There are 2 things to note about this change:

  1. if folks still want to build for x86_64 from a Apple Silicon host they can pass --macos_cpus
  2. attempting to run bazel with arch -x86_64 on Apple Silicon isn't actually respect, it seems to be related to the JVM arch instead, which means host tools built on Apple Silicon will always be built for arm64, even if you use a rosetta simulated terminal

thii
thii approved these changes May 6, 2021
View reviewed changes
Copy link
Member

@thii thii left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this.

@aiuto aiuto requested a review from susinmotion May 7, 2021 00:32
@lberki lberki removed their request for review May 7, 2021 09:06
@keith keith mentioned this pull request May 10, 2021
Merged
keith added a commit to keith/bazel that referenced this pull request May 10, 2021
@keith
Add --host_macos_cpus flag
9384545 
When your macOS build has tools that build with the Apple rules, such as
`macos_command_line_application`, you likely want to share caches
between Apple Silicon and Intel machines. Today your tool would build
for the host architecture (ideally, see bazelbuild#13440)
which means the tools would differ between the different architectures,
leading to split caches. This flag allows you to pass
`--host_macos_cpus=arm64,x86_64` in order to produce a fat binary for
host tools. This has the downside of increasing binary size for your
tools, but likely that trade-offs is worth it for almost everyone versus
having split caches.
@keith keith mentioned this pull request May 10, 2021
Closed
@bazel-io bazel-io closed this in aaae8ce May 12, 2021
katre pushed a commit that referenced this pull request Jul 12, 2021
@keith @katre
Update DEFAULT_MACOS_CPU to match host
6034219 
With Apple Silicon machines host tools that use the apple_binary, or
apple fragment to fetch the CPU would be built for the wrong
architecture, even though the C++ logic would correctly identify the
host CPU. This changes this hardcoded value to correctly determine Apple
Silicon as arm64, and otherwise fallback to x86_64.

Fixes #12671

Closes #13440.

PiperOrigin-RevId: 373421791
katre pushed a commit to katre/bazel that referenced this pull request Jul 13, 2021
@keith @katre
Update DEFAULT_MACOS_CPU to match host
9ad3d3a 
With Apple Silicon machines host tools that use the apple_binary, or
apple fragment to fetch the CPU would be built for the wrong
architecture, even though the C++ logic would correctly identify the
host CPU. This changes this hardcoded value to correctly determine Apple
Silicon as arm64, and otherwise fallback to x86_64.

Fixes bazelbuild#12671

Closes bazelbuild#13440.

PiperOrigin-RevId: 373421791
katre pushed a commit to katre/bazel that referenced this pull request Jul 13, 2021
@keith @katre
Update DEFAULT_MACOS_CPU to match host
5608dfa 
With Apple Silicon machines host tools that use the apple_binary, or
apple fragment to fetch the CPU would be built for the wrong
architecture, even though the C++ logic would correctly identify the
host CPU. This changes this hardcoded value to correctly determine Apple
Silicon as arm64, and otherwise fallback to x86_64.

Fixes bazelbuild#12671

Closes bazelbuild#13440.

PiperOrigin-RevId: 373421791
@keith keith deleted the ks/default-cpu branch October 20, 2021 18:37
keith added a commit to keith/bazel that referenced this pull request Feb 14, 2022
@keith
Fix default CPU for macOS and iOS
3537351 
Since M1 support was added we started defaulting the CPU for macOS and
iOS builds to the host CPU in the case another CPU wasn't passed. This
change mirrors bazelbuild#13873,
bazelbuild#13440 and
https://github.com/bazelbuild/rules_apple//commit/99e5b631bf060358241a8eaabd285be5c120490f
in the newer starlark transition.

Without this you end up with architecture mismatches for builds. Fixes bazelbuild#14803
@keith keith mentioned this pull request Feb 14, 2022
Closed
bazel-io pushed a commit that referenced this pull request Mar 1, 2022
@keith
Fix default CPU for macOS and iOS
1ec1068 
Since M1 support was added we started defaulting the CPU for macOS and
iOS builds to the host CPU in the case another CPU wasn't passed. This
change mirrors #13873, #13440 and https://github.com/bazelbuild/rules_apple//commit/99e5b631bf060358241a8eaabd285be5c120490f
in the newer starlark transition.

Without this you end up with architecture mismatches for builds. Fixes #14803

Closes #14816.

PiperOrigin-RevId: 431641312
brentleyjones pushed a commit to brentleyjones/bazel that referenced this pull request Mar 1, 2022
@keith @brentleyjones
Fix default CPU for macOS and iOS
cffd672 
Since M1 support was added we started defaulting the CPU for macOS and
iOS builds to the host CPU in the case another CPU wasn't passed. This
change mirrors bazelbuild#13873, bazelbuild#13440 and https://github.com/bazelbuild/rules_apple//commit/99e5b631bf060358241a8eaabd285be5c120490f
in the newer starlark transition.

Without this you end up with architecture mismatches for builds. Fixes bazelbuild#14803

Closes bazelbuild#14816.

PiperOrigin-RevId: 431641312
(cherry picked from commit 1ec1068)
@brentleyjones brentleyjones mentioned this pull request Mar 1, 2022
Merged
Wyverald pushed a commit that referenced this pull request Mar 2, 2022
@brentleyjones @keith
Fix default CPU for macOS and iOS (#14923)
d42ab0c 
Since M1 support was added we started defaulting the CPU for macOS and
iOS builds to the host CPU in the case another CPU wasn't passed. This
change mirrors #13873, #13440 and https://github.com/bazelbuild/rules_apple//commit/99e5b631bf060358241a8eaabd285be5c120490f
in the newer starlark transition.

Without this you end up with architecture mismatches for builds. Fixes #14803

Closes #14816.

PiperOrigin-RevId: 431641312
(cherry picked from commit 1ec1068)

Co-authored-by: Keith Smiley <keithbsmiley@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thii thii thii approved these changes

@susinmotion susinmotion susinmotion approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Swift host tools compiled with wrong cpu on Apple Silicon
3 participants
@keith @thii @susinmotion