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

Combination of Apple toolchain and java change leading to test failures #17762

Open
keith opened this issue Mar 13, 2023 · 3 comments
Open

Combination of Apple toolchain and java change leading to test failures #17762

keith opened this issue Mar 13, 2023 · 3 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules team-Rules-Java Issues for Java rules type: bug

Comments

@keith
Copy link
Member

keith commented Mar 13, 2023

The combination of #16619 and #16960 lead to some test failures in bazel itself when building targets that depend on the darwin_universal_binary rule added in java PR.

The failure looks like this:

(20:06:01) ERROR: /private/var/tmp/_bazel_buildkite/4b66776e1a7e7db9d1f29690af6b43c8/external/local_config_cc/BUILD:66:19: in cc_toolchain_suite rule @local_config_cc//:toolchain: cc_toolchain_suite '@local_config_cc//:toolchain' does not contain a toolchain for cpu 'darwin_x86_64'

And is caused by the fact that if you want to build non-host platform macOS binaries you must use the new toolchain that lives in the apple_support repo, which bazel does not. I don't have full context on the java issue so I wanted to file this to discuss possible solutions. I see at least 2 options:

  1. Make the builtin cc toolchain on macOS support cross-compiling binaries for x86_64/arm. The downside of this is that it adds complexity to the cc toolchain that's macOS specific and the goal of migrating the cc toolchain to the other repo was partially to remove that complexity from bazel itself. I think at the very least it would require some custom setup of the toolchain for macOS (to setup 2 toolchains), as well as adding at least 1 new feature for adding the right -target flags, which are just not passed and the compiler picks the default today
  2. Try to unblock java using a separate binary on the different hosts, specifically this comment from the java PR made it sound like that might be possible:

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 wonder if this change #17547 also would help with that.

I'm happy to take on the changes here soon so we can ideally avoid reverts, but I'm hoping for some feedback before choosing a path here.

// @googlewalt, @comius, @hvadehra

@ShreeM01 ShreeM01 added type: bug team-Rules-Java Issues for Java rules team-Rules-CPP Issues for C++ rules untriaged labels Mar 13, 2023
@keith
Copy link
Member Author

keith commented Mar 14, 2023

looks like commit was reverted a50cca5 so any CI jobs failing for this will be back to green.

@zhengwei143 can you provide context on the internal failures so I can fix them and resubmit?

@keith
Copy link
Member Author

keith commented Mar 14, 2023

also looks like this commit #17767 could solve the original issue here

@googlewalt
Copy link
Contributor

I have a fix forward for the internal failure.

copybara-service bot pushed a commit that referenced this issue Mar 15, 2023
*** 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
@buildbreaker2021 buildbreaker2021 added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Mar 21, 2023
fweikert pushed a commit to fweikert/bazel that referenced this issue May 25, 2023
*** 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules team-Rules-Java Issues for Java rules type: bug
Projects
None yet
Development

No branches or pull requests

4 participants