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

[windows/arm64] Add missing JDK toolchain for java build #14700

Closed
wants to merge 5 commits into from

Conversation

niyas-sait
Copy link
Contributor

Extend configurations to add JDK 11 and 17 for windows/arm64 platforms.

This should fix the Java builds on windows/arm64

@niyas-sait
Copy link
Contributor Author

@meteorcloudy and @philwo - Can you please review this change? This would be needed for the java builds.

WORKSPACE Outdated Show resolved Hide resolved
distdir_deps.bzl Outdated Show resolved Hide resolved
@philwo
Copy link
Member

philwo commented Feb 3, 2022

Huh, not sure why the mirror URL is giving HTTP 404... let me check.

Edit: Fixed! I'll rerun the failed CI jobs.

@niyas-sait
Copy link
Contributor Author

Huh, not sure why the mirror URL is giving HTTP 404... let me check.

Edit: Fixed! I'll rerun the failed CI jobs.

Thanks! Looks like the failing job also has trouble downloading JDK package for darwin from Bazel mirror. If this is already fixed or just an intermittent issue, could you retrigger the build please ?

test_verify_urls FAILED: Invalid urls: https://mirror.bazel.build/zulu/bin/zulu17.30.15-ca-jdk17.0.1-macosx_aarch64.tar.gz.
/b/f/w/bazel-out/k8-fastbuild/bin/src/test/shell/bazel/verify_workspace.runfiles/io_bazel/src/test/shell/bazel/verify_workspace:62: in call to test_verify_urls
INFO[verify_workspace 2022-02-03 15:11:37 (+0000)] Cleaning up workspace

@philwo
Copy link
Member

philwo commented Feb 3, 2022

@nsait-linaro I'm really sorry, I don't know why this test is suddenly failing with this error. The URL seems fine and I can download it on my own machine. I'll retry it later today, maybe it's some really weird kind of network flakyness between RBE and the remote server? I don't see anything in your PR that would cause it. @meteorcloudy Any idea?

@meteorcloudy
Copy link
Member

Hmm, it does seem to be flaky in another presubmit: https://buildkite.com/bazel/bazel-bazel-github-presubmit/builds/10200#f7c14b0e-60b7-4f4f-9440-e78c19da31ef

But not sure what caused it..

@gregestren gregestren added the team-Rules-Java Issues for Java rules label Feb 3, 2022
@niyas-sait
Copy link
Contributor Author

@nsait-linaro I'm really sorry, I don't know why this test is suddenly failing with this error. The URL seems fine and I can download it on my own machine. I'll retry it later today, maybe it's some really weird kind of network flakyness between RBE and the remote server? I don't see anything in your PR that would cause it. @meteorcloudy Any idea?

Thanks, @philwo. Could you do one more retrigger please ?

@comius
Copy link
Contributor

comius commented Feb 4, 2022

Hmm, it does seem to be flaky in another presubmit: https://buildkite.com/bazel/bazel-bazel-github-presubmit/builds/10200#f7c14b0e-60b7-4f4f-9440-e78c19da31ef

But not sure what caused it..

I noticed in this in one of my PRs before. At that PR it looked like we're hitting some kind of traffic limiter - one url in the list consistently failed remotely (i'm only assuming it was the last one), and the same url didn't fail locally. The issue was resolved, because somebody else did a cleanup, removing a couple of remote repos.

@philwo
Copy link
Member

philwo commented Feb 4, 2022

I'll submit a change to make this test easier to debug, as we currently have no idea or log output why it's failing. :/

@comius That's a good idea. If it's indeed a traffic limit, maybe RBE team can help us out, as this is only failing on RBE, so it must be something specific to the environment.

@philwo
Copy link
Member

philwo commented Feb 4, 2022

@nsait-linaro Could you please rebase this PR against master to pick-up 938e209? I'm curious if this will tell us why the test is failing.

@niyas-sait
Copy link
Contributor Author

@nsait-linaro Could you please rebase this PR against master to pick-up 938e209? I'm curious if this will tell us why the test is failing.

@nsait-linaro Could you please rebase this PR against master to pick-up 938e209? I'm curious if this will tell us why the test is failing.

Looks like still timing out

Checking https://mirror.bazel.build/zulu/bin/zulu17.30.15-ca-jdk17.0.1-macosx_aarch64.tar.gz ...
curl: (7) Failed to connect to mirror.bazel.build port 443: Connection timed out

@philwo
Copy link
Member

philwo commented Feb 4, 2022

Nice, at least now we get an error message. I'll ask the RBE folks for help.

@philwo
Copy link
Member

philwo commented Feb 4, 2022

I filed a bug asking for help, will follow-up as soon as I hear back!

@comius comius assigned philwo and unassigned comius Feb 8, 2022
@niyas-sait
Copy link
Contributor Author

I filed a bug asking for help, will follow-up as soon as I hear back!

@philwo Is CI issue specific to this patch ? I just retriggered and still seems to have the same issue

@meteorcloudy
Copy link
Member

@nsait-linaro Yes, because this PR adds more URLs, which I think happen to triggered some traffic limit.

@meteorcloudy
Copy link
Member

While the RBE team is looking into this (which looks like going to take some time), one mitigation I can think of is to deduplicate the urls we parsed from files:
https://cs.opensource.google/bazel/bazel/+/master:src/test/shell/bazel/verify_workspace.sh;l=50, so that we have less curl requests.

