Skip to content

Commit 45f3c54

Browse files
committed
Add toolchain options API to WORKSPACE and Bzlmod
Updates `scala_toolchains()` to accept either boolean or dict arguments for specific toolchains, and updates `//scala/extensions:deps.bzl` to generate them from tag classes. Part of bazel-contrib#1482. Notable qualities: - Adds toolchain options support to the `scala_toolchains()` parameters `scalafmt`, `scala_proto`, and `twitter_scrooge`, and to the `scalafmt` tag class. - Eliminates the `scalafmt_default_config`, `scala_proto_options`, and `twitter_scrooge_deps` option parameters from `scala_toolchains()`. - Provides uniform, strict evaluation and validation of toolchain options passed to `scala_toolchains()`. - Configures enabled toolchains using root module settings or the default toolchain settings only. - Introduces the shared `TOOLCHAIN_DEFAULTS` dict in `//scala/private:toolchains_defaults.bzl` to aggregate individual `TOOLCHAIN_DEFAULTS` macro parameter dicts. This change also: - Replaces the non-dev dependency `scala_deps.scala()` tag instantiation in `MODULE.bazel` with `dev_deps.scala()`. - Renames the `options` parameter of the `scala_deps.scala_proto` tag class to `default_gen_opts` to match `setup_scala_proto_toolchain()`. - Introduces `_stringify_args()` to easily pass all toolchain macro args compiled from `scala_toolchains_repo()` attributes through to the generated `BUILD` file. - Extracts `_DEFAULT_TOOLCHAINS_REPO_NAME` and removes the `scala_toolchains_repo()` macro. - Includes docstrings for the new private implementation functions, and updates all other docstrings, `README.md`, and other relevant documentation accordingly. --- Inspired by @simuons's suggestion to replace toolchain macros with a module extension in: - bazel-contrib#1722 (comment) Though a full replacement is a ways off, this is a step in that direction that surfaced several immediate improvements. First, extensibility and maintainability: - The new implementation enables adding options support for other toolchains in the future while maintaining backward compatibility, for both the `WORKSPACE` and Bzlmod APIs. Adding this support will only require a minor release, not a major one. - The `scala_deps` module extension implementation is easier to read, since all toolchains now share the `_toolchain_settings` mechanism. Next, improved consistency of the API and implementation: - Toolchain options parameters should present all the same parameters as the macros to which they correspond, ensured by the `TOOLCHAIN_DEFAULTS` mechanism. This is to make it easier for users and maintainers to see the direct relationship between these separate sets of parameters. (They can also define additional parameters to those required by the macro, like `default_config` from the `scalafmt` options.) This principle drove the renaming of the `scala_deps.scala_proto` tag class parameter from `options` to `default_gen_opts`. It also inspired updating `scala_toolchains_repo()` to pass toolchain options through `_stringify_args()` to generate `BUILD` macro arguments. - The consolidated `TOOLCHAIN_DEFAULTS` dict reduces duplication between the `scala/extensions/deps.bzl` and `scala/toolchains.bzl` files. It ensures consistency between tag class `attr` default values for Bzlmod users and the `scala_toolchains()` default parameter values for `WORKSPACE` users. The `TOOLCHAINS_DEFAULTS` dicts corresponding to each toolchain macro do duplicate the information in the macro argument lists. However, the duplicated values in this case are physically adjacent to one another, minimizing the risk of drift. - Extracting `_DEFAULT_TOOLCHAINS_REPO_NAME` is a step towards enabling customized repositories based on the builtin toolchains, while specifying different options. This extraction revealed the fact that the `scala_toolchains_repo()` macro was no longer necessary, since only `scala_toolchains()` ever called it. Finally, fixes for the following design bugs: - Previously, `scala_deps.scala_proto(options = [...])` compiled the list of `default_gen_opts` from all tag instances in the module graph. This might've been convenient, but didn't generalize to other options for other toolchains. In particular, it differed from the previous `toolchains`, `scalafmt`, and `twitter_scrooge` tag class behavior. The new semantics are unambiguous, consistent, and apply to all toolchains equally; they do not show a preference for any one toolchain over the others. They also maintain the existing `scalafmt` and `twitter_scrooge` tag class semantics, but now using a generic mechanism, simplifying the implementation. - Instantating `scala_deps.scala()` was a bug left over from the decision in bazel-contrib#1722 _not_ to enable the builtin Scala toolchain by default under Bzlmod. The previous `scala_deps.toolchains()` tag class had a default `scala = True` parameter. The user could set `scala = False` to disable the builtin Scala toolchain. After replacing `toolchains()` with individual tag classes, the documented behavior was that the user must enable the builtin Scala toolchain by instantiating `scala_deps.scala()`. By instantiating `scala_deps.scala()` in our own `MODULE.bazel` file, we ensured that `rules_scala` would always instantiate the builtin Scala toolchain. While relatively harmless, it violated the intention of allowing the user to avoid instantiating the toolchain altogether.
1 parent 819449a commit 45f3c54

