Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make runtime deps transitive #3774

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

lihaoyi
Copy link
Member

@lihaoyi lihaoyi commented Oct 20, 2024

Fixes #3761

  • Introduce runModuleDeps, the runtime version of moduleDeps and compileModuleDeps

  • We introduce transitiveRunIvyDeps, the runtime version of transitiveIvyDeps, and use it in most places as a replacement for runIvyDeps

  • I also flipped the ordering of transitiveModuleDeps/transitiveRunModuleDeps to put this on the right/last, to follow the rest of the module resolution logic where "upstream" things come on the left/first in the classpath ordering

There are some behavioral changes, e.g. the A module's direct compileModuleDeps no longer end up on your runClasspath, as shown by the change in the test case mill.scalalib.ScalaMultiModuleClasspathsTests.modCompile. I think the new behavior is more correct than the old one?

One (benign?) consequence of this change is that the contents of the various *run* tasks now have considerable overlap with the non-run tasks, e.g. transitiveRunIvyDeps overlaps with transitiveIvyDeps.

The way transitive runtime dependencies are now managed, both "run{Module,Ivy}Dep of {module,ivy}Dep" and "{module,ivy}Dep of run{Module,Ivy}Dep" are both aggregated into the run classpath. I'm not sure if this matches what Maven does (it does according to chatgpt!) but it seems reasonable to me

Added some additional unit tests to cover the new transitive behavior. The whole dependency-wiring stuff is pretty gnarly but hopefully the existing test suite will stop us from breaking too much (especially mill.scalalib.ScalaMultiModuleClasspathsTests which is pretty rigorous and we add to).

This is a binary-compatible but semantically incompatible change, would be good to get it into 0.12.0

Best reviewed with Hide Whitespace enabled

@lihaoyi lihaoyi requested review from lefou and lolgab October 20, 2024 01:44
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.

runIvyDeps should be transitive for consistency with Maven resolution
1 participant