Skip to content

Commit

Permalink
Fixed semanticdb file copying issue, added more tests (#3080)
Browse files Browse the repository at this point in the history
Added incremental test for `JavaModule.semanticDbData` and
`ScalaModule.semanticDbData`. Looks like zinc isn't compiling
incrementally at all when we stop at an early compiler phase. The test
don't really catch that, yet. But I also found a bug where we moved
files instead of copying them.

Fix #3077

Pull request: #3080
  • Loading branch information
lefou authored Apr 25, 2024
1 parent 937802b commit 654f585
Show file tree
Hide file tree
Showing 5 changed files with 185 additions and 42 deletions.
8 changes: 6 additions & 2 deletions scalalib/src/mill/scalalib/ScalaModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -676,8 +676,12 @@ trait ScalaModule extends JavaModule with TestModule.ScalaModuleBase { outer =>
incrementalCompilation = zincIncrementalCompilation(),
auxiliaryClassFileExtensions = zincAuxiliaryClassFileExtensions()
)
.map(r =>
SemanticDbJavaModule.copySemanticdbFiles(r.classes.path, T.workspace, T.dest / "data")
.map(compileRes =>
SemanticDbJavaModule.copySemanticdbFiles(
compileRes.classes.path,
T.workspace,
T.dest / "data"
)
)
}
}
5 changes: 4 additions & 1 deletion scalalib/src/mill/scalalib/SemanticDbJavaModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -214,13 +214,16 @@ object SemanticDbJavaModule {
sourceroot: os.Path,
targetDir: os.Path
): PathRef = {
assert(classesDir != targetDir)
os.remove.all(targetDir)
os.makeDir.all(targetDir)

val ups = sourceroot.segments.size
val semanticPath = os.rel / "META-INF" / "semanticdb"
val toClean = classesDir / semanticPath / sourceroot.segments.toSeq

// copy over all found semanticdb-files into the target directory
// but with corrected directory layout
os.walk(classesDir, preOrder = true)
.filter(os.isFile)
.foreach { p =>
Expand All @@ -231,7 +234,7 @@ object SemanticDbJavaModule {
} else {
targetDir / p.relativeTo(classesDir)
}
os.move(p, target, createFolders = true)
os.copy(p, target, createFolders = true)
}
}
PathRef(targetDir)
Expand Down
90 changes: 75 additions & 15 deletions scalalib/test/src/mill/scalalib/HelloJavaTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -55,24 +55,84 @@ object HelloJavaTests extends TestSuite {
)
}
"semanticDbData" - {
val eval = init()
val Right((result, evalCount)) = eval.apply(HelloJava.core.semanticDbData)
val expectedFile1 =
os.rel / "META-INF" / "semanticdb" / "core" / "src" / "Core.java.semanticdb"

val outputFiles = os.walk(result.path).filter(os.isFile)
val dataPath = eval.outPath / "core" / "semanticDbData.dest" / "data"
"fromScratch" - {
val eval = init()
val Right((result, evalCount)) = eval.apply(HelloJava.core.semanticDbData)

val expectedSemFiles =
Seq(dataPath / "META-INF" / "semanticdb" / "core" / "src" / "Core.java.semanticdb")
assert(
result.path == dataPath,
outputFiles.nonEmpty,
outputFiles.forall(expectedSemFiles.contains),
evalCount > 0
)
val outputFiles = os.walk(result.path).filter(os.isFile).map(_.relativeTo(result.path))
val dataPath = eval.outPath / "core" / "semanticDbData.dest" / "data"

assert(
result.path == dataPath,
outputFiles.nonEmpty,
outputFiles == Seq(expectedFile1),
evalCount > 0
)

// don't recompile if nothing changed
val Right((_, unchangedEvalCount)) = eval.apply(HelloJava.core.semanticDbData)
assert(unchangedEvalCount == 0)
// don't recompile if nothing changed
val Right((_, unchangedEvalCount)) = eval.apply(HelloJava.core.semanticDbData)
assert(unchangedEvalCount == 0)
}
"incremental" - {
val eval = init()

// create a second source file
val secondFile = eval.evaluator.workspace / "core" / "src" / "hello" / "Second.java"
os.write(
secondFile,
"""package hello;
|
|public class Second {
| public static String msg() {
| return "Hello World";
| }
|}
|""".stripMargin,
createFolders = true
)
val thirdFile = eval.evaluator.workspace / "core" / "src" / "hello" / "Third.java"
os.write(
thirdFile,
"""package hello;
|
|public class Third {
| public static String msg() {
| return "Hello World";
| }
|}
|""".stripMargin,
createFolders = true
)
val Right((result, evalCount)) = eval.apply(HelloJava.core.semanticDbData)

val dataPath = eval.outPath / "core" / "semanticDbData.dest" / "data"
val outputFiles = os.walk(result.path).filter(os.isFile).map(_.relativeTo(result.path))

val expectedFile2 =
os.rel / "META-INF" / "semanticdb" / "core" / "src" / "hello" / "Second.java.semanticdb"
val expectedFile3 =
os.rel / "META-INF" / "semanticdb" / "core" / "src" / "hello" / "Third.java.semanticdb"
assert(
result.path == dataPath,
outputFiles.nonEmpty,
outputFiles.toSet == Set(expectedFile1, expectedFile2, expectedFile3),
evalCount > 0
)

// delete one, keep one, change one
os.remove(secondFile)
os.write.append(thirdFile, " ")

val Right((result2, changedEvalCount)) = eval.apply(HelloJava.core.semanticDbData)
val files2 = os.walk(result2.path).filter(os.isFile).map(_.relativeTo(result2.path))
assert(
files2.toSet == Set(expectedFile1, expectedFile3),
changedEvalCount > 0
)
}
}
"docJar" - {
"withoutArgsFile" - {
Expand Down
116 changes: 96 additions & 20 deletions scalalib/test/src/mill/scalalib/HelloWorldTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import mill._
import mill.api.Result
import mill.define.NamedTask
import mill.eval.{Evaluator, EvaluatorPaths}
import mill.scalalib.Assembly
import mill.scalalib.publish.{VersionControl, _}
import mill.util.{TestEvaluator, TestUtil}
import utest._
Expand Down Expand Up @@ -38,14 +37,19 @@ object HelloWorldTests extends TestSuite {
"4.1.9"
}
}

trait SemanticModule extends scalalib.ScalaModule {
def scalaVersion = scala213Version
}
trait HelloWorldModuleWithMain extends HelloWorldModule {
override def mainClass: T[Option[String]] = Some("Main")
}

object HelloWorld extends HelloBase {
object core extends HelloWorldModule
}
object SemanticWorld extends HelloBase {
object core extends SemanticModule
}
object HelloWorldNonPrecompiledBridge extends HelloBase {
object core extends HelloWorldModule {
override def scalaVersion = "2.12.1"
Expand Down Expand Up @@ -432,10 +436,6 @@ object HelloWorldTests extends TestSuite {
os.rel / "Person.class",
os.rel / "Person$.class"
)
def semanticDbFiles: Seq[os.SubPath] = Seq(
os.sub / "core" / "src" / "Main.scala.semanticdb",
os.sub / "core" / "src" / "Result.scala.semanticdb"
).map(os.sub / "META-INF" / "semanticdb" / _)

def workspaceTest[T](
m: TestUtil.BaseModule,
Expand Down Expand Up @@ -660,23 +660,99 @@ object HelloWorldTests extends TestSuite {
}

"semanticDbData" - {
"fromScratch" - workspaceTest(HelloWorld) { eval =>
val Right((result, evalCount)) = eval.apply(HelloWorld.core.semanticDbData)
def semanticDbFiles: Set[os.SubPath] = Set(
os.sub / "META-INF" / "semanticdb" / "core" / "src" / "Main.scala.semanticdb",
os.sub / "META-INF" / "semanticdb" / "core" / "src" / "Result.scala.semanticdb"
)

val outputFiles = os.walk(result.path).filter(os.isFile)
val dataPath = eval.outPath / "core" / "semanticDbData.dest" / "data"
"fromScratch" - workspaceTest(SemanticWorld, debug = true) { eval =>
{
println("first - expected full compile")
val Right((result, evalCount)) = eval.apply(SemanticWorld.core.semanticDbData)

val expectedSemFiles = semanticDbFiles.map(dataPath / _)
assert(
result.path == dataPath,
outputFiles.nonEmpty,
outputFiles.forall(expectedSemFiles.contains),
evalCount > 0
)
val dataPath = eval.outPath / "core" / "semanticDbData.dest" / "data"
val outputFiles = os.walk(result.path).filter(os.isFile).map(_.relativeTo(result.path))

// don't recompile if nothing changed
val Right((_, unchangedEvalCount)) = eval.apply(HelloWorld.core.semanticDbData)
assert(unchangedEvalCount == 0)
val expectedSemFiles = semanticDbFiles
assert(
result.path == dataPath,
outputFiles.nonEmpty,
outputFiles.toSet == expectedSemFiles,
evalCount > 0,
os.exists(dataPath / os.up / "zinc")
)
}
{
println("second - expected no compile")
// don't recompile if nothing changed
val Right((_, unchangedEvalCount)) = eval.apply(SemanticWorld.core.semanticDbData)
assert(unchangedEvalCount == 0)
}
}
"incremental" - workspaceTest(SemanticWorld, debug = true) { eval =>
// create some more source file to have a reasonable low incremental change later
val extraFiles = Seq("Second", "Third", "Fourth").map { f =>
val file = eval.evaluator.workspace / "core" / "src" / "hello" / s"${f}.scala"
os.write(
file,
s"""package hello
|class ${f}
|""".stripMargin,
createFolders = true
)
val sem =
os.sub / "META-INF" / "semanticdb" / "core" / "src" / "hello" / s"${f}.scala.semanticdb"
(file, sem)
}
// val resultFile = eval.evaluator.workspace / "core" / "src" / "Result.scala"

{
println("first - expected full compile")
val Right((result, evalCount)) = eval.apply(SemanticWorld.core.semanticDbData)

val dataPath = eval.outPath / "core" / "semanticDbData.dest" / "data"
val outputFiles = os.walk(result.path).filter(os.isFile).map(_.relativeTo(result.path))

val expectedSemFiles = semanticDbFiles ++ extraFiles.map(_._2)
assert(
result.path == dataPath,
outputFiles.toSet == expectedSemFiles,
evalCount > 0
)
}
// change nothing
{
println("second - expect no compile due to Mill caching")
val Right((_, evalCount)) = eval.apply(SemanticWorld.core.semanticDbData)
assert(evalCount == 0)
}

// change one
{
println("third - expect inc compile of one file\n")
os.write.append(extraFiles.head._1, " ")

val Right((result, evalCount)) = eval.apply(SemanticWorld.core.semanticDbData)
val outputFiles = os.walk(result.path).filter(os.isFile).map(_.relativeTo(result.path))
val expectedFiles = semanticDbFiles ++ extraFiles.map(_._2)
assert(
outputFiles.toSet == expectedFiles,
evalCount > 0
)
}
// remove one
{
println("fourth - expect inc compile with one deleted file")
os.remove(extraFiles.head._1)

val Right((result, evalCount)) = eval.apply(SemanticWorld.core.semanticDbData)
val outputFiles = os.walk(result.path).filter(os.isFile).map(_.relativeTo(result.path))
val expectedFiles = semanticDbFiles ++ extraFiles.map(_._2).drop(1)
assert(
outputFiles.toSet == expectedFiles,
evalCount > 0
)
}
}
}

Expand Down
8 changes: 4 additions & 4 deletions scalalib/worker/src/mill/scalalib/worker/ZincWorkerImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,8 @@ class ZincWorkerImpl(
reporter: Option[CompileProblemReporter],
reportCachedProblems: Boolean,
incrementalCompilation: Boolean,
auxiliaryClassFileExtensions: Seq[String]
auxiliaryClassFileExtensions: Seq[String],
zincFile: os.SubPath = os.sub / "zinc"
)(implicit ctx: ZincWorkerApi.Ctx): Result[CompilationResult] = {
os.makeDir.all(ctx.dest)

Expand Down Expand Up @@ -538,12 +539,11 @@ class ZincWorkerImpl(

val lookup = MockedLookup(analysisMap)

val zincFile = ctx.dest / "zinc"
val classesDir =
if (compileToJar) ctx.dest / "classes.jar"
else ctx.dest / "classes"

val store = FileAnalysisStore.binary(zincFile.toIO)
val store = FileAnalysisStore.binary((ctx.dest / zincFile).toIO)

// Fix jdk classes marked as binary dependencies, see https://github.com/com-lihaoyi/mill/pull/1904
val converter = MappedFileConverter.empty
Expand Down Expand Up @@ -616,7 +616,7 @@ class ZincWorkerImpl(
newResult.setup()
)
)
Result.Success(CompilationResult(zincFile, PathRef(classesDir)))
Result.Success(CompilationResult((ctx.dest / zincFile), PathRef(classesDir)))
} catch {
case e: CompileFailed =>
Result.Failure(e.toString)
Expand Down

0 comments on commit 654f585

Please sign in to comment.