Add darwin_arm64 java_tools #16960
Add darwin_arm64 java_tools #16960
Conversation
hvadehra
force-pushed
the
hvadehra-java_tools_darwin_arm64
branch
14 times, most recently
from
0432054
to
0d68448
Compare
hvadehra
force-pushed
the
hvadehra-java_tools_darwin_arm64
branch
from
0d68448
to
0c7c49f
Compare
sgowroji
added
team-Rules-Java
hvadehra
force-pushed
the
hvadehra-java_tools_darwin_arm64
branch
3 times, most recently
from
a8de756
to
33d4262
Compare
java_tools for darwin will be built as a fat universal binary after bazelbuild/bazel#16960 so it needs to use the osx toolchain.
hvadehra
force-pushed
the
hvadehra-java_tools_darwin_arm64
branch
from
33d4262
to
ae0e759
Compare
There was a problem hiding this comment.
Is there a reason to build a fat binary and move away from the previously used pattern where 3 prebuilt repositories are released? Technically you should be able to extend for the 4th repository, build for that platform and select the correct prebuilt.
That option looks less complex as the current one, because it doesn't involve additional rule and this makes it probably easier to maintain.
src/darwin_universal_binary.bzl
Outdated
| @@ -0,0 +1,54 @@ | |||
| def _universal_split_transition_impl(ctx, attr): | |||
| return { | |||
| "x86_64" : { | |||
There was a problem hiding this comment.
This doesn't support platformization, which we'll need to update when we turn it on.
There was a problem hiding this comment.
Please also set --platforms flag to match the cpu. This then won't cause problems when we flip --incompatible_enable_cc_toolchain_resolution
That's what I tried first, but it was a more invasive change. If we were to have add a new repo for apple silicon, I think we should rename the existing one to "darwin_intel" or "darwin_x86_64" (leaving it as "darwin" would be misleading/incorrect). So I tried that, and it ended up getting very messy. There are a lot of assumptions in code and tests that there is only a single darwin platform (i.e. platform == OS).
I feel the opposite. Going down that route seems less extensible in general, requiring a lot more updates whenever something changes. At least, not without significant refactoring of all BUILD, BUILD.tools, BUILD.java_tools, etc |
src/darwin_universal_binary.bzl
Outdated
| @@ -0,0 +1,54 @@ | |||
| def _universal_split_transition_impl(ctx, attr): | |||
| return { | |||
| "x86_64" : { | |||
There was a problem hiding this comment.
Please also set --platforms flag to match the cpu. This then won't cause problems when we flip --incompatible_enable_cc_toolchain_resolution
hvadehra
force-pushed
the
hvadehra-java_tools_darwin_arm64
branch
from
ae0e759
to
fc931ff
Compare
hvadehra
removed request for
a team,
tetromino,
Wyverald,
brandjon,
lberki,
ted-xie,
gregestren,
ahumesky,
fweikert,
kevin1e100 and
oquenchil
*** Reason for rollback *** Conflicts with #17762 Will instead go with the alternative idea discussed in this PR: #17767 *** Original change description *** Add darwin_arm64 java_tools Build a fat universal binary for java_tools_prebuilt on darwin Work towards: bazelbuild/java_tools#57 and #13944 Closes #16960. PiperOrigin-RevId: 516799739 Change-Id: I2ff5f615bd7c23e38a334bf836c31ed964443c31
Build a fat universal binary for java_tools_prebuilt on darwin Work towards: bazelbuild/java_tools#57 and bazelbuild#13944 Closes bazelbuild#16960. PiperOrigin-RevId: 512683379 Change-Id: Ie9db26c729a301fbb22f17dd15065861f3198f57
*** Reason for rollback *** Conflicts with bazelbuild#17762 Will instead go with the alternative idea discussed in this PR: bazelbuild#17767 *** Original change description *** Add darwin_arm64 java_tools Build a fat universal binary for java_tools_prebuilt on darwin Work towards: bazelbuild/java_tools#57 and bazelbuild#13944 Closes bazelbuild#16960. PiperOrigin-RevId: 516799739 Change-Id: I2ff5f615bd7c23e38a334bf836c31ed964443c31
java_tools for darwin will be built as a fat universal binary after bazelbuild/bazel#16960 so it needs to use the osx toolchain.
Build a fat universal binary for java_tools_prebuilt on darwin
Work towards: bazelbuild/java_tools#57 and #13944