Skip to content

Commit 77ce01c

Browse files
authored
Fix custom twitter_scrooge toolchain breakages (#1747)
* Fix custom `twitter_scrooge` toolchain breakages Adds the `examples/twitter_scrooge` repository, adds `test_twitter_scrooge_example` to `test/shell/test_examples.sh`, and enables overriding all `twitter_scrooge` dependencies. Fixes #1744. Adds `docs/twitter_scrooge.md` and includes extensive comments in `examples/twitter_scrooge/{BUILD,MODULE.bazel,WORKSPACE}`. Also updates `test/shell/test_examples.sh`: - It now uses `run_tests` from `test/shell/test_runner.sh`, and all test functions begin with `test_`. This enables automatic discovery and easy skipping of test cases (using the prefix `_test_`). - Replaces `test_example` with `run_in_example_dir`. This removes a subshell per test case, and allows for passing individually quoted arguments. --- Fixes two breakages related to overriding default `twitter_scrooge` dependencies under Bzlmod (neither problem manifested under `WORKSPACE). The first happened when using `setup_scrooge_toolchain` without instantiating the default Scala and `twitter_scrooge` toolchains and importing some of their repos. Even with all the available dependency parameters to `setup_scrooge_toolchain` specified, the build would break with errors like: ```txt no such package '@@[unknown repo 'io_bazel_rules_scala_scopt_2_13_16' requested from @@]//': The repository '@@[unknown repo 'io_bazel_rules_scala_scopt_2_13_16' requested from @@]' could not be resolved: No repository visible as '@io_bazel_rules_scala_scopt_2_13_16' from main repository and referenced by '//tools/scala:compiler_classpath_provider' ``` This was because `setup_scala_toolchain` still contained hardcoded references to `io_bazel_rules_scala_scopt` and other internal dependency repos. The workaround was to instantiate the builtin toolchains, then use `use_repo` to bring these internal repositories into scope. The fix was to make all of these repos overridable via `setup_scala_toolchain` and the `scala_deps.twitter_scrooge()` tag class. Now providing all the dependency arguments to `setup_scala_toolchain` avoids the need instantiate the builtin toolchains. The second breakage happened when specifying overrides for the `scala_deps.twitter_scrooge()` tag class: ```txt @@rules_scala+//scala/extensions:rules_scala++scala_deps+rules_scala_toolchains: expected value of type 'string' for dict value element, but got Label("@@rules_jvm_external++maven+maven//:org_apache_thrift_libthrift") ``` This happened because: - The `scala_deps` module extension compiled its `twitter_scrooge` tag class Labels into a dictionary. - It passed this `string` to `Label` dict to `scala_toolchains()`, which assigned it to the `twitter_scrooge_options` attribute of `scala_toolchains_repo`. - The `scala_toolchains_repo` attribute is of type `attr.string_dict`, not `attr.string_keyed_label_dict`. The fix was to translate the Labels in this dictionary into strings at the point of `scala_toolchains_repo` assignment. We don't officially support Bazel 6 anymore, but I didn't want to change the attribute to `string_keyed_label_dict` just yet. Things still work fine without it, and it gives Bazel 6 users a little more wiggle room and time to migrate. * Remove examples/twitter_scrooge/BUILD first line This is left over from my first draft, when I'd written the new tests in `test/shell/test_twitter_scrooge_toolchains.sh`.
1 parent 05ecd67 commit 77ce01c

File tree

16 files changed

+1166
-72
lines changed

16 files changed

+1166
-72
lines changed

.bazelignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ examples/semanticdb
1313
examples/testing/multi_frameworks_toolchain
1414
examples/testing/scalatest_repositories
1515
examples/testing/specs2_junit_repositories
16+
examples/twitter_scrooge
1617
test/proto_cross_repo_boundary/repo
1718
test_cross_build
1819
third_party/test/example_external_workspace

README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ This project defines core build rules for [Scala](https://www.scala-lang.org/) t
3030
- [scala_import](./docs/scala_import.md)
3131
- [scala_doc](./docs/scala_doc.md)
3232

33+
See the [docs](./docs/) directory for documentation on other `rules_scala`
34+
capabilities as well.
35+
3336
## Getting started
3437

3538
[Install Bazel][], preferably using the [Bazelisk][] wrapper. See the

docs/twitter_scrooge.md

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
# Using the `twitter_scrooge` toolchain and rules
2+
3+
## Rules
4+
5+
```py
6+
load("@rules_scala//thrift:thrift.bzl", "thrift_library")
7+
load(
8+
"@rules_scala//twitter_scrooge/toolchain:toolchain.bzl",
9+
"setup_scrooge_toolchain",
10+
)
11+
load(
12+
"@rules_scala//twitter_scrooge:twitter_scrooge.bzl",
13+
"scrooge_java_library",
14+
"scrooge_scala_library",
15+
)
16+
```
17+
18+
## Examples
19+
20+
The [`//test/src/main/scala/scalarules/test/twitter_scrooge`][] package provides
21+
extensive examples of `twitter_scrooge` rule usage.
22+
23+
## Toolchain configuration
24+
25+
### Default builtin toolchain
26+
27+
To use the builtin toolchain with its default dependencies under Bzlmod:
28+
29+
```py
30+
# MODULE.bazel
31+
32+
scala_deps = use_extension("//scala/extensions:deps.bzl", "scala_deps")
33+
dev_deps.scala()
34+
dev_deps.twitter_scrooge()
35+
```
36+
37+
And under `WORKSPACE`:
38+
39+
```py
40+
# WORKSPACE
41+
load(
42+
"@rules_scala//scala:toolchains.bzl",
43+
"scala_register_toolchains",
44+
"scala_toolchains",
45+
)
46+
47+
scala_toolchains(twitter_scrooge = True)
48+
49+
scala_register_toolchains()
50+
```
51+
52+
### Builtin toolchain dependency overrides
53+
54+
The [`examples/twitter_scrooge`][] repository shows how to configure the
55+
toolchains for `twitter_scrooge` rules in both [`MODULE.bazel`][] and
56+
[`WORKSPACE`][]. Both use [`rules_jvm_external`][] to import Maven artifacts for
57+
overriding the builtin `twitter_scrooge` toolchain defaults.
58+
59+
### Defining a custom toolchain
60+
61+
[`examples/twitter_scrooge/BUILD`][] shows how to use `setup_scrooge_toolchain`
62+
to define a custom `twitter_scrooge` toolchain with [`rules_jvm_external`][]
63+
artifacts.
64+
65+
### More information
66+
67+
See the comments in the above [`examples/twitter_scrooge`][] files for
68+
configuration details.
69+
70+
See the [Bazel manual on toolchain resolution](
71+
https://bazel.build/extending/toolchains#toolchain-resolution) for guidance on
72+
selecting a specific toolchain.
73+
74+
[`//test/src/main/scala/scalarules/test/twitter_scrooge`]: ../test/src/main/scala/scalarules/test/twitter_scrooge
75+
[`examples/twitter_scrooge`]: ../examples/twitter_scrooge/
76+
[`MODULE.bazel`]: ../examples/twitter_scrooge/MODULE.bazel
77+
[`WORKSPACE`]: ../examples/twitter_scrooge/WORKSPACE
78+
[`examples/twitter_scrooge/BUILD`]: ../examples/twitter_scrooge/BUILD
79+
[`rules_jvm_external`]: https://github.com/bazel-contrib/rules_jvm_external

examples/twitter_scrooge/.bazelrc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import ../../.bazelrc
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
7.6.1

examples/twitter_scrooge/BUILD

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# Targets adapted from //test/src/main/scala/scalarules/test/twitter_scrooge.
2+
load("@rules_scala//scala:scala.bzl", "scala_library")
3+
load("@rules_scala//thrift:thrift.bzl", "thrift_library")
4+
load(
5+
"@rules_scala//twitter_scrooge:twitter_scrooge.bzl",
6+
"scrooge_scala_library",
7+
)
8+
load(
9+
"@rules_scala//twitter_scrooge/toolchain:toolchain.bzl",
10+
"setup_scrooge_toolchain",
11+
)
12+
13+
# When using `setup_scrooge_toolchain` with all its dependencies specified, you
14+
# don't need to instantiate the builtin toolchain. In that case, make sure to
15+
# register your custom toolchain via `register_toolchains` in `MODULE.bazel` or
16+
# `WORKSPACE`. See the comments in those files for further details.
17+
#
18+
# It's OK to remove any of these overrides in order to use the builtin defaults
19+
# for those dependencies instead. However, in that case, you _must_ instantiate
20+
# the default `twitter_scrooge` toolchain in `MODULE.bazel` or `WORKSPACE`,
21+
# without the corresponding dependency overrides. This allows `rules_scala` to
22+
# generate the necessary builtin dependency repositories, even if you don't use
23+
# the default toolchain.
24+
#
25+
# However, if you remove the `scrooge_generator` override, the toolchain will
26+
# also depend on the builtin `mustache` and `scopt` repos. You will need to
27+
# remove the `mustache` and `scopt` overrides, too, to use the builtin repos
28+
# instead.
29+
setup_scrooge_toolchain(
30+
name = "toolchain_from_build_file",
31+
javax_annotation_api = "@maven//:javax_annotation_javax_annotation_api",
32+
libthrift = "@maven//:org_apache_thrift_libthrift",
33+
mustache = "@maven//:com_github_spullara_mustache_java_compiler",
34+
scopt = "@maven//:com_github_scopt_scopt_2_12",
35+
scrooge_core = "@maven//:com_twitter_scrooge_core_2_12",
36+
scrooge_generator = "@maven//:com_twitter_scrooge_generator_2_12",
37+
util_core = "@maven//:com_twitter_util_core_2_12",
38+
util_logging = "@maven//:com_twitter_util_logging_2_12",
39+
)
40+
41+
scala_library(
42+
name = "justscrooge",
43+
srcs = ["JustScrooge.scala"],
44+
exports = [":scrooge"],
45+
deps = [":scrooge"],
46+
)
47+
48+
scrooge_scala_library(
49+
name = "scrooge",
50+
visibility = ["//visibility:public"],
51+
deps = [":thrift"],
52+
)
53+
54+
thrift_library(
55+
name = "thrift",
56+
srcs = ["Thrift1.thrift"],
57+
visibility = ["//visibility:public"],
58+
deps = [":thrift2"],
59+
)
60+
61+
thrift_library(
62+
name = "thrift2",
63+
srcs = ["Thrift2.thrift"],
64+
visibility = ["//visibility:public"],
65+
)
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package examples.twitter_scrooge
2+
3+
object JustScrooge {
4+
val classes = Seq(classOf[Struct1])
5+
6+
def main(args: Array[String]) {
7+
print(s"classes ${classes.mkString(",")}")
8+
}
9+
}

examples/twitter_scrooge/MODULE.bazel

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
# Test configuration for test/shell/test_twitter_scrooge_toolchains.sh.
2+
module(name = "twitter_scrooge_toolchains")
3+
4+
bazel_dep(name = "rules_scala")
5+
local_path_override(
6+
module_name = "rules_scala",
7+
path = "../..",
8+
)
9+
10+
bazel_dep(name = "latest_dependencies")
11+
local_path_override(
12+
module_name = "latest_dependencies",
13+
path = "../../deps/latest",
14+
)
15+
16+
bazel_dep(
17+
name = "protobuf",
18+
version = "31.1",
19+
repo_name = "com_google_protobuf",
20+
)
21+
22+
# Temporarily required for `protoc` toolchainization until resolution of
23+
# protocolbuffers/protobuf#19679.
24+
single_version_override(
25+
module_name = "protobuf",
26+
patch_strip = 1,
27+
patches = ["//:protobuf.patch"],
28+
version = "31.1",
29+
)
30+
31+
bazel_dep(name = "rules_jvm_external", version = "6.7")
32+
33+
maven = use_extension("@rules_jvm_external//:extensions.bzl", "maven")
34+
maven.install(
35+
artifacts = [
36+
"com.github.scopt:scopt_2.12:4.0.0-RC2",
37+
"com.github.spullara.mustache.java:compiler:0.8.18",
38+
"com.twitter:scrooge-core_2.12:21.2.0",
39+
"com.twitter:scrooge-generator_2.12:21.2.0",
40+
"com.twitter:util-core_2.12:21.2.0",
41+
"com.twitter:util-logging_2.12:21.2.0",
42+
"javax.annotation:javax.annotation-api:1.3.2",
43+
"org.apache.thrift:libthrift:0.10.0",
44+
],
45+
lock_file = "//:maven_install.json",
46+
)
47+
use_repo(maven, "maven")
48+
49+
scala_protoc = use_extension(
50+
"@rules_scala//scala/extensions:protoc.bzl",
51+
"scala_protoc",
52+
dev_dependency = True,
53+
)
54+
use_repo(scala_protoc, "rules_scala_protoc_toolchains")
55+
56+
register_toolchains("@rules_scala_protoc_toolchains//...:all")
57+
58+
scala_config = use_extension(
59+
"@rules_scala//scala/extensions:config.bzl",
60+
"scala_config",
61+
)
62+
scala_config.settings(scala_version = "2.12.20")
63+
64+
scala_deps = use_extension(
65+
"@rules_scala//scala/extensions:deps.bzl",
66+
"scala_deps",
67+
dev_dependency = True,
68+
)
69+
scala_deps.scala()
70+
71+
# When using `setup_scrooge_toolchain` with all its dependencies specified in a
72+
# `BUILD` file, you don't need to instantiate this builtin toolchain. In that
73+
# case, make sure to register your custom toolchain via `register_toolchains`
74+
# (see below). See the `//:toolchain_from_build_file` comments in the `BUILD`
75+
# file for further details.
76+
#
77+
# It's OK to remove any of these overrides in order to use the builtin defaults
78+
# for those dependencies instead.
79+
#
80+
# However, if you remove the `scrooge_generator` override, the toolchain will
81+
# also depend on the builtin `mustache` and `scopt` repos. You will need to
82+
# remove the `mustache` and `scopt` overrides, too, to use the builtin repos
83+
# instead.
84+
scala_deps.twitter_scrooge(
85+
javax_annotation_api = "@maven//:javax_annotation_javax_annotation_api",
86+
libthrift = "@maven//:org_apache_thrift_libthrift",
87+
mustache = "@maven//:com_github_spullara_mustache_java_compiler",
88+
scopt = "@maven//:com_github_scopt_scopt_2_12",
89+
scrooge_core = "@maven//:com_twitter_scrooge_core_2_12",
90+
scrooge_generator = "@maven//:com_twitter_scrooge_generator_2_12",
91+
util_core = "@maven//:com_twitter_util_core_2_12",
92+
util_logging = "@maven//:com_twitter_util_logging_2_12",
93+
)
94+
95+
# If you want to depend on any of the builtin repos when using
96+
# `setup_scala_toolchain` in a `BUILD` file, you will need to:
97+
#
98+
# - Remove the `scala_deps.twitter_scrooge()` overrides for those repos. This
99+
# enables the module extension to generate the builtin repos for those
100+
# dependencies.
101+
#
102+
# - Uncomment the `use_repo` call below to import the builtin repos into the
103+
# main module's scope.
104+
#
105+
# `version_suffix` should match the `scala_version` argument to
106+
# `scala_config.settings()`.
107+
#version_suffix = "_2_12_20"
108+
#[
109+
# use_repo(scala_deps, repo + version_suffix)
110+
# for repo in [
111+
# "libthrift",
112+
# "io_bazel_rules_scala_scrooge_core",
113+
# "io_bazel_rules_scala_scrooge_generator",
114+
# "io_bazel_rules_scala_util_core",
115+
# "io_bazel_rules_scala_util_logging",
116+
# "io_bazel_rules_scala_javax_annotation_api",
117+
# "io_bazel_rules_scala_mustache",
118+
# "io_bazel_rules_scala_scopt",
119+
# ]
120+
#]
121+
122+
# To depend on the toolchain defined by `setup_scala_toolchain` by default,
123+
# instead of the builtin toolchain, uncomment this line. You can also specify it
124+
# on demand via:
125+
#
126+
# bazel build --extra_toolchains=//:toolchain_from_build_file //...
127+
#
128+
#register_toolchains("//:toolchain_from_build_file")
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
namespace java examples.twitter_scrooge
2+
3+
include "Thrift2.thrift"
4+
5+
struct Struct1 {
6+
1: Thrift2.Struct2 msg
7+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
namespace java examples.twitter_scrooge
2+
3+
struct Struct2 {
4+
1: string msg
5+
}

0 commit comments

Comments
 (0)