Skip to content

Commit

Permalink
Move BuildInfo from sources into resources, defer use of resources du…
Browse files Browse the repository at this point in the history 
…ring compilation (#2425)

Instead of compiling the buildinfo values into the source code, we only
compile the buildinfo keys into the source code and store the buildinfo
values in the resources, to be loaded at runtime. This greatly reduces
the amount of time spent compiling things when buildinfo changes. In
this repo, some rough tests indicate that it makes the second `-i
installLocal` after a one-line change in the `LICENSE` file drop from
50s to 14s of runtime

`mill-profile.json` Before:


[mill-profile.txt](https://github.com/com-lihaoyi/mill/files/11177836/mill-profile.txt)


`mill-profile.json` After:


[mill-profile.txt](https://github.com/com-lihaoyi/mill/files/11177798/mill-profile.txt)

Notes:

1. To really benefit from this, I had to make `compileClasspath` depend
only on upstream module's `compileClasspath()`/`compile().classes`, so
it doesn't end up depending on their runtime `resources()` and forcing
unnecessary recompiles when the buildinfo resources change. This is
probably a change we want to do anyway, given the direction set by
#1811

2. I consolidated the BuildInfo implementation with
`contrib/buildinfo/`.

3. Extended `contrib/buildinfo/` to support Java modules, which we
dogfood in `main.client`.

4. The old behavior of statically compiling the values into the binary
is still available under `def buildInfoStaticCompiled = true`, for
anyone who needs it. That includes anyone using `BuildInfo` in Scala.js,
which doesn't support JVM resources by default

5. For now, I copy-pasted the implementation into the root `build.sc`
file, and updated `mill-bootstrap.patch` to remove it and make use of
the `mill-contrib` version. When we re-bootstrap Mill on latest main, we
can remove the copy-pasta

6. `contrib/buildinfo/` should be mostly source compatible with the
previous implementation, except that I made `buildInfoPackageName` a
required field to encourage the best practice of not putting code in the
empty package
  • Loading branch information
@lihaoyi
lihaoyi committed Apr 8, 2023
1 parent 3d0a251 commit d9cf409
Show file tree
Hide file tree
Showing 18 changed files with 942 additions and 388 deletions.
502 changes: 285 additions & 217 deletions build.sc
View file

Large diffs are not rendered by default.

225 changes: 215 additions & 10 deletions ci/mill-bootstrap.patch
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
diff --git a/build.sc b/build.sc
index 4ae8bd899..c7647c7cc 100644
index cbc7b7aaa..351a37fbc 100644
--- a/build.sc
+++ b/build.sc
@@ -2,22 +2,10 @@
@@ -2,22 +2,12 @@
import $file.ci.shared
import $file.ci.upload
import $ivy.`org.scalaj::scalaj-http:2.4.2`
-import $ivy.`de.tototec::de.tobiasroeser.mill.vcs.version_mill0.10:0.3.0`
-import $ivy.`com.github.lolgab::mill-mima_mill0.10:0.0.13`
import $ivy.`net.sourceforge.htmlcleaner:htmlcleaner:2.25`
+import $ivy.`com.lihaoyi::mill-contrib-buildinfo:$MILL_VERSION`
+import mill.contrib.buildinfo.BuildInfo

// imports
-import com.github.lolgab.mill.mima
Expand All @@ -25,7 +27,7 @@ index 4ae8bd899..c7647c7cc 100644
import mill._
import mill.define.{Command, Source, Sources, Target, Task}
import mill.eval.Evaluator
@@ -179,12 +167,8 @@ object Deps {
@@ -185,12 +175,8 @@ object Deps {
val requests = ivy"com.lihaoyi::requests:0.8.0"
}

Expand All @@ -40,7 +42,210 @@ index 4ae8bd899..c7647c7cc 100644
def millBinPlatform: T[String] = T {
val tag = millLastTag()
if (tag.contains("-M")) tag
@@ -244,27 +228,8 @@ trait MillCoursierModule extends CoursierModule {
@@ -202,202 +188,6 @@ def millBinPlatform: T[String] = T {
def baseDir = build.millSourcePath


-trait BuildInfo extends JavaModule {
- /**
- * The package name under which the BuildInfo data object will be stored.
- */
- def buildInfoPackageName: String
-
- /**
- * The name of the BuildInfo data object, defaults to "BuildInfo"
- */
- def buildInfoObjectName: String = "BuildInfo"
-
- /**
- * Enable to compile the BuildInfo values directly into the classfiles,
- * rather than the default behavior of storing them as a JVM resource. Needed
- * to use BuildInfo on Scala.js which does not support JVM resources
- */
- def buildInfoStaticCompiled: Boolean = false
-
- /**
- * A mapping of key-value pairs to pass from the Build script to the
- * application code at runtime.
- */
- def buildInfoMembers: T[Seq[BuildInfo.Value]] = Seq.empty[BuildInfo.Value]
-
- def resources =
- if (buildInfoStaticCompiled) super.resources
- else T.sources{ super.resources() ++ Seq(buildInfoResources()) }
-
- def buildInfoResources = T{
- val p = new java.util.Properties
- for (v <- buildInfoMembers()) p.setProperty(v.key, v.value)
-
- val stream = os.write.outputStream(
- T.dest / os.SubPath(buildInfoPackageName.replace('.', '/')) / s"$buildInfoObjectName.buildinfo.properties",
- createFolders = true
- )
-
- p.store(stream, s"mill.contrib.buildinfo.BuildInfo for ${buildInfoPackageName}.${buildInfoObjectName}")
- stream.close()
- PathRef(T.dest)
- }
-
- private def isScala = this.isInstanceOf[ScalaModule]
-
- override def generatedSources = T {
- super.generatedSources() ++ buildInfoSources()
- }
-
- def buildInfoSources = T{
- if (buildInfoMembers().isEmpty) Nil
- else {
- val code = if (buildInfoStaticCompiled) BuildInfo.staticCompiledCodegen(
- buildInfoMembers(), isScala, buildInfoPackageName, buildInfoObjectName
- ) else BuildInfo.codegen(
- buildInfoMembers(), isScala, buildInfoPackageName, buildInfoObjectName
- )
-
- val ext = if (isScala) "scala" else "java"
-
- os.write(
- T.dest / buildInfoPackageName.split('.') / s"${buildInfoObjectName}.$ext",
- code,
- createFolders = true
- )
- Seq(PathRef(T.dest))
- }
- }
-}
-
-object BuildInfo{
- case class Value(key: String, value: String, comment: String = "")
- object Value{
- implicit val rw: upickle.default.ReadWriter[Value] = upickle.default.macroRW
- }
- def staticCompiledCodegen(buildInfoMembers: Seq[Value],
- isScala: Boolean,
- buildInfoPackageName: String,
- buildInfoObjectName: String): String = {
- val bindingsCode = buildInfoMembers
- .sortBy(_.key)
- .map {
- case v =>
- if (isScala) s"""${commentStr(v)}val ${v.key} = ${pprint.Util.literalize(v.value)}"""
- else s"""${commentStr(v)}public static java.lang.String ${v.key} = ${pprint.Util.literalize(v.value)};"""
- }
- .mkString("\n\n ")
-
-
- if (isScala) {
- val mapEntries = buildInfoMembers
- .map { case v => s""""${v.key}" -> ${v.key}"""}
- .mkString(",\n")
-
- s"""
- |package $buildInfoPackageName
- |
- |object $buildInfoObjectName {
- | $bindingsCode
- | val toMap = Map[String, String](
- | $mapEntries
- | )
- |}
- """.stripMargin.trim
- } else {
- val mapEntries = buildInfoMembers
- .map { case v => s"""map.put("${v.key}", ${v.key});""" }
- .mkString(",\n")
-
- s"""
- |package $buildInfoPackageName;
- |
- |public class $buildInfoObjectName {
- | $bindingsCode
- |
- | public static java.util.Map<String, String> toMap(){
- | Map<String, String> map = new HashMap<String, String>();
- | $mapEntries
- | return map;
- | }
- |}
- """.stripMargin.trim
- }
- }
-
- def codegen(buildInfoMembers: Seq[Value],
- isScala: Boolean,
- buildInfoPackageName: String,
- buildInfoObjectName: String): String = {
- val bindingsCode = buildInfoMembers
- .sortBy(_.key)
- .map {
- case v =>
- if (isScala) s"""${commentStr(v)}val ${v.key} = buildInfoProperties.getProperty("${v.key}")"""
- else s"""${commentStr(v)}public static final java.lang.String ${v.key} = buildInfoProperties.getProperty("${v.key}");"""
- }
- .mkString("\n\n ")
-
- if (isScala)
- s"""
- |package ${buildInfoPackageName}
- |
- |object $buildInfoObjectName {
- | private val buildInfoProperties = new java.util.Properties
- |
- | private val buildInfoInputStream = getClass
- | .getResourceAsStream("$buildInfoObjectName.buildinfo.properties")
- |
- | buildInfoProperties.load(buildInfoInputStream)
- |
- | $bindingsCode
- |}
- """.stripMargin.trim
- else
- s"""
- |package ${buildInfoPackageName};
- |
- |public class $buildInfoObjectName {
- | private static java.util.Properties buildInfoProperties = new java.util.Properties();
- |
- | static {
- | java.io.InputStream buildInfoInputStream = $buildInfoObjectName
- | .class
- | .getResourceAsStream("$buildInfoObjectName.buildinfo.properties");
- |
- | try{
- | buildInfoProperties.load(buildInfoInputStream);
- | }catch(java.io.IOException e){
- | throw new RuntimeException(e);
- | }finally{
- | try{
- | buildInfoInputStream.close();
- | }catch(java.io.IOException e){
- | throw new RuntimeException(e);
- | }
- | }
- | }
- |
- | $bindingsCode
- |}
- """.stripMargin.trim
- }
-
- def commentStr(v: Value) = {
- if (v.comment.isEmpty) ""
- else {
- val lines = v.comment.linesIterator.toVector
- lines.length match{
- case 1 => s"""/** ${v.comment} */\n """
- case _ => s"""/**\n ${lines.map("* " + _).mkString("\n ")}\n */\n """
- }
-
- }
- }
-}
-
-
trait MillPublishModule extends PublishModule {
override def artifactName = "mill-" + super.artifactName()
def publishVersion = millVersion()
@@ -447,27 +237,8 @@ trait MillCoursierModule extends CoursierModule {
)
}

Expand Down Expand Up @@ -69,40 +274,40 @@ index 4ae8bd899..c7647c7cc 100644
}

/** A Module compiled with applied Mill-specific compiler plugins: mill-moduledefs. */
@@ -658,6 +623,7 @@ object scalajslib extends MillModule {
@@ -758,6 +529,7 @@ object scalajslib extends MillModule with BuildInfo{
}
object worker extends Cross[WorkerModule]("1")
class WorkerModule(scalajsWorkerVersion: String) extends MillInternalModule {
+ override def millSourcePath: os.Path = super.millSourcePath / scalajsWorkerVersion
override def moduleDeps = Seq(scalajslib.`worker-api`)
override def ivyDeps = Agg(
Deps.Scalajs_1.scalajsLinker,
@@ -720,6 +686,7 @@ object contrib extends MillModule {
@@ -820,6 +592,7 @@ object contrib extends MillModule {

object worker extends Cross[WorkerModule](Deps.play.keys.toSeq: _*)
class WorkerModule(playBinary: String) extends MillInternalModule {
+ override def millSourcePath: os.Path = super.millSourcePath / playBinary
override def sources = T.sources {
// We want to avoid duplicating code as long as the Play APIs allow.
// But if newer Play versions introduce incompatibilities,
@@ -922,6 +889,7 @@ object scalanativelib extends MillModule {
@@ -1011,6 +784,7 @@ object scalanativelib extends MillModule {
object worker extends Cross[WorkerModule]("0.4")
class WorkerModule(scalaNativeWorkerVersion: String)
extends MillInternalModule {
+ override def millSourcePath: os.Path = super.millSourcePath / scalaNativeWorkerVersion
override def moduleDeps = Seq(scalanativelib.`worker-api`)
override def ivyDeps = scalaNativeWorkerVersion match {
case "0.4" =>
@@ -1104,6 +1072,7 @@ trait IntegrationTestModule extends MillScalaModule {
@@ -1171,6 +945,7 @@ trait IntegrationTestModule extends MillScalaModule {
}

trait IntegrationTestCrossModule extends IntegrationTestModule {
+ override def millSourcePath = super.millSourcePath / repoSlug
object local extends ModeModule
object fork extends ModeModule
object server extends ModeModule
@@ -1657,53 +1626,7 @@ def launcher = T {
}
@@ -1725,53 +1500,7 @@ def launcher = T {


def uploadToGithub(authKey: String) = T.command {
- val vcsState = VcsVersion.vcsState()
Expand Down
11 changes: 11 additions & 0 deletions ci/prepare-mill-bootstrap.sh
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/usr/bin/env sh
# Apply a patch, if present, before bootstrapping

set -eux

./mill contrib.buildinfo.publishLocal

# Patch local build
ci/patch-mill-bootstrap.sh


8 changes: 4 additions & 4 deletions ci/test-mill-bootstrap-0.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ rm -rf ~/.mill/ammonite
git config user.name "Your Name"
echo "Build 2" > info.txt && git add info.txt && git commit -m "Add info.txt"

# Patch local build
ci/patch-mill-bootstrap.sh
# Prepare local build
ci/prepare-mill-bootstrap.sh

# Second build
target/mill-1 -i -j 0 installLocal --binFile target/mill-2
Expand All @@ -35,8 +35,8 @@ git stash pop "$(git stash list | grep "preserve mill-2" | head -n1 | sed -E 's/

rm -rf ~/.mill/ammonite

# Patch local build
ci/patch-mill-bootstrap.sh
# Prepare local build
ci/prepare-mill-bootstrap.sh

# Use second build to run tests using Mill
target/mill-2 -i "{main,scalalib,scalajslib,scalanativelib,bsp}.__.test"
Expand Down
8 changes: 4 additions & 4 deletions ci/test-mill-bootstrap-1.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ rm -rf ~/.mill/ammonite
git config user.name "Your Name"
echo "Build 2" > info.txt && git add info.txt && git commit -m "Add info.txt"

# Patch local build
ci/patch-mill-bootstrap.sh
# Prepare local build
ci/prepare-mill-bootstrap.sh

# Second build
target/mill-1 -i -j 0 installLocal --binFile target/mill-2
Expand All @@ -35,8 +35,8 @@ git stash pop "$(git stash list | grep "preserve mill-2" | head -n1 | sed -E 's/

rm -rf ~/.mill/ammonite

# Patch local build
ci/patch-mill-bootstrap.sh
# Prepare local build
ci/prepare-mill-bootstrap.sh

# Use second build to run tests using Mill
target/mill-2 -i "example.basic[1-hello-world].server.test"
Expand Down
6 changes: 3 additions & 3 deletions ci/test-mill-dev.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ git stash -a

rm -rf ~/.mill/ammonite

# Patch local build
ci/patch-mill-bootstrap.sh
# Prepare local build
ci/prepare-mill-bootstrap.sh

# Second build & run tests
out/dev/assembly.dest/mill -i -j 0 main.test.compile

out/dev/assembly.dest/mill -i contrib.buildinfo.publishLocal
out/dev/assembly.dest/mill -i "{main,scalalib,scalajslib,scalanativelib,bsp,contrib.twirllib,contrib.scalapblib}.test"
out/dev/assembly.dest/mill -i "example.basic[1-hello-world].server.test"
4 changes: 2 additions & 2 deletions ci/test-mill-release.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ git stash pop "$(git stash list | grep "preserve mill-release" | head -n1 | sed

rm -rf ~/.mill/ammonite

# Patch local build
ci/patch-mill-bootstrap.sh
# Prepare local build
ci/prepare-mill-bootstrap.sh

export MILL_TEST_RELEASE="$(pwd)/target/mill-release"

Expand Down
Loading

0 comments on commit d9cf409

Please sign in to comment.