Skip to content

Commit

Permalink
Fix incremental compilation when a Scala project depends on a Java pr…
Browse files Browse the repository at this point in the history 
…oject

Before this commit, JavaModule#compile simply called javac
unconditionally, thus generating new classfiles every time. But if a
Scala project depends on a Java project, this will throw off the
incremental compilation algorithm which will unnecessarily recompile
files. To avoid this we now use Zinc to compile Java projects too (as a
bonus this means that Java compilation becomes incremental). This
required some refactoring in ZincWorkerImpl to be able to compile stuff
without having to pass Scala-specific options.

The issue solved by this commit could be reproduced by running in the
Mill repository:

$ mill main.compile
$ mill -i
@ main.compile()

and observing that before this commit, the `main.compile()` call ended
up recompiling code.
  • Loading branch information
@smarter
smarter committed Aug 23, 2018
1 parent 4a7a699 commit 17792b4
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 106 deletions.
24 changes: 17 additions & 7 deletions scalalib/src/mill/scalalib/JavaModule.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ trait JavaModule extends mill.Module with TaskModule { outer =>
override def moduleDeps = Seq(outer)
override def repositories = outer.repositories
override def javacOptions = outer.javacOptions
override def zincWorker = outer.zincWorker
}
def defaultCommandName() = "run"

Expand All @@ -38,7 +39,16 @@ trait JavaModule extends mill.Module with TaskModule { outer =>
def finalMainClassOpt: T[Either[String, String]] = T{
mainClass() match{
case Some(m) => Right(m)
case None => Left("No main class specified or found")
case None =>
zincWorker.worker().discoverMainClasses(compile())match {
case Seq() => Left("No main class specified or found")
case Seq(main) => Right(main)
case mains =>
Left(
s"Multiple main classes found (${mains.mkString(",")}) " +
"please explicitly specify which one to use by overriding mainClass"
)
}
}
}

Expand Down Expand Up @@ -133,12 +143,12 @@ trait JavaModule extends mill.Module with TaskModule { outer =>
} yield PathRef(path)
}

