Toolchainize //scala:toolchain_type

[mbland]

Description

Moves the toolchain targets for //scala:toolchain_type to a new @io_bazel_rules_scala_toolchains repository as a step towards Bzlmodification. Part of #1482.

Motivation

Instantiating toolchains in their own repository enables module extensions to define the the repositories required by those toolchains within the extension's own namespace. Bzlmod users can then register the toolchains from this repository without having to import all the toolchains' dependencies into their own namespace via use_repo().

---

The scala_toolchains_repo() macro wraps the underlying repository rule and assigns it the standard name io_bazel_rules_scala_toolchains. Right now it's only instantiating the main Scala toolchain via the default scala = True parameter. Future changes will expand this macro and repository rule with more boolean parameters to instantiate other toolchains, specifically:

• scalatest
• junit
• specs2
• twitter_scrooge
• jmh
• scala_proto and scala_proto_enable_all_options
• testing (includes all of scalatest, junit, and specs2)
• scalafmt

---

WORKSPACE users will now have to import and call the scala_toolchains_repo() macro to instantiate @io_bazel_rules_scala_toolchains.

load("@io_bazel_rules_scala//:scala_config.bzl", "scala_config")

scala_config()

load(
    "//scala:scala.bzl",
    "rules_scala_setup",
    "rules_scala_toolchain_deps_repositories",
    "scala_toolchains_repo",
)

rules_scala_setup()

rules_scala_toolchain_deps_repositories()

scala_toolchains_repo()

register_toolchains("@io_bazel_rules_scala_toolchains//...:all")

Or, they can use the new single scala_toolchains() macro to elide most of the calls:

load("@io_bazel_rules_scala//:scala_config.bzl", "scala_config")

scala_config()

load("//scala:scala.bzl", "scala_toolchains")

scala_toolchains()

register_toolchains("@io_bazel_rules_scala_toolchains//...:all")

This is what the corresponding MODULE.bazel setup would look like:

module(name = "rules_scala", version = "7.0.0")

scala_config = use_extension(
    "//scala/extensions:config.bzl", "scala_config"
)
scala_config.settings(scala_version = "2.13.14")

scala_deps = use_extension("//scala/extensions:deps.bzl", "scala_deps")
scala_deps.toolchains()

The register_toolchains() call in WORKSPACE isn't strictly required at this point, but is recommended. All the WORKSPACE files in this repo already registered their required toolchains using existing macros. However, they've been updated to use register_toolchains(...) instead.

These register_toolchains() calls maintain the order of previous toolchain registrations to avoid the following breakages:

• test_compilation_fails_with_plus_one_deps_undefined from test/shell/test_compilation.sh depends on //scala:unused_dependency_checker_error_toolchain resolving first. This toolchains sets the scala_toolchain() parameters dependency_tracking_method = "ast-plus" and unused_dependency_checker_mode = "error", and the @io_bazel_rules_scala_toolchains//scala toolchains don't.

• test_scala_binary_allows_opt_in_to_use_of_argument_file_in_runner_for_improved_performance from test/shell/test_scala_binary.sh depends on the use_argument_file_in_runner parameter of scala_toolchain being False. This is the default, but the @io_bazel_rules_scala_toolchains//scala toolchains explicitly set this to True instead.

In the Bzlmod case, the register_toolchains() call isn't necessary at all. This is because @io_bazel_rules_scala_toolchains includes one package per set of toolchains, and the rules_scala MODULE.bazel calls register_toolchains("@io_bazel_rules_scala_toolchains//...:all"). This will automatically register all configured rules_scala toolchains, while allowing users to override any of them using register_toolchains() in their own MODULE.bazel files.

Technically, the scala_deps.toolchains() call isn't required when only using the default scala = True parameter; the rules_scala MODULE.bazel will instantiate this automatically, too.

[mbland]

I've rebased this PR onto master after #1635 landed, updated it substantially with two new commits, and adjusted the PR description.

The first new commit introduces the new scala_toolchains() macro, which will be the common funnel point between WORKSPACE and Bzlmod. However, that first commit doesn't add the macro to existing WORKSPACE calls. This demonstrates that the changes to rules_scala_setup and scala_repositories are compatible with existing API usage.

