Skip to content

Commit

Permalink
Re-introduce Mill special handling of ivydeps, overhaul test classpat…
Browse files Browse the repository at this point in the history
…h management to allow testing (#2476)

Fixes #2474

These were overlooked in #2377
and deleted, and were not covered by any tests. But it's straightforward
to put them back.

# Major Changes

1. Put back `MillIvy.scala` and call it in `MillBuildRootModule.scala`

# Testing 

1. I added an example test `example/misc/6-contrib-import` to both
exercise the code as well as serve as an example we can include in our
docs

2. In order to allow usage of contrib module in `.local` integration and
example tests, I moved the handling of Mill test classpath overrides
from `Util.millProjectModule` to `CoursierSupport#resolveDependencies`.
This lets us be more override contrib modules dependency resolution,
even though they don't have a neat single location for us to call our
`millProjectModule` helper.

3. I refactored `millProjectModule` to not need a `key`, so we just
compute the key based on the dependency name, keeping them consistent
and removing an unnecessary degree of freedom

4. The local-testing-classpath-overrides were moved from using JVM
system properties to instead use classpath resources: we look for
overrides in `mill/local-test-overrides/*`. This should remove any
security worries:
1. Before you only needed to modify the JVM props or `JAVA_OPTS`
environment variable, and could replace the code of a Mill module to any
local filesystem path
2. Now, you need access to modify the Mill classpath to trigger the test
overrides, and at that point you already have access to modifying the
Mill classfiles being executed anyway

While it probably was not strictly necessary to clean up the test
classpath overrides logic as part of this PR, the status quo in master
was a pile of hacks, and I didn't feel like adding another hack to get
`.local` testability of contrib libraries working. With these changes,
contrib libraries work the same as the existing Mill test classpath
overrides, and things are cleaned up so much that the net lines of code
for this PR is negative

There's still more cleanup to do in `build.sc`, but those can come in
follow up PRs
  • Loading branch information
lihaoyi authored Apr 29, 2023
1 parent fed9986 commit fff545a
Show file tree
Hide file tree
Showing 19 changed files with 271 additions and 282 deletions.
7 changes: 2 additions & 5 deletions bsp/src/mill/bsp/BSP.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import mill.{Agg, T, BuildInfo => MillBuildInfo}
import mill.define.{Command, Discover, ExternalModule, Task}
import mill.eval.Evaluator
import mill.main.{BspServerHandle, BspServerResult, BspServerStarter}
import mill.modules.Util.millProjectModule
import mill.scalalib.{CoursierModule, Dep}
import mill.util.PrintLogger
import os.Path
Expand All @@ -20,11 +21,7 @@ object BSP extends ExternalModule with CoursierModule with BspServerStarter {
private[this] val millServerHandle = Promise[BspServerHandle]()

private def bspWorkerLibs: T[Agg[PathRef]] = T {
mill.modules.Util.millProjectModule(
"MILL_BSP_WORKER",
"mill-bsp-worker",
repositoriesTask()
)
millProjectModule("mill-bsp-worker", repositoriesTask())
}

/**
Expand Down
Loading

0 comments on commit fff545a

Please # to comment.