def compile: T[CompilationResult] = T{
Lib.compileJava(
allSourceFiles().map(_.path.toIO).toArray,
compileClasspath().map(_.path.toIO).toArray,
javacOptions(),
upstreamCompileOutput()
def compile: T[CompilationResult] = T.persistent{
zincWorker.worker().compileJava(
upstreamCompileOutput(),
allSourceFiles().map(_.path),
compileClasspath().map(_.path),
javacOptions()
)
}

Expand Down
28 changes: 0 additions & 28 deletions scalalib/src/mill/scalalib/Lib.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,34 +27,6 @@ object CompilationResult {
case class CompilationResult(analysisFile: Path, classes: PathRef)

object Lib{
def compileJava(sources: Array[java.io.File],
classpath: Array[java.io.File],
javaOpts: Seq[String],
upstreamCompileOutput: Seq[CompilationResult])
(implicit ctx: mill.util.Ctx) = {
val javac = ToolProvider.getSystemJavaCompiler()
if (javac == null) {
throw new Exception(
"Your Java installation is not a JDK, so it can't compile Java code;" +
" Please install the JDK version of Java")
}

rm(ctx.dest / 'classes)
mkdir(ctx.dest / 'classes)
val cpArgs =
if(classpath.isEmpty) Seq()
else Seq("-cp", classpath.mkString(File.pathSeparator))

val args = Seq("-d", ctx.dest / 'classes) ++ cpArgs ++ javaOpts ++ sources

javac.run(
ctx.log.inStream, ctx.log.outputStream, ctx.log.errorStream,
args.map(_.toString):_*
)
if (ls(ctx.dest / 'classes).isEmpty) mill.eval.Result.Failure("Compilation Failed")
else mill.eval.Result.Success(CompilationResult(ctx.dest / 'zinc, PathRef(ctx.dest / 'classes)))
}

private val ReleaseVersion = raw"""(\d+)\.(\d+)\.(\d+)""".r
private val MinorSnapshotVersion = raw"""(\d+)\.(\d+)\.([1-9]\d*)-SNAPSHOT""".r
private val DottyVersion = raw"""0\.(\d+)\.(\d+).*""".r
Expand Down
31 changes: 6 additions & 25 deletions scalalib/src/mill/scalalib/ScalaModule.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,29 +60,10 @@ trait ScalaModule extends JavaModule { outer =>
)
}

override def finalMainClassOpt: T[Either[String, String]] = T{
mainClass() match{
case Some(m) => Right(m)
case None =>
zincWorker.worker().discoverMainClasses(compile())match {
case Seq() => Left("No main class specified or found")
case Seq(main) => Right(main)
case mains =>
Left(
s"Multiple main classes found (${mains.mkString(",")}) " +
"please explicitly specify which one to use by overriding mainClass"
)
}
}
}


def scalacPluginIvyDeps = T{ Agg.empty[Dep] }

def scalacOptions = T{ Seq.empty[String] }

override def repositories: Seq[Repository] = zincWorker.repositories

private val Milestone213 = raw"""2.13.(\d+)-M(\d+)""".r

def scalaCompilerBridgeSources = T {
Expand Down Expand Up @@ -142,16 +123,16 @@ trait ScalaModule extends JavaModule { outer =>
}

override def compile: T[CompilationResult] = T.persistent{
zincWorker.worker().compileScala(
scalaVersion(),
zincWorker.worker().compileMixed(
upstreamCompileOutput(),
allSourceFiles().map(_.path),
scalaCompilerBridgeSources(),
compileClasspath().map(_.path),
scalaCompilerClasspath().map(_.path),
javacOptions(),
scalaVersion(),
scalacOptions(),
scalaCompilerBridgeSources(),
scalaCompilerClasspath().map(_.path),
scalacPluginClasspath().map(_.path),
javacOptions(),
upstreamCompileOutput()
)
}

Expand Down
20 changes: 13 additions & 7 deletions scalalib/src/mill/scalalib/ZincWorkerApi.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,25 @@ trait ZincWorkerModule extends mill.Module{
}

trait ZincWorkerApi {
/** Compile a Java-only project */
def compileJava(upstreamCompileOutput: Seq[CompilationResult],
sources: Agg[Path],
compileClasspath: Agg[Path],
javacOptions: Seq[String])
(implicit ctx: mill.util.Ctx): mill.eval.Result[CompilationResult]

def compileScala(scalaVersion: String,
/** Compile a mixed Scala/Java or Scala-only project */
def compileMixed(upstreamCompileOutput: Seq[CompilationResult],
sources: Agg[Path],
compilerBridgeSources: Path,
compileClasspath: Agg[Path],
compilerClasspath: Agg[Path],
scalacOptions: Seq[String],
scalacPluginClasspath: Agg[Path],
javacOptions: Seq[String],
upstreamCompileOutput: Seq[CompilationResult])
scalaVersion: String,
scalacOptions: Seq[String],
compilerBridgeSources: Path,
compilerClasspath: Agg[Path],
scalacPluginClasspath: Agg[Path])
(implicit ctx: mill.util.Ctx): mill.eval.Result[CompilationResult]


def discoverMainClasses(compilationResult: CompilationResult)
(implicit ctx: mill.util.Ctx): Seq[String]
}
129 changes: 90 additions & 39 deletions scalalib/worker/src/mill/scalalib/worker/ZincWorkerImpl.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,29 @@ case class MockedLookup(am: File => Optional[CompileAnalysis]) extends PerClassp

class ZincWorkerImpl(ctx0: mill.util.Ctx,
compilerBridgeClasspath: Array[String]) extends mill.scalalib.ZincWorkerApi{
@volatile var compilersCache = Option.empty[(Long, Compilers)]
private val ic = new sbt.internal.inc.IncrementalCompilerImpl()
val javaOnlyCompilers = {
// Keep the classpath as written by the user
val classpathOptions = ClasspathOptions.of(false, false, false, false, false)

val dummyFile = new java.io.File("")
// Zinc does not have an entry point for Java-only compilation, so we need
// to make up a dummy ScalaCompiler instance.
val scalac = ZincUtil.scalaCompiler(
new ScalaInstance("", null, null, dummyFile, dummyFile, new Array(0), Some("")), null,
classpathOptions // this is used for javac too
)

ic.compilers(
instance = null,
classpathOptions,
None,
scalac
)
}

@volatile var mixedCompilersCache = Option.empty[(Long, Compilers)]


/** Compile the bridge if it doesn't exist yet and return the output directory.
* TODO: Proper invalidation, see #389
Expand Down Expand Up @@ -78,27 +100,38 @@ class ZincWorkerImpl(ctx0: mill.util.Ctx,
.getOrElse(Seq.empty[String])
}

def compileJava(upstreamCompileOutput: Seq[CompilationResult],
sources: Agg[Path],
compileClasspath: Agg[Path],
javacOptions: Seq[String])
(implicit ctx: mill.util.Ctx): mill.eval.Result[CompilationResult] = {
compileInternal(
upstreamCompileOutput,
sources,
compileClasspath,
javacOptions,
scalacOptions = Nil,
javaOnlyCompilers
)
}

def compileScala(scalaVersion: String,
def compileMixed(upstreamCompileOutput: Seq[CompilationResult],
sources: Agg[Path],
compilerBridgeSources: Path,
compileClasspath: Agg[Path],
compilerClasspath: Agg[Path],
scalacOptions: Seq[String],
scalacPluginClasspath: Agg[Path],
javacOptions: Seq[String],
upstreamCompileOutput: Seq[CompilationResult])
scalaVersion: String,
scalacOptions: Seq[String],
compilerBridgeSources: Path,
compilerClasspath: Agg[Path],
scalacPluginClasspath: Agg[Path])
(implicit ctx: mill.util.Ctx): mill.eval.Result[CompilationResult] = {
val compileClasspathFiles = compileClasspath.map(_.toIO).toArray
val compilerJars = compilerClasspath.toArray.map(_.toIO)

val compilerBridge = compileZincBridgeIfNeeded(scalaVersion, compilerBridgeSources, compilerJars)

val ic = new sbt.internal.inc.IncrementalCompilerImpl()

val compilerBridgeSig = compilerBridge.mtime.toMillis

val compilersSig = compilerBridgeSig + compilerClasspath.map(p => p.toString().hashCode + p.mtime.toMillis).sum
val compilers = compilersCache match {
val compilers = mixedCompilersCache match {
case Some((k, v)) if k == compilersSig => v
case _ =>
val compilerName =
Expand All @@ -120,12 +153,28 @@ class ZincWorkerImpl(ctx0: mill.util.Ctx,
None,
ZincUtil.scalaCompiler(scalaInstance, compilerBridge.toIO)
)
compilersCache = Some((compilersSig, compilers))
mixedCompilersCache = Some((compilersSig, compilers))
compilers
}

mkdir(ctx.dest)
compileInternal(
upstreamCompileOutput,
sources,
compileClasspath,
javacOptions,
scalacOptions = scalacPluginClasspath.map(jar => s"-Xplugin:${jar}").toSeq ++ scalacOptions,
compilers
)
}

private def compileInternal(upstreamCompileOutput: Seq[CompilationResult],
sources: Agg[Path],
compileClasspath: Agg[Path],
javacOptions: Seq[String],
scalacOptions: Seq[String],
compilers: Compilers)
(implicit ctx: mill.util.Ctx): mill.eval.Result[CompilationResult] = {
mkdir(ctx.dest)

val logger = {
val consoleAppender = MainAppender.defaultScreen(ConsoleOut.printStreamOut(
Expand Down Expand Up @@ -158,33 +207,35 @@ class ZincWorkerImpl(ctx0: mill.util.Ctx,

val store = FileAnalysisStore.binary(zincIOFile)

val inputs = ic.inputs(
classpath = classesIODir +: compileClasspath.map(_.toIO).toArray,
sources = sources.toArray.map(_.toIO),
classesDirectory = classesIODir,
scalacOptions = scalacOptions.toArray,
javacOptions = javacOptions.toArray,
maxErrors = 10,
sourcePositionMappers = Array(),
order = CompileOrder.Mixed,
compilers = compilers,
setup = ic.setup(
lookup,
skip = false,
zincIOFile,
new FreshCompilerCache,
IncOptions.of(),
new ManagedLoggedReporter(10, logger),
None,
Array()
),
pr = {
val prev = store.get()
PreviousResult.of(prev.map(_.getAnalysis), prev.map(_.getMiniSetup))
}
)

try {
val newResult = ic.compile(
ic.inputs(
classpath = classesIODir +: compileClasspathFiles,
sources = sources.toArray.map(_.toIO),
classesDirectory = classesIODir,
scalacOptions = (scalacPluginClasspath.map(jar => s"-Xplugin:${jar}") ++ scalacOptions).toArray,
javacOptions = javacOptions.toArray,
maxErrors = 10,
sourcePositionMappers = Array(),
order = CompileOrder.Mixed,
compilers = compilers,
setup = ic.setup(
lookup,
skip = false,
zincIOFile,
new FreshCompilerCache,
IncOptions.of(),
new ManagedLoggedReporter(10, logger),
None,
Array()
),
pr = {
val prev = store.get()
PreviousResult.of(prev.map(_.getAnalysis), prev.map(_.getMiniSetup))
}
),
in = inputs,
logger = logger
)

Expand Down

0 comments on commit 17792b4

Please sign in to comment.