search

com-lihaoyi / mill

Mill is a fast JVM build tool that supports Java and Scala. 2-4x faster than Gradle and 4-10x faster than Maven for common workflows, Mill aims to make your project’s build process performant, maintainable, and flexible
https://mill-build.org/
MIT License
2.19k stars 347 forks source link

Peculiar cache behavior for external modules #2419

Closed lefou closed 1 year ago

lefou commented 1 year ago

In the process of fixing regressions in the BSP module (after the remove-ammonite) I noticed some strange behavior which is probably caused by some stale caches.

When I run a newer Mill version, the bspWorkerLibs target of the external mill.bsp.BSP module uses a stale cache value. I think, previously, it was re-computed. The default value of that target is computed inside of that target. As the external module is part of the classpath, a change was tracked by the classpath sig and resulted in an invalidation (I think). It looks like somehow we no longer track a change in the classpath.

lihaoyi commented 1 year ago

Seems like it could be related to https://github.com/com-lihaoyi/mill/blob/df86a0757b40ef1ad454e9fcea1f62c739213f58/runner/src/mill/runner/MillBuildBootstrap.scala#L132-L140

It seems to me that that logic relies on at least one meta-build to be present in mill-build/build.sc, in order to propagate the Mill executable classpath to the user-level evaluator in order for it to invalidate stuff. The Mill executable classpath is added to the unmanagedClasspath of all levels of meta-builds, so >1 meta build would work fine. However, with 0 meta builds, the getOrElse(0) would return 0, ignoring the Mill executable classpath entirely.

Maybe instead of getOrElse(0), it should be getOrElse(millBootClasspath.map(PathRef(_).sig) or similar?

lefou commented 1 year ago

Yeah, that seem to fix the issue for me. Any idea, how to test this?

I opened PR #2421 to address it.

lihaoyi commented 1 year ago

Not sure about testing TBH. This misbehavior only happens when the Mill version changes, and we don't really have any precedence for using different Mill versions in our tests. All our tests use Mill build from HEAD

lefou commented 1 year ago

It's probably ok to merge the linked PR as is, as it appears to fix my concrete case.

Maybe, we can come up with some shell or Scala scripting doing the necessary version bumps and building two different mill versions, in the spirit of our bootstrap tests.

NB: We also have issues with handling a Mill version change in a running server, which we could approach this time with some tests. I fixed most of them manually in the past, but it was a time consuming process, and it partially regressed when we switched our rpc library to junixsocket.