Make --import flag work again

[lihaoyi]

Fixes #2485

We take the config.imports field and pass it to MillIvy.processMillIvyDepSignature inside ivyDeps. This was overlooked in #2377, but seems easy enough to put back

Honestly not super sure how to test this. But when I run

./mill -i dev.run example/tasks/3-anonymous-tasks -i --import ivy:com.lihaoyi::mill-contrib-bloop:0.11.0-M8  mill.contrib.bloop.Bloop/install

On main branch, I get

Cannot resolve external module mill.contrib.bloop.Bloop

On this PR I get

java.lang.NoSuchMethodError: 'java.lang.ThreadLocal mill.eval.Evaluator$.currentEvaluator()'
    mill.contrib.bloop.Bloop$$anonfun$$lessinit$greater$1.apply(Bloop.scala:8)
    mill.contrib.bloop.Bloop$$anonfun$$lessinit$greater$1.apply(Bloop.scala:8)

Seems like there's a binary incompatibility, but at least the --import flag took effect?

[lefou]

I think after a mill __.publishLocal you can retry the manual test with the just published bloop module, which should be binary compatible then.

[lihaoyi]

@lefou ok, ./mill -i dev.run example/tasks/3-anonymous-tasks -i --import ivy:com.lihaoyi::mill-contrib-bloop:0.11.0-M8-19-f7fe30 mill.contrib.bloop.Bloop/install (echo $? prints 0) passes after a __.publishLocal. I think that means this works then?

[lefou]

@lefou ok, ./mill -i dev.run example/tasks/3-anonymous-tasks -i --import ivy:com.lihaoyi::mill-contrib-bloop:0.11.0-M8-19-f7fe30 mill.contrib.bloop.Bloop/install (echo $? prints 0) passes after a __.publishLocal. I think that means this works then?

If you now see a .bloop/ directory with config files for each module, than yes.

[lihaoyi]

Yep, seems to be there

lihaoyi mill$ find example/scalabuilds/10-scala-realistic/.bloop
example/scalabuilds/10-scala-realistic/.bloop
example/scalabuilds/10-scala-realistic/.bloop/foo[2.13.8].test.json
example/scalabuilds/10-scala-realistic/.bloop/foo[3.2.2].test.json
example/scalabuilds/10-scala-realistic/.bloop/bar[2.13.8].test.json
example/scalabuilds/10-scala-realistic/.bloop/foo[2.13.8].json
example/scalabuilds/10-scala-realistic/.bloop/bar[3.2.2].json
example/scalabuilds/10-scala-realistic/.bloop/bar[2.13.8].json
example/scalabuilds/10-scala-realistic/.bloop/qux.json
example/scalabuilds/10-scala-realistic/.bloop/foo[3.2.2].json
example/scalabuilds/10-scala-realistic/.bloop/bar[3.2.2].test.json

[lefou]

Something is not right. It's probably caching too much.

I locally published as 0.11.0-M8-20-1e6ba7 but then tested with your example (and used the wrong version 0.11.0-M8-19-f7fe30). After correcting my error, it still tries to fetch the wrong version.

$ mill -i dev.run example/tasks/3-anonymous-tasks -i --import ivy:com.lihaoyi::mill-contrib-bloop:0.11.0-M8-19-f7fe30 mill.contrib.bloop.Bloop/install
[3/1047] de.tobiasroeser.mill.vcs.version.VcsVersion.vcsState 
[1047/1047] dev.run 
[build.sc] [28/48] resolvedIvyDeps | Downloading [2/2] artifacts (~0/0 bytes)
1 targets failed
resolvedIvyDeps 
Resolution failed for 1 modules:
--------------------------------------------
  com.lihaoyi:mill-contrib-bloop_2.13:0.11.0-M8-19-f7fe30 
        not found: /home/lefou/.ivy2/local/com.lihaoyi/mill-contrib-bloop_2.13/0.11.0-M8-19-f7fe30/ivys/ivy.xml
        not found: https://repo1.maven.org/maven2/com/lihaoyi/mill-contrib-bloop_2.13/0.11.0-M8-19-f7fe30/mill-contrib-bloop_2.13-0.11.0-M8-19-f7fe30.pom

--------------------------------------------

For additional information on library dependencies, see the docs at
https://com-lihaoyi.github.io/mill/mill/Library_Dependencies.html

$ mill -i dev.run example/tasks/3-anonymous-tasks -i --import ivy:com.lihaoyi::mill-contrib-bloop:0.11.0-M8-20-1e6ba7 mill.contrib.bloop.Bloop/install
[3/1047] de.tobiasroeser.mill.vcs.version.VcsVersion.vcsState 
[1047/1047] dev.run 
[build.sc] [28/48] resolvedIvyDeps | Downloading [2/2] artifacts (~0/0 bytes)
1 targets failed
resolvedIvyDeps 
Resolution failed for 1 modules:
--------------------------------------------
  com.lihaoyi:mill-contrib-bloop_2.13:0.11.0-M8-19-f7fe30 
        not found: /home/lefou/.ivy2/local/com.lihaoyi/mill-contrib-bloop_2.13/0.11.0-M8-19-f7fe30/ivys/ivy.xml
        not found: https://repo1.maven.org/maven2/com/lihaoyi/mill-contrib-bloop_2.13/0.11.0-M8-19-f7fe30/mill-contrib-bloop_2.13-0.11.0-M8-19-f7fe30.pom

--------------------------------------------

For additional information on library dependencies, see the docs at
https://com-lihaoyi.github.io/mill/mill/Library_Dependencies.html

$ mill -i dev.run example/tasks/3-anonymous-tasks -i mill.contrib.bloop.Bloop/install
[3/1047] de.tobiasroeser.mill.vcs.version.VcsVersion.vcsState 
[1047/1047] dev.run 
[build.sc] [28/48] resolvedIvyDeps | Downloading [2/2] artifacts (~0/0 bytes)
1 targets failed
resolvedIvyDeps 
Resolution failed for 1 modules:
--------------------------------------------
  com.lihaoyi:mill-contrib-bloop_2.13:0.11.0-M8-19-f7fe30 
        not found: /home/lefou/.ivy2/local/com.lihaoyi/mill-contrib-bloop_2.13/0.11.0-M8-19-f7fe30/ivys/ivy.xml
        not found: https://repo1.maven.org/maven2/com/lihaoyi/mill-contrib-bloop_2.13/0.11.0-M8-19-f7fe30/mill-contrib-bloop_2.13-0.11.0-M8-19-f7fe30.pom

--------------------------------------------

For additional information on library dependencies, see the docs at
https://com-lihaoyi.github.io/mill/mill/Library_Dependencies.html

$ mill -i dev.run example/tasks/3-anonymous-tasks clean
[3/1047] de.tobiasroeser.mill.vcs.version.VcsVersion.vcsState 
[1047/1047] dev.run 
[build.sc] [28/48] resolvedIvyDeps | Downloading [2/2] artifacts (~0/0 bytes)
1 targets failed
resolvedIvyDeps 
Resolution failed for 1 modules:
--------------------------------------------
  com.lihaoyi:mill-contrib-bloop_2.13:0.11.0-M8-19-f7fe30 
        not found: /home/lefou/.ivy2/local/com.lihaoyi/mill-contrib-bloop_2.13/0.11.0-M8-19-f7fe30/ivys/ivy.xml
        not found: https://repo1.maven.org/maven2/com/lihaoyi/mill-contrib-bloop_2.13/0.11.0-M8-19-f7fe30/mill-contrib-bloop_2.13-0.11.0-M8-19-f7fe30.pom

--------------------------------------------

For additional information on library dependencies, see the docs at
https://com-lihaoyi.github.io/mill/mill/Library_Dependencies.html

[lihaoyi]

I suspect the millBuildRootModuleInfo.cliImports needs to go in a T.input, since it can vary between runs. Let me try that

[lefou]

I was thinking, whether we should use the extra cli-import as runClasspath only. This would have the advantage, that we don't need to recompile. And it limits the side-effects of such an imports. IIUC, we don't use external modules anywhere at compile time. When we resolve the target, we search in the runClasspath, so the bloop plugin should keep working as expected. But maybe, I'm missing some details. WDYT?

[lihaoyi]

I'm not sure about the exact use case for CLI imports. If it's just to let us directly evaluate targets on the external modules, then we don't need them at compile time. But as of now, they also let you write code in build.sc that references things in the external module pulled in by --import right? Is that something that we want/need to support?

[lefou]

I'm not sure about the exact use case for CLI imports. If it's just to let us directly evaluate targets on the external modules, then we don't need them at compile time. But as of now, they also let you write code in build.sc that references things in the external module pulled in by --import right? Is that something that we want/need to support?

That latter possibility is rather some side-effect from the initial implementation, which was not as flexible as the current one. This also means, a build.sc compilation can fail or misbehave based on the given --import. I'm convinced, we did not intend this when we introduced it. It's also against the "single source of truth" principle, which Mill follows*. IMHO, we should change to runtime-only.

*) mostly, as we currently accept a mill-build without referencing it from build.sc.

[lihaoyi]

Ok. I'll update this PR

[lihaoyi]

Updated the PR to add config.imports to runIvyDeps rather than ivyDeps, and move usage of config.Imports inside a def cliImports = T.input

The caching model here is a bit unintuitive:

1. We generally cache MillBuildRootModules for performance. By that logic, we would expect the T.input on a constructor param for MillBuildRootModule to never invalidate, since we're using the same MillBuildRootModule which has the same millBuildRootModuleInfo.cliImports value

2. However, the bootstrap MillBuildRootModule.Bootstrap module is never cached. This wasn't super intentional, but it's the way it is, and MillBuildRootModule.Bootstrap is small enough it probably doesn't matter that we re-evaluate it every time (v.s. user defined build.sc files which can grow large and expensive to evaluate).

3. As a result, every time we run something, a new MillBuildRootModule.Bootstrap is created, millBuildRootModuleInfo.cliImports has an opportunity to change, and the T.input has a chance to notice this change

4. Each level of MillBuildRootModule propagates the millBuildRootModuleInfo.cliImports to the next level in the generated sources, and the change in the generated sources (if any) should invalidate the next-level MillBuildRootModule if necessary, causing the module to re-evaluate and letting the T.input pick up the change

[lefou]

Looks good to me. Changing the import does not force a re-compilation of the build script.