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

Intel x86 Mac Cpu Fix #1955

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Intel x86 Mac Cpu Fix #1955

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions apple/internal/transition_support.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ def _cpu_string(*, cpu, platform_type, settings = {}):
return "ios_sim_arm64"
return "ios_x86_64"
if platform_type == "macos":
host_cpu = settings["//command_line_option:host_cpu"]
if host_cpu == "darwin":
return "darwin"
Copy link
Member

Choose a reason for hiding this comment

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

this should be below all the other conditions in this block, since --cpu and --macos_cpus should take precedence over this logic

Copy link
Author

Choose a reason for hiding this comment

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

I tried moving it below the cpu and macos_cpu options and both failed

Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this order mean users couldn't cross compile things then for darwin_arm64?

Copy link
Author

@alexbeattie42 alexbeattie42 May 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not able to test darwin_arm64 since I don't have an apple silicone mac, but I think it's host cpu value should report as darwin_arm64 so it would not match darwin.

Copy link
Member

Choose a reason for hiding this comment

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

right but if you are on your intel mac, where the host_cpu is darwin, and you pass --cpu=darwin_arm64 to produce an output for an arm64 mac, i think this condition would take precedence

Copy link
Author

@alexbeattie42 alexbeattie42 May 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I can confirm that this change does break apple silicone mac. I'm also facing another problem with building on apple silicone. Is there a better approach that could be used to solve this problem without breaking things for the above conditions?

Copy link
Member

Choose a reason for hiding this comment

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

you'd probably need to check the times we return darwin_x86_64 and return darwin in the case that the host CPU is just darwin, and not affect the return value otherwise

if cpu:
return "darwin_{}".format(cpu)
macos_cpus = settings["//command_line_option:macos_cpus"]
Expand Down Expand Up @@ -342,6 +345,7 @@ _apple_rule_common_transition_inputs = [
"//command_line_option:apple_crosstool_top",
]
_apple_rule_base_transition_inputs = _apple_rule_common_transition_inputs + [
"//command_line_option:host_cpu",
"//command_line_option:cpu",
"//command_line_option:ios_multi_cpus",
"//command_line_option:macos_cpus",
Expand Down