@philwo
Copy link
Member

philwo commented Feb 8, 2022

@meteorcloudy Yes, I also looked into this and one easy win would be to update all our other OpenJDKs to the latest version - we have at least one case where we have two separate versions for the same platform in the file. This should save some URLs 😅

Deduping the list would be a good improvement for the test anyway, great idea!

@meteorcloudy
Copy link
Member

Deduping the list would be a good improvement for the test anyway, great idea!

@nsait-linaro Do you mind trying to fix the verify_workspace test? I'm quite overloaded currently.

@niyas-sait
Copy link
Contributor Author

Deduping the list would be a good improvement for the test anyway, great idea!

@nsait-linaro Do you mind trying to fix the verify_workspace test? I'm quite overloaded currently.

Yes of course!

@niyas-sait
Copy link
Contributor Author

niyas-sait commented Feb 8, 2022

Deduping the list would be a good improvement for the test anyway, great idea!

@nsait-linaro Do you mind trying to fix the verify_workspace test? I'm quite overloaded currently.

Yes of course!

Hopefully, #14763 should fix!

bazel-io pushed a commit that referenced this pull request Feb 9, 2022
#14700 added couple more URLs to fetch JDK package and seems to be causing some infrastructure as discussed in #14700.

This patch workaround the issue by removing the duplicated URLs and reduce the crawl request.

Closes #14763.

PiperOrigin-RevId: 427464876
WORKSPACE Outdated Show resolved Hide resolved
distdir_deps.bzl Show resolved Hide resolved
@bazel-io bazel-io closed this in b602425 Feb 10, 2022
@niyas-sait
Copy link
Contributor Author

Thanks, @meteorcloudy, and @philwo.

Looking for the per-commit build for validation, Is it still at the same location? I've tried with the latest commit but cannot find anything there.

https://storage.googleapis.com/bazel-builds/artifacts/windows_arm64/b6024258c5b9b396b022bad6e7e3d64423048289/bazel.exe

@meteorcloudy
Copy link
Member

@nsait-linaro Thanks for the contribution!
There is a roughly 15 mins delay to publish the binary, the link is at (still without .exe ;)):
https://storage.googleapis.com/bazel-builds/artifacts/windows_arm64/b6024258c5b9b396b022bad6e7e3d64423048289/bazel

@niyas-sait
Copy link
Contributor Author

Thanks, @meteorcloudy, I can confirm that java and c++ builds are working fine now. It will be nice to get this delivered with the next Bazel release with the .exe suffix :-)

@meteorcloudy
Copy link
Member

@bazel-io fork 5.1

@meteorcloudy
Copy link
Member

@nsait-linaro That's nice! Can you send a PR to cherry-pick Windows ARM64 related changes to Bazel 5.1? I have opened issues to track the progress.

the next Bazel release with the .exe suffix

Yes, the official release will of course come with ".exe" suffix.

niyas-sait added a commit to niyas-sait/bazel that referenced this pull request Feb 11, 2022
Extend configurations to add JDK 11 and 17 for windows/arm64 platforms.

This should fix the Java builds on windows/arm64

Closes bazelbuild#14700.

PiperOrigin-RevId: 427737536
niyas-sait added a commit to niyas-sait/bazel that referenced this pull request Feb 11, 2022
bazelbuild#14700 added couple more URLs to fetch JDK package and seems to be causing some infrastructure as discussed in bazelbuild#14700.

This patch workaround the issue by removing the duplicated URLs and reduce the crawl request.

Closes bazelbuild#14763.

PiperOrigin-RevId: 427464876
meteorcloudy pushed a commit that referenced this pull request Feb 14, 2022
* Enable native support for Windows on arm64 (Part 1)

Contains following changes to third_party:

 - Extended def_parser to handle ARM64 binaries
 - Add grpc patch to workaround build issues

Closes: #14689

Partial commit for third_party/*, see #14689.

Signed-off-by: Yun Peng <[email protected]>

* Enable native support for Windows on arm64

This PR will enable cross-compilation of Bazel binaries for win/arm64 from win/x64

`bazel build -c opt --cpu=x64_arm64_windows //src:bazel.exe`

Generated bazel executable can be used for native compilation in win/arm64

`bazel.exe build //main:hello-world`

Following changes are included

- Add win/arm64 JDK 17
- Fix AutoCpuConverter.java to identify win/arm64 CPU
- Extend build_bazel_binaries.yml to cross-compile for win/arm64
- Fix msvc toolchain to look for tools in HostX86 directory as well
- add clang-cl support for windows/arm64 host
- Extend host_windows config to handle windows x64 and arm64 hosts.

Closes #14340.

PiperOrigin-RevId: 425919351

* [windows/arm64] Add missing JDK toolchain for java build

Extend configurations to add JDK 11 and 17 for windows/arm64 platforms.

This should fix the Java builds on windows/arm64

Closes #14700.

PiperOrigin-RevId: 427737536

* add missing openjdk11_windows_arm64_archive

* Deduplicate urls parsed to reduce crawl requests

#14700 added couple more URLs to fetch JDK package and seems to be causing some infrastructure as discussed in #14700.

This patch workaround the issue by removing the duplicated URLs and reduce the crawl request.

Closes #14763.

PiperOrigin-RevId: 427464876

* fix jdk_http_archives.tmpl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-Java Issues for Java rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants