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

Upgrade to Bazel 5 (again) #17319

Merged
merged 15 commits into from
Sep 1, 2023
Merged

Upgrade to Bazel 5 (again) #17319

merged 15 commits into from
Sep 1, 2023

Conversation

akrmn
Copy link
Contributor

@akrmn akrmn commented Aug 24, 2023

This PR is based on @moritzkiefer-da's unmerged work in #12935 for upgrading to Bazel 5.

@akrmn
Copy link
Contributor Author

akrmn commented Aug 24, 2023

bazelbuild/bazel#14848 is still a problem, not sure how to work around it

@akrmn akrmn closed this Aug 24, 2023
@akrmn akrmn reopened this Aug 24, 2023
@akrmn
Copy link
Contributor Author

akrmn commented Aug 24, 2023

let's see how bad it gets without --distinct_host_configuration=false, since it is deprecated/a noop/removed in Bazel 5/6/7 (resp.) anyway

@akrmn
Copy link
Contributor Author

akrmn commented Aug 24, 2023

hm, I was able to get past the cannot find symbol var ... errors locally with the .bazelrc changes (build --java_language_version=11; build --java_runtime_version=11), but they're still showing on Linux CI. mayybe a caching issue?

@akrmn akrmn force-pushed the akrmn/bazel-5 branch 2 times, most recently from f6fa8dc to 940d2cb Compare August 28, 2023 08:57
.bazelrc Outdated
@@ -1,7 +1,7 @@
# Bazel distributed cache, can be temporarily disabled by passing the following
# flag: --noremote_accept_cached
build:darwin --remote_http_cache=https://bazel-cache.da-ext.net
build:linux --remote_http_cache=https://bazel-cache.da-ext.net/ubuntu_20_04
build:darwin --remote_cache=https://bazel-cache.da-ext.net/bump
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're doing that please take the opportunity to clean this up a bit and go for something like /macos/202308 & /ubuntu/202308.

I'd really like if we could have some sort of shell escape in .bazelrc to make this /$(uname -s)/$(date -I | cut -c 1-7) or something. Last I checked the Bazel devs didn't believe in templating .bazelrc though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the suggestion, if it works with /bump I'll clean up as you described

Copy link
Contributor Author

Choose a reason for hiding this comment

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

since this built on windows I'm confident it should also build on macos and linux, so I applied this change. for the windows cache, I removed my /bump and instead increased -v{13=>14} (the big comment in ci/configure-bazel.sh made me worry that a cleaner (but longer) suffix might break things on windows)

@akrmn
Copy link
Contributor Author

akrmn commented Aug 28, 2023

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@akrmn
Copy link
Contributor Author

akrmn commented Aug 29, 2023

the linux build appeared to fail on //ledger-service/http-json:integration-tests-ce_test_suite_src_it_scala_http_WebsocketServiceIntegrationTest.scala, but it passes locally. The windows and macOS builds were killed after running for 3h, so maybe it was also a timeout on linux? @garyverhaegen-da do you think it would be possible to give this PR more time to build?

@garyverhaegen-da
Copy link
Contributor

Running out of time is definitely expected in a "blank cache" run. Timeout is controlled by the YAML files (linux, macos, windows), so one option is to increase them in this branch.

Alternatively, subsequent builds should benefit from the cache, so "keep rerunning" should work. I've restarted the current build.

@akrmn akrmn force-pushed the akrmn/bazel-5 branch 2 times, most recently from a72bddb to 8f3e5ed Compare August 30, 2023 09:03
@akrmn
Copy link
Contributor Author

akrmn commented Aug 30, 2023

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

patches = oldAttrs.patches ++ [
# This should be upstreamed. Bazel is too aggressive
# in treating arguments starting with @ as response files.
./bazel-cc-wrapper-response-file.patch
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fix has been upstreamed and is available from Bazel 5.1.0
bazelbuild/bazel@48b60d2

@akrmn
Copy link
Contributor Author

akrmn commented Aug 31, 2023

@garyverhaegen-da should we undo the timeout bump in this PR after the macOS build finishes? or do you think it's safer to do it in a separate PR after this is merged?

@garyverhaegen-da
Copy link
Contributor

@garyverhaegen-da should we undo the timeout bump in this PR after the macOS build finishes? or do you think it's safer to do it in a separate PR after this is merged?

I believe Windows uses a different cache for main branch builds, so it will probably be less painful to leave the expanded timeouts in this PR and remove them in a follow-up one.

build --java_language_version=11
build --java_runtime_version=nixpkgs_java_11
build --tool_java_runtime_version=nixpkgs_java_11
build --tool_java_language_version=11
Copy link
Contributor

Choose a reason for hiding this comment

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

So much nicer.

Copy link
Contributor

@garyverhaegen-da garyverhaegen-da left a comment

Choose a reason for hiding this comment

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

Huge thanks for taking this on!

@akrmn akrmn merged commit 6ae0e01 into main Sep 1, 2023
18 checks passed
@akrmn akrmn deleted the akrmn/bazel-5 branch September 1, 2023 13:15
remyhaemmerle-da added a commit that referenced this pull request Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants