Skip to content

Commit

Permalink
Support module.sc files in subfolders (#3213)
Browse files Browse the repository at this point in the history
This PR implements support for per-subfolder `build.sc` files, named
`module.sc`. This allows large builds to be split up into multiple
`build.sc` files each relevant to the sub-folder that they are placed
in, rather than having a multi-thousand-line `build.sc` in the root
configuring Mill for the entire codebase.

## Semantics

1. The `build.sc` in the project root and all nested `module.sc` files
are recursively within the Mill directory tree are discovered (TODO
ignoring files listed in `.millignore`), and compiled together

2. We ignore any subfolders that have their own `build.sc` file,
indicating that they are the root of their own project and not part of
the enclosing folder.

3. Each `foo/bar/qux/build.sc` file is compiled into a
`millbuild.foo.bar.qux` `package object`, with the `build.sc` and
`module.sc` files being compiled into a `millbuild` `package object`
(rather than a plain `object` in the status quo)

4. An `object blah extends Module` within each `foo/bar/qux/build.sc`
file can be referenced in code via `foo.bar.qux.blah`, or referenced
from the command line via `foo.bar.qux.blah`

5. The base modules of `module.sc` files do not have the `MainModule`
tasks: `init`, `version`, `clean`, etc.. Only the base module of the
root `build.sc` file has those


## Design 


### Uniform vs Non-uniform hierarchy

One design decision here is whether a `module.sc` file in a subfolder
`foo/bar/` containing `object qux{ def baz }` would have their targets
referenced via `foo.bar.qux.baz` syntax, or via some alternative e.g.
`foo/bar/qux.baz`.

A non-uniform hierarchy `foo/bar/qux.baz` would be similar to how Bazel
treats folders v.s. targets non-uniformly `foo/bar:qux-baz`, and also
similar to how external modules in Mill are handled e.g.
`mill.idea.GenIdea/idea`, as well as existing foreign modules. However,
it introduces significant user-facing complexity:

1. What's the difference between `foo/bar/qux.baz` vs `foo/bar.qux.baz`
or `foo/bar/qux/baz`?
2. What query syntax would we use to query targets in all nested
`module.sc` files rather than just the top-level one e.g. `__.compile`?
3. Would there be a correspondingly different way of referencing nested
`module.sc` modules and targets in Scala code as well?

Bazel has significant complexity to handle these cases, e.g. query via
`...` vs `:all` vs `*`. It works, but it does complicate the user-facing
semantics.

The alternative of a uniform hierarchy also has downsides: 

1. How do you go from a module name e.g. `foo.bar.qux.baz` to the
`build.sc` or `module.sc` file in which it is defined?
2. If a module is defined in both the root `build.sc` and in a nested
`module.sc`, what happens?

I decided to go with a uniform hierarchy where everything, both in
top-level `build.sc` and in nested `module.sc`, end up squashed together
in a single uniform `foo.bar.qux.baz` hierarchy.

### Package Objects

The goal of this is to try and make modules defined in nested
`module.sc` files "look the same" as modules defined in the root
`build.sc`. There are two possible approaches:

1. Splice the source code of the various nested `module.sc` files into
the top-level `object build`. This is possible, but very complex and
error prone. Especially when it comes to reporting proper error
locations in stack traces (filename/linenumber), this will likely
require a custom compiler plugin similar to the `LineNumberPlugin` we
have today

5. Convert the `object`s into `package object`s, such that module tree
defined in the root `build.sc` becomes synonymous with the JVM package
tree. While the `package object`s will cause the compiler to synthesize
`object package { ... }` wrappers, that is mostly hidden from the end
user.

I decided to go with (2) because it seemed much simpler, making use of
existing language features rather than trying to force the behavior we
want using compiler hackery. Although `package object`s may go away at
some point in Scala 3, they should be straightforward to replace with
explicit `export foo.*` statements when that time comes.

### Existing Foreign Modules

Mill already supports existing `foo.sc` files which support targets and
modules being defined within them, but does not support referencing them
from the command line.

I have removed the ability to define targets and modules in random
`foo.sc` files. We should encourage people to put things in `module.sc`,
since that would allow the user to co-locate the build logic within the
folder containing the files it is related to, rather than as a bunch of
loose `foo.sc` scripts. Removing support for modules/targets in `foo.sc`
files greatly simplifies the desugaring of these scripts, and since we
are already making a breaking change by overhauling how per-folder
`module.sc` files work we might as well bundle this additional breakage
together (rather than making multiple breaking changes in series)

### `build.sc`/`module.sc` file discovery

For this implementation, I chose to make `module.sc` files discovered
automatically by traversing the filesystem: we recursively walk the
subfolders of the root `build.sc` project, look for any files named
`module.sc`. We only traverse folders with `module.sc` files to avoid
having to traverse the entire filesystem structure every time. Empty
`module.sc` files can be used as necessary to allow `module.sc` files to
be placed deeper in the folder tree

This matches the behavior of Bazel and SBT in discovering their
`BUILD`/`build.sbt` files, and notably goes against Maven/Gradle which
require submodules/subprojects to be declared in the top level build
config.

This design has the following characteristics:

1. In future, if we wish to allow `mill` invocations from within a
subfolder, the distinction between `build.sc` and `module.sc` allows us
to easily find the "enclosing" project root.

2. It ensures that any folders containing `build.sc`/`module.sc` files
that accidentally get included within a Mill build do not end up getting
picked up and confusing the top-level build, because we automatically
skip any subfolders containing `build.sc`

3. Similarly, it ensures that adding a `build.sc` file "enclosing" an
existing project, it would not affect Mill invocations in the inner
project, because we only walk to the nearest enclosing `build.sc` file
to find the project root

4. We do not automatically traverse deeply into sub-folders to discover
`module.sc` files, which means that it should be almost impossible to
accidentally pick up `module.sc` files that happen to be on the
filesystem but you did not intend to include in the build

This mechanism should do the right thing 99.9% of the time. For the last
0.1% where it doesn't do the right thing, we can add a
`.millignore`/`.config/millignore` file to support ignoring things we
don't want picked up, but I expect that should be a very rare edge case

## Task Resolution

I have aimed to keep the logic in `resolve/` mostly intact. The core
change is replacing `rootModule: BaseModule` with `baseModules:
BaseModuleTree`, which provides enough metadata to allow
`resolveDirectChildren` and `resolveTransitiveChildren` to find
`BaseModule`s in sub-folders in addition to `Module` `object`s nested
within the parent `Module`. Other than that, the logic should be
basically unchanged, which hopefully should mitigate the risk of
introducing new bugs

## Compatibility

This change is not binary compatible, and the change in the `.sc` file
desugaring is invasive enough we should consider it a major breaking
change. This will need to go into Mill 0.12.x

## Out-Of-Scope/TODO

1. Running `mill` without a subfolder of the enclosing project.
Shouldn't be hard to add given this design, but the PR is complicated
enough as is that I'd like to leave it for follow up

2. Error reporting when a module is duplicated in an enclosing `object`
and in a nested `module.sc` file. Again, probably not hard to add, but
can happen in a follow up

Pull request: #3213
  • Loading branch information
lihaoyi authored Aug 8, 2024
1 parent e29eb1e commit 33fa953
Show file tree
Hide file tree
Showing 68 changed files with 616 additions and 656 deletions.
4 changes: 2 additions & 2 deletions bsp/worker/src/mill/bsp/worker/State.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import mill.eval.Evaluator
private class State(evaluators: Seq[Evaluator], debug: String => Unit) {
lazy val bspModulesById: Map[BuildTargetIdentifier, (BspModule, Evaluator)] = {
val modules: Seq[(Module, Seq[Module], Evaluator)] = evaluators
.map(ev => (ev.rootModule, JavaModuleUtils.transitiveModules(ev.rootModule), ev))
.flatMap(ev => ev.rootModules.map(rm => (rm, JavaModuleUtils.transitiveModules(rm), ev)))

val map = modules
.flatMap { case (rootModule, otherModules, eval) =>
Expand All @@ -30,7 +30,7 @@ private class State(evaluators: Seq[Evaluator], debug: String => Unit) {
map
}

lazy val rootModules: Seq[mill.define.BaseModule] = evaluators.map(_.rootModule)
lazy val rootModules: Seq[mill.define.BaseModule] = evaluators.flatMap(_.rootModules)

lazy val bspIdByModule: Map[BspModule, BuildTargetIdentifier] =
bspModulesById.view.mapValues(_._1).map(_.swap).toMap
Expand Down
13 changes: 12 additions & 1 deletion build.sc
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,18 @@ trait MillStableScalaModule extends MillPublishScalaModule with Mima {
),
ProblemFilter.exclude[ReversedMissingMethodProblem](
"mill.scalalib.JavaModule.mill$scalalib$JavaModule$$super$runMain"
)
),

// Not sure why mima is picking up this stuff which is private[mill]
ProblemFilter.exclude[Problem]("mill.resolve.*.resolve0"),
ProblemFilter.exclude[Problem]("mill.resolve.*.resolveRootModule"),

// These methods are private so it doesn't matter
ProblemFilter.exclude[ReversedMissingMethodProblem]("mill.resolve.Resolve.handleResolved"),
ProblemFilter.exclude[Problem]("mill.resolve.*.resolveNonEmptyAndHandle*"),
ProblemFilter.exclude[Problem]("mill.resolve.ResolveCore*"),
ProblemFilter.exclude[InheritedNewAbstractMethodProblem]("mill.main.MainModule.mill$define$BaseModule0$_setter_$watchedValues_="),
ProblemFilter.exclude[InheritedNewAbstractMethodProblem]("mill.main.MainModule.mill$define$BaseModule0$_setter_$evalWatchedValues_="),
)
def mimaPreviousVersions: T[Seq[String]] = Settings.mimaBaseVersions

Expand Down
13 changes: 13 additions & 0 deletions ci/mill-bootstrap.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
diff --git a/build.sc b/build.sc
index 526ebaa9cf..616be491f9 100644
--- a/build.sc
+++ b/build.sc
@@ -1968,7 +1968,7 @@ def uploadToGithub(authKey: String) = T.command {

private def resolveTasks[T](taskNames: String*): Seq[NamedTask[T]] = {
mill.resolve.Resolve.Tasks.resolve(
- build,
+ build.`package`,
taskNames,
SelectMode.Separated
).map(x => x.asInstanceOf[Seq[mill.define.NamedTask[T]]]).getOrElse(???)
2 changes: 1 addition & 1 deletion contrib/bloop/src/mill/contrib/bloop/BloopImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class BloopImpl(evs: () => Seq[Evaluator], wd: os.Path) extends ExternalModule {
val evals = evs()
evals.flatMap { eval =>
if (eval != null)
JavaModuleUtils.transitiveModules(eval.rootModule, accept)
eval.rootModules.flatMap(JavaModuleUtils.transitiveModules(_, accept))
.collect { case jm: JavaModule => jm }
else
Seq.empty
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,15 @@ trait ScoverageReport extends Module {
dataTargets: String
): Task[PathRef] = {
val sourcesTasks: Seq[Task[Seq[PathRef]]] = Resolve.Tasks.resolve(
evaluator.rootModule,
evaluator.rootModules,
Seq(sources),
SelectMode.Separated
) match {
case Left(err) => throw new Exception(err)
case Right(tasks) => tasks.asInstanceOf[Seq[Task[Seq[PathRef]]]]
}
val dataTasks: Seq[Task[PathRef]] = Resolve.Tasks.resolve(
evaluator.rootModule,
evaluator.rootModules,
Seq(dataTargets),
SelectMode.Separated
) match {
Expand Down
2 changes: 1 addition & 1 deletion example/javaweb/4-hello-micronaut/build.sc
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ trait MicronautModule extends MavenModule{
> mill runBackground
> curl http://localhost:8087/hello
> curl http://localhost:8088/hello
...Hello World...
> mill clean runBackground
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
#Tue Jun 18 12:49:41 UTC 2024
micronaut.application.name=default
micronaut.server.port=8087
micronaut.server.port=8088
Empty file.
5 changes: 5 additions & 0 deletions example/misc/8-multi-build-file/bar/qux/module.sc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import mill._, scalalib._

object module extends build.MyModule {
def ivyDeps = Agg(ivy"com.lihaoyi::scalatags:0.8.2")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package bar.qux
import scalatags.Text.all._
object BarQux {
def printText(text: String): Unit = {
val value = p("world")
println("BarQux.value: " + value)
}
def main(args: Array[String]) = printText(args(0))
}
30 changes: 30 additions & 0 deletions example/misc/8-multi-build-file/build.sc
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import mill._, scalalib._

trait MyModule extends ScalaModule {
def scalaVersion = "2.13.11"
}

// Example Docs


/** Usage
> ./mill resolve __
bar
...
bar.qux.module
...
bar.qux.module.compile
...
foo
...
foo.compile
> ./mill bar.qux.module.compile
> ./mill foo.compile
> ./mill foo.run --foo-text hello --bar-qux-text world
Foo.value: hello
BarQux.value: <p>world</p>
*/
6 changes: 6 additions & 0 deletions example/misc/8-multi-build-file/foo/module.sc
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import mill._, scalalib._

object build extends RootModule with MyModule {
def moduleDeps = Seq(bar.qux.module)
def ivyDeps = Agg(ivy"com.lihaoyi::mainargs:0.4.0")
}
14 changes: 14 additions & 0 deletions example/misc/8-multi-build-file/foo/src/Foo.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package foo
import mainargs.{main, ParserForMethods, arg}
object Foo {
val value = "hello"

@main
def main(@arg(name = "foo-text") fooText: String,
@arg(name = "bar-qux-text") barQuxText: String): Unit = {
println("Foo.value: " + Foo.value)
bar.qux.BarQux.printText(barQuxText)
}

def main(args: Array[String]): Unit = ParserForMethods(this).runOrExit(args)
}
13 changes: 6 additions & 7 deletions example/src/mill/integration/ExampleTestSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,17 @@ object ExampleTestSuite extends IntegrationTestSuite {
}

test("exampleUsage") {

val parsed = upickle.default.read[Seq[(String, String)]](sys.env("MILL_EXAMPLE_PARSED"))
val usageComment = parsed.collect { case ("example", txt) => txt }.mkString("\n\n")
val commandBlocks = ("\n" + usageComment.trim).split("\n> ").filter(_.nonEmpty)

retryOnTimeout(3) {
try {
for (commandBlock <- commandBlocks) processCommandBlock(workspaceRoot, commandBlock)
if (integrationTestMode != "fork") evalStdout("shutdown")
} finally {
try os.remove.all(workspaceRoot / "out")
catch { case e: Throwable => /*do nothing*/ }
}
try os.remove.all(workspaceRoot / "out")
catch { case e: Throwable => /*do nothing*/ }

for (commandBlock <- commandBlocks) processCommandBlock(workspaceRoot, commandBlock)
if (integrationTestMode != "fork") evalStdout("shutdown")
}
}
}
Expand Down
8 changes: 5 additions & 3 deletions idea/src/mill/idea/GenIdeaImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ case class GenIdeaImpl(
)(implicit ctx: Ctx) {
import GenIdeaImpl._

val workDir: os.Path = evaluators.head.rootModule.millSourcePath
val workDir: os.Path = evaluators.head.rootModules.head.millSourcePath
val ideaDir: os.Path = workDir / ".idea"

val ideaConfigVersion = 4
Expand Down Expand Up @@ -67,7 +67,9 @@ case class GenIdeaImpl(
fetchMillModules: Boolean = true
): Seq[(os.SubPath, scala.xml.Node)] = {

val rootModules = evaluators.zipWithIndex.map { case (ev, idx) => (ev.rootModule, ev, idx) }
val rootModules = evaluators.zipWithIndex.map { case (ev, idx) =>
(ev.rootModules.head, ev, idx)
}
val transitive: Seq[(BaseModule, Seq[Module], Evaluator, Int)] = rootModules
.map { case (rootModule, ev, idx) =>
(rootModule, JavaModuleUtils.transitiveModules(rootModule), ev, idx)
Expand Down Expand Up @@ -118,7 +120,7 @@ case class GenIdeaImpl(
}

// is head the right one?
val buildDepsPaths = Classpath.allJars(evaluators.head.rootModule.getClass.getClassLoader)
val buildDepsPaths = Classpath.allJars(evaluators.head.rootModules.head.getClass.getClassLoader)
.map(url => os.Path(java.nio.file.Paths.get(url.toURI)))

def resolveTasks: Map[Evaluator, Seq[Task[ResolvedModule]]] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ object CompileErrorTests extends IntegrationTestSuite {
assert(res.err.contains("""println(doesntExist)"""))
assert(res.err.contains("""qux.sc:3:34: type mismatch;"""))
assert(res.err.contains(
"""build.sc:8:5: value noSuchMethod is not a member of object build.this.foo"""
"""build.sc:8:5: value noSuchMethod is not a member of object"""
))
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ object CrossCollisionsTests extends IntegrationTestSuite {
test("detect-collision") {
val res = evalStdout("resolve", "foo._")
assert(!res.isSuccess)
assert(res.err.contains("Cross module "))
assert(
res.err.contains(
"Cross module millbuild.build#foo contains colliding cross values: List(List(a, b), List(c)) and List(List(a), List(b, c))"
" contains colliding cross values: List(List(a, b), List(c)) and List(List(a), List(b, c))"
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ object MultipleTopLevelModulesTests extends IntegrationTestSuite {
val res = evalStdout("resolve", "_")
assert(!res.isSuccess)
assert(res.err.contains(
"Only one RootModule can be defined in a build, not 2: millbuild.build$bar$,millbuild.build$foo$"
"Only one RootModule can be defined in a build, not 2:"
))
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ object DocAnnotationsTests extends IntegrationTestSuite {

assert(eval("inspect", "core.target"))
val target = ujson.read(meta("inspect"))("value").str
pprint.log(target)
assert(
globMatches(
"""core.target(build.sc:...)
Expand Down
23 changes: 0 additions & 23 deletions integration/feature/foreign/repo/conflict/build.sc

This file was deleted.

3 changes: 0 additions & 3 deletions integration/feature/foreign/repo/conflict/inner/build.sc

This file was deleted.

30 changes: 0 additions & 30 deletions integration/feature/foreign/repo/outer/build.sc

This file was deleted.

13 changes: 0 additions & 13 deletions integration/feature/foreign/repo/outer/inner/build.sc

This file was deleted.

Loading

0 comments on commit 33fa953

Please sign in to comment.