Skip to content

Conversation

jadenPete
Copy link
Contributor

@jadenPete jadenPete commented Oct 6, 2025

Description

This PR refactors the ruleset to use rules_jvm_external for fetching all Maven artifacts. It deletes all of the rules in third_party/repositories and scala/scala_maven_import_external.bzl.

Please read the "Maven Dependencies" section in our Unifying the Scala Rulesets proposal for more information:
https://docs.google.com/document/d/12BmUPpJpworA9ep2P_J00TxXwFRBZDQttUCsoYo0Yao/edit?tab=t.0#heading=h.h50z2i1vo4z6

Motivation

Advantages of using rules_jvm_external over using our own rules include:

  • Reduced maintenance burden: We benefit from contributions to rules_jvm_external, and it's easier for newcomers to contribute to this ruleset as they're more likely to have experience with rules_jvm_external than the existing rules
  • Artifact pinning: Downloaded artifacts, their checksums, and their versions are “pinned” into a file checked into the repository
    • Currently, we do something similar with the files in third_party/repositories, but these files aren't generated in the sense that deleting them and re-creating them produces an identical file. There's also no testing in place to ensure they aren't arbitrarily modified.
  • Unified repository: Having all external artifacts under a single repository (rules_scala_maven with these changes) imposes numerous benefits:
    • It enforces that we're using pulling a single version of each artifact. This is helpful for preventing classpath conflicts and reducing the number of artifacts downloaded, among other things.
    • It minimizes the number of use_repo calls needed
    • It minimizes the likelihood of conflicting repository names
    • It's more queryable

Breaking Changes

There aren't many breaking changes, as I did my best to preserve the existing interfaces as much as possible. The only breaking changes are:

  • The overridden_artifact tag class from scala_deps has been removed
    • This change was explained in the second-to-last commit
  • The settings tag class from scala_deps has been removed
    • fetch_sources: Sources are now always fetched
    • maven_sources: Setting the Maven servers without providing your own artifacts is now impossible
    • validate_scala_version: Scala version validation is no longer needed

Unresolved Questions

WORKSPACE Support

Since we now declare all of the artifacts we need in MODULE.bazel, I haven't found a way to avoid duplicating that list in both MODULE.bazel and WORKSPACE. include exists, but:

  • It's only allowed in top-level repositories (which rules_scala wouldn't be when used by another repository)
  • It can't be used like load, in that the variables defined in an included file are isolated to that file

The two solutions I can think of are:

  1. Duplicate the list and add testing for WORKSPACE support
  2. Drop WORKSPACE support
    • @mbland has indicated that this may not be an option

Option 1 may be less painful if we can slim down the number of artifacts fetched. In this PR, I've copied toolchains' dependencies verbatim, but I realize that many of the artifacts we fetch are transitive dependencies not used directly. For example, the Scala compilation toolchain defines JLine as a dependency, despite us never using JLine. As a dependency of the compiler, it would've been included on the compiler classpath anyway.

I'm curious about others' thoughts on this.

Jaden Peterson added 13 commits September 30, 2025 16:05
It's my goal to remove `dev_deps`, and switching to
`buildifier-prebuilt` gets us one step closer to doing that. It should
also make CI faster.
We need `--incompatible_use_plus_in_repo_names` to be set in wokrspaces
using this repository, since we construct the canonical repository name
for `rules_scala_maven` in a few places.
`overridden_artifacts` is no longer possible with the new
`rules_jvm_external` setup and has been removed. We could fetch
artifacts for the non-scala toolchains as we do with the scala
toolchain, using `maven_install`, which would allow for overriding
artifacts, but I don't think we should do that for a couple reasons:

- That wouldn't allow us to share external dependencies across
  repositories or utilize lockfiles
- We already support providing custom dependencies via
  `scala_compiler_classpath`, etc. Arguably, `overridden_artifacts` is a
  redundant feature
@shs96c
Copy link

shs96c commented Oct 7, 2025

For reference:

maven_sources: Setting the Maven servers without providing your own artifacts is now impossible

In the root project, people can define an install tag with name = "rules_scala_maven", and they can then set the repositories to whichever values they like there. The install tag will require its own lock file, but then it'll be used in preference to the one in this repo. Alternatively, people can use the --downloader_config flag in bazel to rewrite URLs.