The second commit does replace as many WORKSPACE calls as possible with scala_toolchains() and register_toolchains(). This shows that the new API produces equivalent results, and proves a clear path towards adding the Bzlmod layer.

If you'd like to see the end state I have in mind for this macro, the scala_toolchains_repo() macro/rule, and the Bzlmod extension, check out the equivalent files in my Bzlmod working branch:

• scala/private/macros/toolchains.bzl
• scala/private/macros/toolchains_repo.bzl
• scala/extensions/deps.bzl

I've tested that all the following work with both the first commit and the second:

• test_lint.sh
• test_all.sh
• test_examples.sh
• test_coverage.sh
• dt_patches/dt_patch_test.sh (after cherry-picking into my local branch the commit from #1632)

Once this change lands, toolchainizing the remaining toolchains should prove very straightforward. Then Bzlmod is possibly only PR away after that.

[simuons]

Hi, @mbland, I just want to let you know that I'm taking a look at this. Whole last week I was away, sorry for delay. Give me couple of days on this. I want to understand how final api in bzlmod would look like.

[mbland]

@simuons No worries. Thanks for the heads-up.

FWIW, now that I've got Bzlmod and WORKSPACE working together in my Bzlmod working branch, you can compare each WORKSPACE/MODULE.bazel pair in that branch. The repos under examples/ are probably the best representation of what I'd expect most users to end up with. The dt_patches/test_dt_patches_user_srcjar/MODULE.bazel file shows how it looks to register compiler srcjars.

[simuons]

I just want to share my ideas/concerns/struggles I've got while looking at this PR (reason it took longer than needed to review). By no means these are blockers or something that needs to be addressed in this PR:

1. Number of internal repositories keeps growing. Maybe these could be merged in bzlmod (probably not possible in workspace). For example I have a feeling that scala_config repository should be merged into toolchains as those are closely related. Maybe this is not an issue at all.
2. There are too many scala toolchains. Comment about toolchains ordering is concerning. Issue is that features orthogonal to scala version are configured on toolchain. Ideally there should be single toolchain per scala version. Maybe I'll continue exploration on https://github.com/bazelbuild/rules_scala/pull/1570

Maybe it's a wrong place/time for this, but I just wanted to share so I won't forget it. Or maybe I just want to get second opinion on this :)

[mbland]

I just want to share my ideas/concerns/struggles I've got while looking at this PR (reason it took longer than needed to review). By no means these are blockers or something that needs to be addressed in this PR:

No worries! It's important to get this right.

BTW, I'm replying now as I run the full test suite again. Will push my updates and ping when that's all done.

1. Number of internal repositories keeps growing. Maybe these could be merged in bzlmod (probably not possible in workspace). For example I have a feeling that scala_config repository should be merged into toolchains as those are closely related. Maybe this is not an issue at all.

I've wondered about this, but haven't reached a conclusion. scala_config defines constants that are then imported everywhere. It may require a significant refactor to accommodate a different schema.

2. There are too many scala toolchains. Comment about toolchains ordering is concerning. Issue is that features orthogonal to scala version are configured on toolchain. Ideally there should be single toolchain per scala version. Maybe I'll continue exploration on Configure rules behaviour/toolchain attributes via build settings #1570

Yeah, I haven't thought about that specifically, but I see what you mean, like the unused deps toolchain. That said, I don't think this change introduces any new toolchains from what was already present; it just repackages them to enable a proper Bzlmod API.

Maybe it's a wrong place/time for this, but I just wanted to share so I won't forget it. Or maybe I just want to get second opinion on this :)

No worries! Good to start thinking these through as the notion arises.

[mbland]

OK, everything's updated, tests are all passing, and I've pushed the changes to the repo. Ready for the next round!

[simuons]

Hi, @mbland, overall looks good. Let's settle on rules_scala_setup question and I think we are good to go.

[mbland]

@simuons In addition to extracting load_rules_dependencies, I just noticed that I could restore all those setup_scala_toolchains() calls in the WORKSPACE files, since we updated that macro to call native.register_toolchains("@io_bazel_rules_scala_toolchains//...:all").

If you want, I'm happy to update this PR accordingly, do it in another PR, or let it go (since all these updates are internal, existing users probably won't notice, and we're introducing the scala_toolchains macro that'll require a documentation update anyway).