File tree

11 files changed

+195
-139
lines changed

11 files changed

+195
-139
lines changed

MODULE.bazel

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ use_repo(
7777
"rules_scala_toolchains",
7878
"scala_compiler_sources",
7979
)
80-
scala_deps.scala()
8180

8281
# Register some of our testing toolchains first when building our repo.
8382
register_toolchains(
@@ -101,6 +100,7 @@ dev_deps = use_extension(
101100
"scala_deps",
102101
dev_dependency = True,
103102
)
103+
dev_deps.scala()
104104
dev_deps.jmh()
105105
dev_deps.junit()
106106
dev_deps.scala_proto()

docs/phase_scalafmt.md

+1-2
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,7 @@ a custom configuration:
8686
# WORKSPACE
8787
scala_toolchains(
8888
# Other toolchains settings...
89-
scalafmt = True,
90-
scalafmt_default_config = "//path/to/my/custom:scalafmt.conf",
89+
scalafmt = {"default_config": "//path/to/my/custom:scalafmt.conf"},
9190
)
9291
```
9392

scala/extensions/deps.bzl

+43-44
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ Provides the `scala_deps` module extension with the following tag classes:
1414
- `twitter_scrooge`
1515
- `jmh`
1616
17-
For documentation, see the `_tag_classes` dict, and the `_<TAG>_attrs` dict
18-
corresponding to each `<TAG>` listed above.
17+
For documentation, see the `_{general,toolchain}_tag_classes` dicts and the
18+
`_<TAG>_attrs` dict corresponding to each `<TAG>` listed above.
1919
2020
See the `scala/private/macros/bzlmod.bzl` docstring for a description of
2121
the defaults, attrs, and tag class dictionaries pattern employed here.
@@ -27,6 +27,7 @@ load(
2727
"root_module_tags",
2828
"single_tag_values",
2929
)
30+
load("//scala/private:toolchain_defaults.bzl", "TOOLCHAIN_DEFAULTS")
3031
load("//scala:scala_cross_version.bzl", "default_maven_server_urls")
3132
load("//scala:toolchains.bzl", "scala_toolchains")
3233

@@ -89,9 +90,7 @@ _compiler_srcjar_attrs = {
8990
"integrity": attr.string(),
9091
}
9192

92-
_scalafmt_defaults = {
93-
"default_config": "//:.scalafmt.conf",
94-
}
93+
_scalafmt_defaults = TOOLCHAIN_DEFAULTS["scalafmt"]
9594

9695
_scalafmt_attrs = {
9796
"default_config": attr.label(
@@ -100,24 +99,16 @@ _scalafmt_attrs = {
10099
),
101100
}
102101

103-
_scala_proto_defaults = {
104-
"options": [],
105-
}
102+
_scala_proto_defaults = TOOLCHAIN_DEFAULTS["scala_proto"]
106103

107104
_scala_proto_attrs = {
108-
"options": attr.string_list(
109-
default = _scala_proto_defaults["options"],
105+
"default_gen_opts": attr.string_list(
106+
default = _scala_proto_defaults["default_gen_opts"],
110107
doc = "Protobuf options, like 'scala3_sources' or 'grpc'",
111108
),
112109
}
113110

114-
_twitter_scrooge_defaults = {
115-
"libthrift": None,
116-
"scrooge_core": None,
117-
"scrooge_generator": None,
118-
"util_core": None,
119-
"util_logging": None,
120-
}
111+
_twitter_scrooge_defaults = TOOLCHAIN_DEFAULTS["twitter_scrooge"]
121112

122113
_twitter_scrooge_attrs = {
123114
k: attr.label(default = v)
@@ -186,39 +177,51 @@ _toolchain_tag_classes = {
186177
),
187178
}
188179

189-
_tag_classes = _general_tag_classes | _toolchain_tag_classes
180+
def _toolchain_settings(module_ctx, tags, tc_names, toolchain_defaults):
181+
"""Configures all builtin toolchains enabled throughout the module graph.
182+
183+
Configures toolchain options for enabled toolchains that support them based
184+
on the root module's settings for each toolchain. In other words, it uses:
190185
191-
def _toolchains(mctx):
192-
result = {k: False for k in _toolchain_tag_classes}
186+
- the root module's tag class settings, if present; and
187+
- the default tag class settings otherwise.
193188
194-
for mod in mctx.modules:
195-
values = {tc: len(getattr(mod.tags, tc)) != 0 for tc in result}
189+
This avoids trying to reconcile different toolchain settings across the
190+
module graph. Non root modules that require specific settings should either:
196191
197-
if mod.is_root:
198-
return values
192+
- publish their required toolchain settings, or
193+
- define and register a custom toolchain instead.
199194
200-
# Don't overwrite `True` values with `False` from another tag.
201-
result.update({k: v for k, v in values.items() if v})
195+
Args:
196+
module_ctx: the module context object
197+
tags: a tags object, presumably the result of `root_module_tags()`
198+
tc_names: names of all supported toolchains
199+
toolchain_defaults: a dict of `{toolchain_name: default options dict}`
202200
203-
return result
201+
Returns:
202+
a dict of `{toolchain_name: bool or options dict}` to pass as keyword
203+
arguments to `scala_toolchains()`
204+
"""
205+
toolchains = {k: False for k in tc_names}
204206

205-
def _scala_proto_options(mctx):
206-
result = {}
207+
for mod in module_ctx.modules:
208+
values = {tc: len(getattr(mod.tags, tc)) != 0 for tc in toolchains}
207209

208-
for mod in mctx.modules:
209-
for tag in mod.tags.scala_proto:
210-
result.update({opt: True for opt in tag.options})
210+
# Don't overwrite True values with False from another tag.
211+
toolchains.update({k: v for k, v in values.items() if v})
211212

212-
return sorted(result.keys())
213+
for tc, defaults in toolchain_defaults.items():
214+
if toolchains[tc]:
215+
values = single_tag_values(module_ctx, getattr(tags, tc), defaults)
216+
toolchains[tc] = {k: v for k, v in values.items() if v != None}
217+
218+
return toolchains
219+
220+
_tag_classes = _general_tag_classes | _toolchain_tag_classes
213221

214222
def _scala_deps_impl(module_ctx):
215223
tags = root_module_tags(module_ctx, _tag_classes.keys())
216-
scalafmt = single_tag_values(module_ctx, tags.scalafmt, _scalafmt_defaults)
217-
scrooge_deps = single_tag_values(
218-
module_ctx,
219-
tags.twitter_scrooge,
220-
_twitter_scrooge_defaults,
221-
)
224+
tc_names = [tc for tc in _toolchain_tag_classes]
222225

223226
scala_toolchains(
224227
overridden_artifacts = repeated_tag_values(
@@ -229,13 +232,9 @@ def _scala_deps_impl(module_ctx):
229232
tags.compiler_srcjar,
230233
_compiler_srcjar_attrs,
231234
),
232-
scala_proto_options = _scala_proto_options(module_ctx),
233-
# `None` breaks the `attr.string_dict` in `scala_toolchains_repo`.
234-
twitter_scrooge_deps = {k: v for k, v in scrooge_deps.items() if v},
235235
**(
236236
single_tag_values(module_ctx, tags.settings, _settings_defaults) |
237-
{"scalafmt_%s" % k: v for k, v in scalafmt.items()} |
238-
_toolchains(module_ctx)
237+
_toolchain_settings(module_ctx, tags, tc_names, TOOLCHAIN_DEFAULTS)
239238
)
240239
)
241240

scala/private/toolchain_defaults.bzl

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
"""Gathers defaults for toolchain macros in one place.
2+
3+
Used by both //scala:toolchains.bzl and //scala/extensions:deps.bzl.
4+
"""
5+
6+
load(
7+
"//scala/scalafmt/toolchain:setup_scalafmt_toolchain.bzl",
8+
_scalafmt = "TOOLCHAIN_DEFAULTS",
9+
)
10+
load("//scala_proto:toolchains.bzl", _scala_proto = "TOOLCHAIN_DEFAULTS")
11+
load(
12+
"//twitter_scrooge/toolchain:toolchain.bzl",
13+
_twitter_scrooge = "TOOLCHAIN_DEFAULTS",
14+
)
15+
16+
TOOLCHAIN_DEFAULTS = {
17+
"scalafmt": _scalafmt,
18+
"scala_proto": _scala_proto,
19+
"twitter_scrooge": _twitter_scrooge,
20+
}

scala/scalafmt/toolchain/setup_scalafmt_toolchain.bzl

+7
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@ load("//scala:providers.bzl", "declare_deps_provider")
88
load("//scala:scala_cross_version.bzl", "version_suffix")
99
load("@rules_scala_config//:config.bzl", "SCALA_VERSIONS")
1010

11+
TOOLCHAIN_DEFAULTS = {
12+
# Used by `scala_toolchains{,_repo}` to generate
13+
# `@rules_scala_toolchains//scalafmt:config`, the default config for
14+
# `ext_scalafmt` from `phase_scalafmt_ext.bzl`.
15+
"default_config": Label("//:.scalafmt.conf"),
16+
}
17+
1118
def setup_scalafmt_toolchain(
1219
name,
1320
scalafmt_classpath,

scala/toolchains.bzl

+85-32
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ load(
77
"scala_version_artifact_ids",
88
"setup_scala_compiler_sources",
99
)
10+
load("//scala/private:toolchain_defaults.bzl", "TOOLCHAIN_DEFAULTS")
1011
load("//scala/scalafmt:scalafmt_repositories.bzl", "scalafmt_artifact_ids")
1112
load("//scala:scala_cross_version.bzl", "default_maven_server_urls")
1213
load("//scala:toolchains_repo.bzl", "scala_toolchains_repo")
@@ -18,14 +19,69 @@ load("//third_party/repositories:repositories.bzl", "repositories")
1819
load(
1920
"//twitter_scrooge/toolchain:toolchain.bzl",
2021
"twitter_scrooge_artifact_ids",
21-
_TWITTER_SCROOGE_DEPS = "TOOLCHAIN_DEPS",
2222
)
2323
load("@rules_scala_config//:config.bzl", "SCALA_VERSIONS")
2424

25-
def _get_unknown_entries(entries, allowed_entries):
26-
return [e for e in entries if e not in allowed_entries]
25+
_DEFAULT_TOOLCHAINS_REPO_NAME = "rules_scala_toolchains"
26+
27+
def _toolchain_opts(tc_arg):
28+
"""Converts a toolchain parameter to a (bool, dict of options).
29+
30+
Used by `scala_toolchains` to parse toolchain arguments as True, False,
31+
None, or a dict of options.
32+
33+
Args:
34+
tc_arg: a bool, dict, or None
35+
36+
Returns:
37+
a bool indicating whether the toolchain is enabled, and a dict
38+
containing any provided toolchain options
39+
"""
40+
if tc_arg == False or tc_arg == None:
41+
return False, {}
42+
return True, ({} if tc_arg == True else tc_arg)
43+
44+
def _process_toolchain_options(toolchain_defaults, **kwargs):
45+
"""Checks the validity of toolchain options and provides defaults.
46+
47+
Updates each toolchain option dictionary with defaults for every missing
48+
entry.
49+
50+
Args:
51+
toolchain_defaults: a dict of `{toolchain_name: default options dict}`
52+
**kwargs: keyword arguments of the form `toolchain_name = options_dict`
53+
54+
Returns:
55+
a list of error messages for invalid toolchains or options
56+
"""
57+
errors = []
58+
59+
for tc, options in kwargs.items():
60+
defaults = toolchain_defaults.get(tc, None)
61+
62+
if defaults == None:
63+
errors.append("unknown toolchain or doesn't have defaults: " + tc)
64+
continue
65+
66+
unexpected = [a for a in options if a not in defaults]
67+
68+
if unexpected:
69+
plural = "s" if len(unexpected) != 1 else ""
70+
errors.append(
71+
"unexpected %s toolchain attribute%s: " % (tc, plural) +
72+
", ".join(unexpected),
73+
)
74+
75+
options.update({
76+
k: v
77+
for k, v in defaults.items()
78+
if k not in options and v != None
79+
})
80+
81+
return errors
2782

2883
def scala_toolchains(
84+
name = _DEFAULT_TOOLCHAINS_REPO_NAME,
2985
maven_servers = default_maven_server_urls(),
3086
overridden_artifacts = {},
3187
fetch_sources = False,
@@ -36,12 +92,9 @@ def scala_toolchains(
3692
junit = False,
3793
specs2 = False,
3894
scalafmt = False,
39-
scalafmt_default_config = Label("//:.scalafmt.conf"),
4095
scala_proto = False,
41-
scala_proto_options = [],
4296
jmh = False,
43-
twitter_scrooge = False,
44-
twitter_scrooge_deps = {}):
97+
twitter_scrooge = False):
4598
"""Instantiates rules_scala toolchains and all their dependencies.
4699
47100
Provides a unified interface to configuring `rules_scala` both directly in a
@@ -54,6 +107,7 @@ def scala_toolchains(
54107
All arguments are optional.
55108
56109
Args:
110+
name: Name of the generated toolchains repository
57111
maven_servers: Maven servers used to fetch dependency jar files
58112
overridden_artifacts: artifacts overriding the defaults for the
59113
configured Scala version, in the format:
@@ -82,28 +136,26 @@ def scala_toolchains(
82136
scalatest: whether to instantiate the ScalaTest toolchain
83137
junit: whether to instantiate the JUnit toolchain
84138
specs2: whether to instantiate the Specs2 JUnit toolchain
85-
scalafmt: whether to instantiate the Scalafmt toolchain
86-
scalafmt_default_config: the default config file for Scalafmt targets
87-
scala_proto: whether to instantiate the scala_proto toolchain
88-
scala_proto_options: protobuf options, like 'scala3_sources' or 'grpc';
89-
`scala_proto` must also be `True` for this to take effect
139+
scalafmt: boolean or dictionary of Scalafmt options:
140+
- default_config: default Scalafmt config file target
141+
scala_proto: boolean or dictionary of `setup_scala_proto_toolchain()`
142+
options
90143
jmh: whether to instantiate the Java Microbenchmarks Harness toolchain
91-
twitter_scrooge: whether to instantiate the twitter_scrooge toolchain
92-
twitter_scrooge_deps: dictionary of string to Label containing overrides
93-
for twitter_scrooge toolchain dependency providers with keys:
94-
libthrift
95-
scrooge_core
96-
scrooge_generator
97-
util_core
98-
util_logging
144+
twitter_scrooge: bool or dictionary of `setup_scrooge_toolchain()`
145+
options
99146
"""
100-
unknown_ts_deps = _get_unknown_entries(
101-
twitter_scrooge_deps,
102-
_TWITTER_SCROOGE_DEPS,
147+
scalafmt, scalafmt_options = _toolchain_opts(scalafmt)
148+
scala_proto, scala_proto_options = _toolchain_opts(scala_proto)
149+
twitter_scrooge, twitter_scrooge_options = _toolchain_opts(twitter_scrooge)
150+
151+
errors = _process_toolchain_options(
152+
TOOLCHAIN_DEFAULTS,
153+
scalafmt = scalafmt_options,
154+
scala_proto = scala_proto_options,
155+
twitter_scrooge = twitter_scrooge_options,
103156
)
104-
105-
if unknown_ts_deps:
106-
fail("unknown twitter_scrooge_deps:", ", ".join(unknown_ts_deps))
157+
if errors:
158+
fail("\n".join(errors))
107159

108160
setup_scala_compiler_sources(scala_compiler_srcjars)
109161

@@ -135,7 +187,7 @@ def scala_toolchains(
135187
if twitter_scrooge:
136188
artifact_ids_to_fetch_sources.update({
137189
id: False
138-
for id in twitter_scrooge_artifact_ids(**twitter_scrooge_deps)
190+
for id in twitter_scrooge_artifact_ids(**twitter_scrooge_options)
139191
})
140192

141193
for scala_version in SCALA_VERSIONS:
@@ -174,20 +226,21 @@ def scala_toolchains(
174226
)
175227

176228
scala_toolchains_repo(
229+
name = name,
177230
scalatest = scalatest,
178231
junit = junit,
179232
specs2 = specs2,
180233
scalafmt = scalafmt,
181-
scalafmt_default_config = scalafmt_default_config,
234+
scalafmt_default_config = scalafmt_options["default_config"],
182235
scala_proto = scala_proto,
183-
scala_proto_options = scala_proto_options,
236+
scala_proto_options = scala_proto_options["default_gen_opts"],
184237
jmh = jmh,
185238
twitter_scrooge = twitter_scrooge,
186-
twitter_scrooge_deps = twitter_scrooge_deps,
239+
twitter_scrooge_deps = twitter_scrooge_options,
187240
)
188241

189-
def scala_register_toolchains():
190-
native.register_toolchains("@rules_scala_toolchains//...:all")
242+
def scala_register_toolchains(name = _DEFAULT_TOOLCHAINS_REPO_NAME):
243+
native.register_toolchains("@%s//...:all" % name)
191244

192245
def scala_register_unused_deps_toolchains():
193246
native.register_toolchains(

0 commit comments

Comments
 (0)