@mbland mbland self-assigned this Oct 7, 2025
@mbland
Copy link
Collaborator

mbland commented Oct 7, 2025

This may take some time to digest. 🙂 I'm also focusing on polishing my Bzlmod Migration Bootcamp for BazelCon, so that'll slow me down a bit. But I'm definitely interested in exploring this, and hearing what others think.

If this lands, since it does contain breaking API changes (which do sound like good changes to me), it will precipitate a rules_scala 8.0.0 release, which I think I'd prefer to have happen after the Bazel 9 release. But it's good to get some review and input on such a significant change early, so I think it's awesome that @jadenPete has put this out there for us to work with. (And @jadenPete, I appreciate you asking me first in a Bazel Slack DM, given the significance of this change.)

I've yet to look at this at all, but a few early thoughts/assumptions:

  • If there's a way to break this down into smaller, separate pull requests, we should. That will make it easier to review and to make progress with minimal disruption. At a minimum I wonder whether we could introduce the r_j_e support without ripping out the existing Maven repository engine in one request, then do the ripping out in another.

  • I'd love to drop legacy WORKSPACE support, but I'm not sure how soon that will be feasible. The changes for rules_scala 7.x aim to build a bridge between the legacy WORKSPACE and Bzlmod worlds, so maybe it would be feasible with 8.0.0. But I don't want to drop it until the benefits-to-drawbacks ratio is sufficiently compelling. (Maybe we could poll rules_scala users on whether legacy WORKSPACE support remains critical to them?)

  • I did wonder, once I got into the depths of the Bzlmodification work, why rules_scala maintained its own Maven artifact management system, when rules_jvm_external appears to be the standard. Maybe this is a question for @simuons: Was this because r_j_e didn't exist or wasn't sufficiently stable at the time? Or are there other reasons for it, and are there reasons to keep it instead of switching to r_j_e entirely?

Also interested in any input or insight that @WojciechMazur may have.

cc: @BillyAutrey

@jadenPete
Copy link
Contributor Author

For reference:

maven_sources: Setting the Maven servers without providing your own artifacts is now impossible

In the root project, people can define an install tag with name = "rules_scala_maven", and they can then set the repositories to whichever values they like there. The install tag will require its own lock file, but then it'll be used in preference to the one in this repo. Alternatively, people can use the --downloader_config flag in bazel to rewrite URLs.

Good to know! Regarding the install tag, are you suggesting that users have the option to define their own repository, which they could then pass to rules_scala? That sounds interesting, but, if so, I have a couple questions:

  • Would it be possible for us to pass the list of artifacts to them, or is MODULE.bazel syntax not expressive enough?
  • Given that users already have the option to provide their own dependencies, would this be a redundant feature?

--download_config sounds like a plausible workaround. @mbland, do you think we should call this out in the documentation for those users wanting to change the Maven repository URLs?

@jadenPete
Copy link
Contributor Author

If there's a way to break this down into smaller, separate pull requests, we should. That will make it easier to review and to make progress with minimal disruption. At a minimum I wonder whether we could introduce the r_j_e support without ripping out the existing Maven repository engine in one request, then do the ripping out in another.

I can definitely do that. This PR is already divided into commits that migrate each toolchain incrementally, so I can add the initial rules_jvm_external dependency and migrate a toolchain in one PR, then open another PR for each toolchain. Even if go down that route, I wonder if it'd be best to keep this PR open for a week or two to let it simmer, so we can discuss the changes as a whole. That way, if there's anything fundamental I should change about my approach, we can discuss that before it's fragmented across multiple PRs.

I'd love to drop legacy WORKSPACE support, but I'm not sure how soon that will be feasible. The changes for rules_scala 7.x aim to build a bridge between the legacy WORKSPACE and Bzlmod worlds, so maybe it would be feasible with 8.0.0. But I don't want to drop it until the benefits-to-drawbacks ratio is sufficiently compelling. (Maybe we could poll rules_scala users on whether legacy WORKSPACE support remains critical to them?)

Understood. I'll plan on preserving WORKSPACE support in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants