Make build.sc dependency resolution independent of Mill assembly
Should fix https://github.com/com-lihaoyi/mill/issues/2985 and https://github.com/com-lihaoyi/mill/issues/2703, at least as a first pass.
- I did not change the actual classpath contents to try and minimize it; that can come in a follow up PR
- I did not try to cache or bundle the dependencies locally; that can also come in a follow up
At least with this change, all the Mill build.sc dependencies go through the same code path (Coursier resolution) and behave similarly (e.g. you should be able to update upickle via import $ivy. Further improvements to hygiene or performance can come in follow ups (e.g. resolving dependencies bundled in the assembly)
Notes:
-
Changed
MillBuildRootModule#ivyDepstomandatoryIvyDepsto prevent accidental overriding (not sure why this didn't cause problems before??? Maybe people just didn't overrideivyDepsmuch in the meta-build) -
Removed
resolveDepsExclusions, since now we are no longer hard-coding a dependency on the Mill assembly, we also no longer need to treat these dependencies specially and they can be upgraded or evicted as the user sees fit -
Removed
Lib.resolveMillBuildDepsand its use sites, since now the build dependencies go through the same code path as any otherivyDeps
Tested the mill.idea.GenIdea/idea workflow manually in example/basic/1-simple-scala and example/misc/4-mill-build-folder manually, the intellij jump-to-definition in build.sc and mill-build/build.sc seem to work
Added a test, which fails, demonstrating that the current code changes are not enough
The issue as of now is that even without the mill jar added to the classpath as part of MillBuildRootModule, it still ends up on the classpath as part of the classloader hierarchy when we spawn sub-classloaders to evaluate each build.sc file.
Removing that connection to make each spawned classloader independent of the parent will require some more work, because as of now the sub-classloaders and parent classloader share a the RootModule/Module/Task interfaces along with everything they depend on (os.Path, mill.define.Discover, upickle, etc.). This will need to be narrowed down significantly to make it possible to make the spawned classloaders independent, and thus the build.sc classpath independent of the Mill binary
Maybe, we can implement this specific test as an server integration test, which should already run in a spawned, thus isolated, Java process?
In this case its not the added test that is a problem, but the festure itself. It seems like it would be a bunch more work to actually isolate the build.sc classpath
Even if we don't support bumping of "provided" dependencies, this PR is still the right change, since it makes dependency resolution more consistent and allows IDEs to easily pull sources.
@lihaoyi Why did you mark this PR with the compat-breaker label? Shouldn't this change only be implementation details and transparent to the user?
The issue with using this PR as is is that because the dependencies may have different versions, your IDE may pull the wrong sources (you'd get the resolved version sources rsther than the bundled version sources)
As I imagined it, if we fully supported resolving Mill's own dependencies via coursier rather than using the bundled versions, people import $ivy stuff may end up with arbitrarily different versions of stuff on the classpath vs what they have now depending on how dependency resolution goes
All classloaders I know repect the order of the entries, so we could try to put our "provided" dependencies first on the classpath. Another option would be to use version ranges or forced versions like [0.10.2] in the ivyDeps. That way, we would at least detect (and fail) when the versions have a conflict. Unfortunately, the old coursier API we're using doesn't properly report mismatches, and my attempts to replace it with the newer API have other issues (see https://github.com/com-lihaoyi/mill/pull/3185).
The issue here is that the build.sc classloader has to share classes with the Mill binary classloader, because they interact via Scala class instances. They have to be using the same classes
So although setting parent = null to fully isolate them is trivial, we would then also need to completely rewrite the interactions to be through thin interfaces, rather than the fat Module/RootModule/Task classes we have today.
It's possible, but would take more work, and is not as simple as just changing the classpath on the MillBuildRootModule
Forcing the ivy resolution to match the provided versions could help land this without the more aggressive changes, just as a simplification without actually allowing those deps to be upgraded. Does coursier allow such a thing?
If you use forced versions (Maven term, I think, like [0.11.8]) or intervals ([0.11.8,0.11.8]) to pin a version, coursier will fail, if transitive selected versions don't match that range.
Documentation: https://get-coursier.io/docs/other-version-selection
Example: https://github.com/com-lihaoyi/mill/issues/3075#issuecomment-1998525943
But it does not work in our current implemntation, due to Mill using the "low level" coursier API.
In PR https://github.com/com-lihaoyi/mill/pull/3185 I therefore replaced that API with the "high level" API, but I struggle to find a proper way to forward mapped dependencies to that API.