Skip to content

Add toolchain options API to WORKSPACE and Bzlmod #1730

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mbland
Copy link
Contributor

@mbland mbland commented Apr 29, 2025

Description

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 #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.

Motivation

Inspired by @simuons's suggestion to replace toolchain macros with a module extension in:

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 Enable Bzlmod #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.

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.
@mbland mbland requested review from liucijus and simuons as code owners April 29, 2025 12:06
@mbland
Copy link
Contributor Author

mbland commented Apr 29, 2025

@simuons @liucijus I'd consider this the last essential change before releasing the next version, since it updates both the WORKSPACE and Bzlmod APIs.

mbland added 3 commits April 29, 2025 15:57
Touched up documentation for the `scalafmt` and `scala_proto`
toolchains.
This avoids the need for the user to use `exports_files` so
`@rules_scala_toolchains//scalafmt:config` can access the config file.
Essentially restores the API from before bazel-contrib#1725, but still fixes the same
bug as bazel-contrib#1725.
Updates the Scalafmt documentation to reflect the current API. Adds a
check to `scala_toolchains_repo` to `fail` if the Scalafmt
`default_config` file doesn't exist.

The previous commit doesn't actually restore the exact pre-bazel-contrib#1725 API.
It eliminates the `exports_files` requirement, but still requires a
`Label` or a relative path string, not an optional `.scalafmt.conf` in
the root directory.

After experimenting a bit and thinking this through, dropping support
for an optional `.scalafmt.conf` provides the most robust and reliable
interface. Specifically, supporting it requires detecting whether it
actually exists before falling back to the default. Having users
explicitly specify their own config seems a small burden to impose for
a more straightforward and correct implementation.

At the same time, I saw the opportunity to provide the user with
explicit feedback if the specified config file doesn't exist. Hence the
new check and `fail()` message.

Also renamed the generated `.scalafmt.conf` file in
`@rules_scala_toolchains//scalafmt` to `scalafmt.conf`. No need for it
to be a hidden file in that context.
@mbland mbland force-pushed the bzlmod-toolchains-extension branch from 5e15e88 to d30ddbb Compare April 30, 2025 17:40
The `scala_deps` docstring now more accurately describes the behavior
implemented by `_toolchain_settings()`.
@mbland
Copy link
Contributor Author

mbland commented Apr 30, 2025

@meteorcloudy I've noticed the cla/google check hanging today when pushing commits to this branch. Is this a consequence of the move to the bazel-contrib org?

@mbland mbland force-pushed the bzlmod-toolchains-extension branch from 82e3f4e to 6588c32 Compare May 1, 2025 15:44
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant