Enable Bzlmod

[mbland]

Description

All but finishes #1482, save for publication of the first rules_scala release to the Bazel Central Registry. Closes #1625.

The module extensions and underlying helper macros comprise the most significant part of the change. The extensions provide a thin layer over the existing WORKSPACE API. The scala/private/macros/bzlmod.bzl docstring explains the common pattern employed throughout.

• scala/extensions/config.bzl: invokes scala_config()
• scala/extensions/deps.bzl: invokes scala_toolchains()
• scala/extensions/protoc.bzl: invokes scala_protoc_toolchains()
• scala/private/extensions/dev_deps.bzl: internal testing dependencies
• scala/private/macros/bzlmod.bzl: module extension helper macros

Existing tests thoroughly validate config.bzl, deps.bzl, dev_deps.bzl, and protoc.bzl. test/shell/test_bzlmod_macros.sh tests the helpers from scala/private/macros/bzlmod.bzl using the new test files from scala/private/macros/test/....

Most of the new files are essentially boilerplate:

• Adds MODULE.bazel files mirroring existing WORKSPACE configurations.

• Adds symlinks to the protobuf toolchainization patch to every nested repo.

These changes update CI:

• Sets --noenable_workspace in .bazelrc and tools/bazel.rc.buildkite.

• Reenables the last_green Bazel build in .bazelci/presubmit.yml.

• Replaces //tools:lint_check in CI with test_lint.sh, which now runs test/shell/test_bzlmod_tidy.sh to lint MODULE.bazel files.

All other changes:

• Adds .bazelignore to work around Loading top-level targets in local_path_override modules in child directory breaks the build bazel#22208.

• Adds .bcr/ files for publishing to the Bazel Central Registry using the Publish to BCR GitHub app. See .bcr/README.md.

• Adds assert_matches to test/shell/test_helper.sh for use in test/shell/test_bzlmod_macros.sh.

• test/shell/test_bzlmod_macros.sh introduces a mechanism for automatically finding and skipping tests, documented in the comment at the bottom of the file.

• Adds bazel mod tidy checks to test_lint.sh and test_version.sh.

• Adds bazel clean --expunge_async to tests that create temporary repos.

• Adds the test_cleanup.sh script to reclaim disk space.

Motivation

This change enables Bazel 7 and 8 users to migrate their rules_scala dependency from WORKSPACE to MODULE.bazel whenever they're ready. It also ensures that new changes are compatible with the last_green build of Bazel, and enables publishing releases to the Bazel Central Registry.

The rest of the "Publish to BCR" workflow will come in a subsequent PR. Once it's ready, we can push a release tag to publish the first rules_scala Bazel module to https://registry.bazel.build/.

At that point, we can close #1482.

[mbland]

I've made a couple fixes and experimented a bit over the weekend. In the process, I've broken out the following branches based on this one, for after this pull request lands:

• bzlmod-bumps: contains version updates including protobuf 30.2
• test-src: Adds src/... to the bazel build and bazel test commands in test_rules_scala.sh to follow up on Add Java 24 compatibility #1719 and Cleanup WorkerTest after SecurityManager was removed #1721. Fixes a Windows line separator-related breakage.
• bzlmod-workflows: Updates the release workflow to eventually use the "Publish to BCR" reusable GitHub workflow, superseding the app, once it's ready.

This change is definitely larger than I'd like, even after accounting for the boilerplate:

$ git show --stat | grep ' | ' | wc -l
    85
$ git show --stat | grep ' | ' | grep 'WORKSPACE\.bzlmod' | wc -l
    16
$ git show --stat | grep ' | ' | grep 'MODULE.bazel' | wc -l
    17
$ git show --stat | grep ' | ' | grep 'protobuf.patch' | wc -l
    13
$ bc <<<'85 - 16 - 17 - 13'
    39

As always, I'm happy to break this into smaller pull requests if desired. For example, all the WORKSPACE.bzlmod and protobuf.patch files could land first without hurting anything. Same with .bazelignore, the .bcr files, and some of the other changes spelled out in the PR description.

[kormide]

Because this is under the bazelbuild org, you will need a .bcr/config.yml that sets a fixedReleaser so that the app knows which BCR fork to push a branch to to open the PR. The app needs to be installed to the ruleset repo as well as to the fork.

[mbland]

@kormide Thanks for the heads-up; I thought it was optional based on the docs. But now I'm trying to square that with @meteorcloudy's comment in #1616 (comment) indicating a fixed releaser won't be necessary if the repo moves before publishing:

I think bazel-contrib can accept the transfer now and it will make the publish to bcr process easier (a fixed releaser isn't necessary anymore).

So should I add the .bcr/config.yml file if we end up transferring the repo before publishing? Or can I just add it, and it won't harm anything later?

[kormide]

@kormide Thanks for the heads-up; I thought it was optional based on the docs. But now I'm trying to square that with @meteorcloudy's comment in #1616 (comment) indicating a fixed releaser won't be necessary if the repo moves before publishing:

I think bazel-contrib can accept the transfer now and it will make the publish to bcr process easier (a fixed releaser isn't necessary anymore).

So should I add the .bcr/config.yml file if we end up transferring the repo before publishing? Or can I just add it, and it won't harm anything later?

If you transfer the repo before publishing then it won't be necessary. After transferring, the app will default to using bazel-contrib's fork of the BCR. If you add the config file now and keep it, that override of which fork to use will remain.

[mbland]

@kormide Thanks for the clarification.

@simuons @liucijus If you'd rather publish the rules_scala module before moving this repo to bazel-contrib (#1616), I'll add the .bcr/config.yml file to this PR. Otherwise, I won't. Please let me know what you'd prefer!

[txdv]

@simuons @liucijus Is there a timeline when this will be merged?

[simuons]

@mbland I'm sorry this PR is hanging for two week now. I'll come to this in the beginning of the next week.

[simuons]

Hi, @mbland, would it be possible to somehow reuse protobuf.patch? As I understand it's the same in each module.

[simuons]

I'm trying to understand proposed design of how toolchains are defined.

As I understood scala_toolchains() was needed to gather all the toolchains into single repo, that was a prerequisite for bzlmod. So in that case bool flags made sense which didn't require to invent a complicated schema.

I think in bzlmod each boolean attr could be a separate tag_class with it's own set of attributes.

Do I understand correctly that defaults are configured in module file though extensions, but if users wants custom toolchain they define those via macros in bzl file?

Ideally users should be able to configure "everything" in a module file via extensions.

What I am missing?

Overall PR look amazing. Thanks for thorough documentation and extensive test suite.

I'm currently looking at proposed extensions and pattern of defining defaults, attrs, and tag which I don't yet fully understand.

[mbland]

The current implementation of the module extensions essentially does two things:

• Collects all the metadata from the graph of MODULE.bazel files. Most of them only care about the root module, and set reasonable defaults if the root module doesn't use the extension. (The rules_scala module _does_use them all, which means they'll always execute either way.)
• Calls scala_toolchains() with this metadata.

The idea is that the Bzlmod API will look and behave very similarly to the WORKSPACE API. The comparison between the two in the "New scala_toolchains() API for WORKSPACE" and "Bzlmod configuration" sections of README.md illustrates this.

We could have users instantiate a separate tag class per toolchain, so if you strongly prefer that API, I can update it accordingly. We do already have tag classes specifically for scalafmt and twitter_scrooge settings; all other potential tag classes wouldn't contain any members. In other words, instead of:

scala_deps.toolchains(                                                          
    scalafmt = True,
    scalatest = True,
) 

We'd have:

scala_deps.scalafmt()
scala_deps.scalatest()

I kinda prefer the existing version (not only because it's already implemented), but will do whatever you want with it here.

As far as trying to support user defined toolchains via module extensions, I don't believe that would work comfortably. User-defined toolchains rely on repos visible to the user's module, possibly via rules_jvm_external. If we wanted to generate them in our own module extension, the user would then likely have to use inject_repo to inject them all into the extension. It seems like it would complicate the UI with no tangible benefit.

Of course, I could be mistaken about that, and would be happy to try implementing that option if I am. Just let me know what you'd like to do either way!

[mbland]

On second thought, regarding having each toolchain have its own tag class: I'll try to experiment with that tomorrow.

I have a couple of other tweaks I'm going to play with tomorrow, too: updating the running of the new Bzlmod tests, and removing the empty WORKSPACE.bzlmod files since the .bazelrc flags already work well enough. I'll ping when I'm done with all of these little experiments and updates.

Regarding registering user toolchains through module extensions, I'm currently seeing more problems and fewer benefits. I'll keep thinking about it, but I'm still thinking that it makes more sense to leave the BUILD macro schema as is.

[simuons]

Regarding registering user toolchains through module extensions, I'm currently seeing more problems and fewer benefits.

Could you clarify what problems this could cause? Definitely you have deeper knowledge that I do at the moment and I might be missing something. My thinking was that we should expose unified interface for configuring toolchains instead of configuring defaults via module extension and for advanced uses resort to macros in build file. Workspaces will go away and I think we should not tie module extensions to it.
I'm taking second round at it and I'll try to understand what I've missed.

[mbland]

Could you clarify what problems this could cause? Definitely you have deeper knowledge that I do at the moment and I might be missing something. My thinking was that we should expose unified interface for configuring toolchains instead of configuring defaults via module extension and for advanced uses resort to macros in build file. Workspaces will go away and I think we should not tie module extensions to it.
I'm taking second round at it and I'll try to understand what I've missed.

Sure. Presenting a more unified interface makes sense, but I feel like releasing the current interface sooner than later provides lower risk and higher value.

Toolchain macros aren't evaluated by either WORKSPACE or Bzlmod; they're always evaluated in BUILD files. The scala_toolchains() call evaluated under both WORKSPACE and Bzlmod is a wrapper around the _scala_toolchains_repo() repository rule. That repo rule emits @rules_scala_toolchains, whose BUILD files call the toolchain macros. So even when WORKSPACE goes away, all these mechanisms will remain and continue to work.

Since WORKSPACE going away won't break toolchain macro calls, replacing them with an extension would delay the next release and add friction to adopting Bzlmod. Then there's potential complications that will likely emerge when actually trying to implement the extension. If this Bzlmod journey has revealed nothing else, it's that complications lurk around almost every corner.

I'd prefer to release what's working now to help rules_scala users migrate to Bzlmod as soon as possible. Then we can try to experiment with updating the module extension to replace direct toolchain macro access. If it works, it can be part of a point release. Then public access to the workspace macros can go away in the next major release after that. That's my preference, but I don't mind taking the time to do things right, so I'm happy to try it first if you insist.

[mbland]

Hi, @mbland, would it be possible to somehow reuse protobuf.patch? As I understand it's the same in each module.

Done, replacing them all with symlinks in 3e377e4.

As I mention in that commit message, I was afraid that this might break the Windows build. However, I should've done my research before, as it actually works fine with Windows set up the right way (and the Bazel CI is, of course, set up to allow them). And for that matter, it seems the Windows build doesn't run any tests that depend on nested modules anyway.

[mbland]

@simuons I updated the tests (involving a bit of Bash and Windows fun), removed the empty WORKSPACE.bzlmod files, and replaced scala_deps.toolchains with individual tag classes. Everything's passing in CI, and it's ready for another look.

[simuons]

@mbland looks awesome to me. Thanks for this. I'm also grateful for all the improvements you did in testing infra. I'll keep it hanging for couple more days in case someone would be willing to take a look as well and then I'll merge it. Thank you!