BOM-related PRs

[alexarchambault]

Opening this discussion to give an overview of my currently opened 4 successive PRs for BOM support.

These are:

• Deprecate resolveFilter stuff #4162
• Overhaul transitive module handling in dependency resolution #4145
• Tweak BOM support to accomodate scopes better #4154
• Allow users to define and publish their own BOM in Mill #4155

Only the last two actually add BOM-related features.

The first two consist in refactorings / clean-ups that help for the other two.

First PR

The first PR should be the less ~controversial. It changes the way workers class path is obtained, so that it doesn't rely on manual filtering of dependencies, and can be fetched just like any other dependency. Changes in the second PR require it.

Second PR

The second one is the one with the broadest changes. It changes the way we pass dependencies (and BOMs) to coursier. Instead of aggregating dependencies from all module dependencies, and passing those to coursier, we create a fake (in-memory) coursier repository for the Mill modules. Dependencies don't need to be aggregated anymore by Mill: these are simply put in the project definitions passed to the fake repository. sbt relies on the same trick. More important than dependencies, Mill also doesn't need to attribute BOMs and dependency management to the right dependencies and modules itself: these are just put as-is in the project definitions, and coursier handles the rest.

A consequence of that second PR is the deprecation of transitiveIvyDeps: it's not used anymore to get dependencies. Instead, one passes the coordinates of the Mill module we're interested in, and make sure the fake coursier repo knows about the right Mill modules. coursier then just gives the dependencies of that module (which includes those pulled by its module dependencies).

transitiveIvyDeps (and related tasks) was used in many places, so the PR touches many things in the codebase.

One thing to remember after this PR, is the way Mill interfaces with coursier: each module has a coursier project (JavaModule#coursierProject), giving details about its dependencies and BOMs / dep management. Each module is identified by Maven coordinates, that one can get via JavaModule#coursierDependency.

Third PR

The PR adds non-regression tests for BOM use in compileIvyDeps and runIvyDeps, and makes sure BOMs of a module can be used in its test modules.

Fourth PR

This PR adds mill.scalalib.BomModule, that one can use to define and publish a BOM (a module with versions in depManagement, that doesn't have sources, and gets published solely as a POM file).

[alexarchambault]

About the way to review them, if you agree with the general direction (change of the way Mill and coursier interface in the second PR, addition of BomModule in the 4th PR), I think these can be reviewed one after the other, waiting for the first to be merged, then reviewing the second one, etc. This should make the diffs of each PR easier to comprehend.

[lefou]

I do see how this can improve the overall experience of an consistence dependency resolution. Especially since it is and always was very tricky to work with all the transitiveXXX tasks. So I agree, that offloading the details of dependency resolution to coursier is the best approach.

I only reviewed the first PR and didn't look into the details of the follow up PRs, but I'd rather try to keep the full coursier API an implementation detail. This means, instead of exposing the full coursier project as an task, we should consider a worker with a dedicated interface to provide just the specific API for our explicit needs. This should give us a bit more freedom in the API evolution.

[lihaoyi]

I think let's get these BOM changes in without the coursier API abstraction, and leave that for 0.13.0. I think we agree on the high level direction so just need to get the PRs reviewed/passing/merged

[alexarchambault]

Abstracting the coursier API would allow to evolve the coursier API (in a binary breaking fashion, coursier/coursier#3229) while not breaking bin compat in Mill. So that could be helpful on the coursier side too.

Some coursier API definitions are quite large though, I fear this would be be gruesome work :/ (but chatgpt might help 😅)

[lihaoyi]

Looks like all the PRs linked in this discussion are merged. @alexarchambault do you have anything else you'd like to merge before I cut 0.12.6, or are you done with the BOM stuff for now?

[alexarchambault]

I think I'm done, apart from reviewing BOM documentation just in case