Intel x86 Mac Cpu Fix #1955
Intel x86 Mac Cpu Fix #1955
Conversation
alexbeattie42
requested review from
BalestraPatrick,
brentleyjones,
keith,
segiddins,
thii and
mattrobmattrob
as code owners
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
| @@ -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" | |||
There was a problem hiding this comment.
this should be below all the other conditions in this block, since --cpu and --macos_cpus should take precedence over this logic
There was a problem hiding this comment.
I tried moving it below the cpu and macos_cpu options and both failed
There was a problem hiding this comment.
Doesn't this order mean users couldn't cross compile things then for darwin_arm64?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
alexbeattie42
force-pushed
the
master
branch
2 times, most recently
from
eb8c548 to
2d8d143
Compare
Fix for this issue