--import does not work when there is a meta-build

[lefou]

It looks like the imported dependency is only available in the meta-build (level 1), but not the main build (level 0).

> mill --import ivy:de.tototec::de.tobiasroeser.mill.palantir.javaformat::0.0.0-3-712191 --meta-level 1 de.tobiasroeser.mill.palantir.javaformat.PalantirJavaFormat/checkFormat __.sources
[build.sc] [9/9] de.tobiasroeser.mill.palantir.javaformat.PalantirJavaFormat.checkFormat 

> mill --import ivy:de.tototec::de.tobiasroeser.mill.palantir.javaformat::0.0.0-3-712191 --meta-level 0 de.tobiasroeser.mill.palantir.javaformat.PalantirJavaFormat/checkFormat __.sources
[build.sc] [48/54] zincReportCachedProblems 
Cannot resolve external module de.tobiasroeser.mill.palantir.javaformat.PalantirJavaFormat

[lefou]

We could take this issue as an opportunity to define how it should behave. Since --import only provides runtime dependencies, compilation is not affected. So we are rather free where to apply.

Currently, it only applies to the runtime of the deepest explicitly defined root module. This is the main build when no meta-build is enabled or the deepest meta-build.

What we clearly need is the application to the main build. But application to a meta-build is also useful. So, do we simply want it to apply to the runtime of all meta-levels or just to the selected one, which is the level selected with --meta-level?

[lefou]

@lihaoyi Could you please help out? I'm sure this must be a easy change, but I wasn't able to spot the right place where to forward the runtime classpath from the meta-build to the main build.

[lihaoyi]

@lefou it looks like we are already propagating the list of cliImports through the various levels of meta-build by means of the MillBuildRootModule.Info data structure:

mill/runner/src/mill/runner/MillBuildRootModule.scala

Line 355 in 2f1fee8

| _root_.scala.Seq(${cliImports.map(literalize(_)).mkString(", ")})

This is meant to then get used as part of the MillBuildRootModule via

mill/runner/src/mill/runner/MillBuildRootModule.scala

Line 86 in 2f1fee8

def cliImports = T.input { millBuildRootModuleInfo.cliImports }

That then gets included in runIvyDeps below.

I'm not actually that familiar with what CLI imports are used for, but it seems to me like it's being propagated. Can you check the show cliImports and show runClasspath for the various meta levels to see whether they're all correctly populated with the thing that you --imported?

[lefou]

I checked. In case of a project with a meta-build, the cliImport is not properly forwarded:

> mill --import ivy:de.tototec::de.tobiasroeser.mill.palantir.javaformat::0.0.0-3-712191 --meta-level 2 show cliImports
[mill-build/build.sc] [1/1] show > [1/1] cliImports 
[
  "de.tototec::de.tobiasroeser.mill.palantir.javaformat::0.0.0-3-712191"
]

> mill --import ivy:de.tototec::de.tobiasroeser.mill.palantir.javaformat::0.0.0-3-712191 --meta-level 1 show cliImports
[build.sc] [1/1] show > [1/1] cliImports 
[]

Same for runClasspath. The imported dependency is only in the built-in bootstrap module (level 2):

> mill --import ivy:de.tototec::de.tobiasroeser.mill.palantir.javaformat::0.0.0-3-712191 --meta-level 2 show runClasspath | grep javaformat
  "qref:v1:22804626:/home/lefou/.ivy2/local/de.tototec/de.tobiasroeser.mill.palantir.javaformat_mill0.11_2.13/0.0.0-3-712191/jars/de.tobiasroeser.mill.palantir.javaformat_mill0.11_2.13.jar",

> mill --import ivy:de.tototec::de.tobiasroeser.mill.palantir.javaformat::0.0.0-3-712191 --meta-level 1 show runClasspath | grep javaformat

[lihaoyi]

Can you try one more thing just for debugging, does -i make a difference?

[lefou]

Nope, already checked -i too.

[lihaoyi]

Also, what does this generate?

mill --import ivy:de.tototec::de.tobiasroeser.mill.palantir.javaformat::0.0.0-3-712191 --meta-level 2 show generateScriptSources

It's meant to take the cliImports (which look present at meta-level 2 according to your first line) and generate scripts for meta-level 1 that will include them as the fourth argument to MillBuildRootModule.Info(...)

[lihaoyi]

I see over here https://github.com/com-lihaoyi/mill/blob/main/runner/src/mill/runner/MillBuildRootModule.scala#L120 we're passing millBuildRootModuleInfo.cliImports rather than referencing the cliImports() which is wrapped in a T.input. I wonder if that's the problem, as if cliImports changes from run to run this generateScriptSources target might not be properly invalidated

[lihaoyi]

You can test this without making Mill code changes by rm -rfing your out/ folder before your call to mill --import ivy:de.tototec::de.tobiasroeser.mill.palantir.javaformat::0.0.0-3-712191 --meta-level 1 show cliImports to see if it populates correctly

[lefou]

That's it. I removed the out/ and added the --import option in the first run, and I see it in the cliImports.

[lefou]

And it persists, when I remove the --import option.

[lihaoyi]

great, yeah just cache invalidation issue then. Replacing millBuildRootModuleInfo.cliImports with cliImports() should force it to invalidate correctly

[lefou]

Thanks for looking at it. I'll fix it.

[lefou]

Would be cool if we could avoid recompilation due to a changed --import option though.

[lefou]

Would be cool if we could avoid recompilation due to a changed --import option though.

I opened #2978 to fix this.

[lihaoyi]

Would be cool if we could avoid recompilation due to a changed --import option though.

This is probably possible, but we may need to do some hackery. There are basically two approaches:

1. Easy but hacky: put cliImports in a global/threadlocal, have def cliImports read it

2. Proper but hard: the Mill build.sc files that we compile and to instantiate the MillBuildRootModule instances are just static objects, and do not have any affordances for dependency injection. To fix that:

   • We would need to change the code wrapper to define a class instead of an object,
   • And then in MillBuildBootstrap.scala instead of using reflection to fish out the static $MODULE field we would instead use reflection to new up an instance of the class and inject cliImports into it, which it can in turn read use inside def cliImports = T.input{...}
   • There may be some additional work e.g. to update Reflect.scala, Resolve.scala, or the CodeSig analyzer, as those parts of the code are basically pattern-matching on the JVM classes & methods generated by the Scala compiler, and may or may not be robust against changing the root module from a static object to a class

[lefou]

I chose option 1 in PR #2978, which isn't IMHO that hacky. It solves some convenience issues, (no recompilation, adding external modules without editng build.sc) without sacrifying the compiled outcome.

We could also try to tighten the resolution of the provided dependencies with coursier. E.g. provide some version intervals to detect incompatibilities (see also coursier/coursier#2891 (comment)), or exclude transitive deps we already provide.