Toolchainize twitter_scrooge #1693
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is the last of the toolchain to receive the "toolchainization" treatment prior to Bzlmodification, and moves `twitter_scrooge()` to `twitter_scrooge/toolchain/toolchain.bzl` for `rules_java` 8 compatibility. Part of bazelbuild#1482 and bazelbuild#1652.
simuons
reviewed
Feb 11, 2025
scala/toolchains.bzl
Outdated
| jmh = False): | ||
| jmh = False, | ||
| twitter_scrooge = False, | ||
| libthrift = None, |
There was a problem hiding this comment.
Could libthrift, scrooge_core, scrooge_generator, util_core and util_logging be passed as a twitter_scrooge_deps dict/struct?
I know it doesn't sound particularly nice but parameter list is getting quite big. It would kinda match scalafmt and scala_proto pattern where one parameter enables toolchain and next one passes some config.
I assume in bzl_mod each toolchain will get its own tag_class with proper attrs.
There was a problem hiding this comment.
Good idea. Done in c90ec75.
Also, as I started doing this, I noticed these arguments were never really propagating properly. The new commit fixes that as well.
There was a problem hiding this comment.
Hmm, ./test_reproducibility.sh on macOS in build #5350 appears to have failed during git checkout. You might want to restart that task, but all the other builds seem fine.
cd /Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala
$ cd /usr/local/var/bazelbuild
# Updating existing repository mirror to find commit c90ec7567ec828e1d68303535b50104dbf5ae4ec
⚠️ Warning: Checkout failed! getting/updating git mirror: setting remote URL: exit status 71 (Attempt 1/3 Retrying in 2s)
# Removing /Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala
# Creating "/Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala"
$ cd /Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala
$ cd /usr/local/var/bazelbuild
# Updating existing repository mirror to find commit c90ec7567ec828e1d68303535b50104dbf5ae4ec
⚠️ Warning: Checkout failed! getting/updating git mirror: setting remote URL: exit status 71 (Attempt 2/3 Retrying in 2s)
# Removing /Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala
# Creating "/Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala"
$ cd /Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala
$ cd /usr/local/var/bazelbuild
# Updating existing repository mirror to find commit c90ec7567ec828e1d68303535b50104dbf5ae4ec
⚠️ Warning: Checkout failed! getting/updating git mirror: setting remote URL: exit status 71 (Attempt 3/3)
# Removing /Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala
# Creating "/Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala"
$ cd /Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala
🚨 Error: getting/updating git mirror: setting remote URL: exit status 71There was a problem hiding this comment.
Hi. Could you please push a commit to trigger build (empty commit will do). It's a shame to say that but I don't have permissions retry builds.
There was a problem hiding this comment.
Huh. Today I Learned about git commit --allow-empty. Done in 19db328, and all is passing now.
As requested by @simuons in bazelbuild#1693, `scala_toolchains` now receives `twitter_scrooge` options as a `dict`. Since all of these options are for alternative toolchain dependencies, I've called this new `dict` argument `twitter_scrooge_deps`. However, after looking closely at how the options propagated, I realized they never made it to the toolchain generated by `scala_toolchains`. So this change also passes these options all the way through to the generated `BUILD` file and through the `setup_scala_toolchain()` call.
Seemingly spurious failure during `git checkout`: - https://buildkite.com/organizations/bazel/pipelines/rules-scala-scala/builds/5350/jobs/0194f660-94eb-464a-a81d-9d1c576c2968/log ```txt cd /Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala $ cd /usr/local/var/bazelbuild⚠️ Warning: Checkout failed! getting/updating git mirror: setting remote URL: exit status 71 (Attempt 1/3 Retrying in 2s) $ cd /Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala $ cd /usr/local/var/bazelbuild⚠️ Warning: Checkout failed! getting/updating git mirror: setting remote URL: exit status 71 (Attempt 2/3 Retrying in 2s) $ cd /Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala $ cd /usr/local/var/bazelbuild⚠️ Warning: Checkout failed! getting/updating git mirror: setting remote URL: exit status 71 (Attempt 3/3) $ cd /Users/buildkite/builds/bk-macos-intel-ggrd/bazel/rules-scala-scala 🚨 Error: getting/updating git mirror: setting remote URL: exit status 71 ```
simuons
approved these changes
Feb 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds
twitter_scroogetoscala_toolchains(). Part of #1482 and #1652.Motivation
This is the last of the toolchains to receive the "toolchainization" treatment prior to Bzlmodification, and moves
twitter_scrooge()totwitter_scrooge/toolchain/toolchain.bzlforrules_java8 compatibility.