From 60210bb2a27df1451cea25b5b9ee10518f3ad487 Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Tue, 1 Nov 2022 16:53:36 +0100 Subject: [PATCH 1/2] `PathRef.sig` is now independent of the base path --- main/api/src/mill/api/PathRef.scala | 44 ++++++------ main/core/src/mill/eval/Evaluator.scala | 10 ++- main/test/src/api/PathRefTests.scala | 78 ++++++++++++++++------ main/testkit/src/testkit/MillTestkit.scala | 5 +- 4 files changed, 92 insertions(+), 45 deletions(-) diff --git a/main/api/src/mill/api/PathRef.scala b/main/api/src/mill/api/PathRef.scala index 6d27ead9b0b..8d3828d7d45 100644 --- a/main/api/src/mill/api/PathRef.scala +++ b/main/api/src/mill/api/PathRef.scala @@ -3,7 +3,6 @@ package mill.api import java.nio.{file => jnio} import java.security.{DigestOutputStream, MessageDigest} import scala.util.Using - import upickle.default.{ReadWriter => RW} /** @@ -11,36 +10,40 @@ import upickle.default.{ReadWriter => RW} * on the contents of the filesystem underneath it. Used to ensure filesystem * changes can bust caches which are keyed off hashcodes. */ -case class PathRef(path: os.Path, quick: Boolean, sig: Int) { - override def hashCode(): Int = sig -} +case class PathRef(path: os.Path, quick: Boolean, sig: Int) object PathRef { /** * Create a [[PathRef]] by recursively digesting the content of a given `path`. + * * @param path The digested path. * @param quick If `true` the digest is only based to some file attributes (like mtime and size). * If `false` the digest is created of the files content. * @return */ def apply(path: os.Path, quick: Boolean = false): PathRef = { + val basePath = path + val sig = { val isPosix = path.wrapped.getFileSystem.supportedFileAttributeViews().contains("posix") val digest = MessageDigest.getInstance("MD5") val digestOut = new DigestOutputStream(DummyOutputStream, digest) + def updateWithInt(value: Int): Unit = { digest.update((value >>> 24).toByte) digest.update((value >>> 16).toByte) digest.update((value >>> 8).toByte) digest.update(value.toByte) } + if (os.exists(path)) { for ( (path, attrs) <- os.walk.attrs(path, includeTarget = true, followLinks = true).sortBy(_._1.toString) ) { - digest.update(path.toString.getBytes) + val sub = path.subRelativeTo(basePath) + digest.update(sub.toString().getBytes()) if (!attrs.isDir) { if (isPosix) { updateWithInt(os.perms(path, followLinks = false).value) @@ -70,8 +73,8 @@ object PathRef { } java.util.Arrays.hashCode(digest.digest()) - } + new PathRef(path, quick, sig) } @@ -80,22 +83,23 @@ object PathRef { */ implicit def jsonFormatter: RW[PathRef] = upickle.default.readwriter[String].bimap[PathRef]( p => { - (if (p.quick) "qref" else "ref") + ":" + - String.format("%08x", p.sig: Integer) + ":" + - p.path.toString() + val prefix = if (p.quick) "qref:" else "ref:" + val sig = String.format("%08x", p.sig: Integer) + prefix + sig + ":" + p.path.toString() }, s => { - val Array(prefix, hex, path) = s.split(":", 3) - PathRef( - os.Path(path), - prefix match { - case "qref" => true - case "ref" => false - }, - // Parsing to a long and casting to an int is the only way to make - // round-trip handling of negative numbers work =( - java.lang.Long.parseLong(hex, 16).toInt - ) + val Array(prefix, hex, pathString) = s.split(":", 3) + + val path = os.Path(pathString) + val quick = prefix match { + case "qref" => true + case "ref" => false + } + // Parsing to a long and casting to an int is the only way to make + // round-trip handling of negative numbers work =( + val sig = java.lang.Long.parseLong(hex, 16).toInt + + PathRef(path, quick, sig) } ) } diff --git a/main/core/src/mill/eval/Evaluator.scala b/main/core/src/mill/eval/Evaluator.scala index 3c88479fabd..2fd5bc5cbf9 100644 --- a/main/core/src/mill/eval/Evaluator.scala +++ b/main/core/src/mill/eval/Evaluator.scala @@ -412,15 +412,19 @@ class Evaluator private[Evaluator] ( destSegments(labelledNamedTask) ) - val cached = for { + val cached: Option[(Any, Int)] = for { cached <- try Some(upickle.default.read[Evaluator.Cached](paths.meta.toIO)) - catch { case NonFatal(_) => None } + catch { + case NonFatal(_) => None + } if cached.inputsHash == inputsHash reader <- labelledNamedTask.format parsed <- try Some(upickle.default.read(cached.value)(reader)) - catch { case NonFatal(_) => None } + catch { + case NonFatal(_) => None + } } yield (parsed, cached.valueHash) val previousWorker = labelledNamedTask.task.asWorker.flatMap { w => diff --git a/main/test/src/api/PathRefTests.scala b/main/test/src/api/PathRefTests.scala index d9741c31e1c..04363c568b0 100644 --- a/main/test/src/api/PathRefTests.scala +++ b/main/test/src/api/PathRefTests.scala @@ -7,7 +7,7 @@ import utest._ object PathRefTests extends TestSuite { val tests: Tests = Tests { - "sig" - { + test("sig") { def check(quick: Boolean) = withTmpDir { tmpDir => val file = tmpDir / "foo.txt" os.write.over(file, "hello") @@ -18,32 +18,47 @@ object PathRefTests extends TestSuite { val sig2 = PathRef(file, quick).sig assert(sig1 != sig2) } - check(quick = true) - check(quick = false) + test("qref") - check(quick = true) + test("ref") - check(quick = false) } - "perms" - { + test("same-sig-other-file") { def check(quick: Boolean) = withTmpDir { tmpDir => val file = tmpDir / "foo.txt" - val content = "hello" - os.write.over(file, content) - Files.setPosixFilePermissions(file.wrapped, PosixFilePermissions.fromString("rw-rw----")) - val rwSig = PathRef(file, quick).sig - val rwSigb = PathRef(file, quick).sig - assert(rwSig == rwSigb) + os.write.over(file, "hello") + val sig1 = PathRef(file, quick).sig + val file2 = tmpDir / "bar.txt" + os.copy(file, file2) + val sig1b = PathRef(file2, quick).sig + assert(sig1 == sig1b) + } +// test("qref") - check(quick = true) + test("ref") - check(quick = false) + } - Files.setPosixFilePermissions(file.wrapped, PosixFilePermissions.fromString("rwxrw----")) - val rwxSig = PathRef(file, quick).sig + test("perms") { + def check(quick: Boolean) = + if (isPosixFs()) withTmpDir { tmpDir => + val file = tmpDir / "foo.txt" + val content = "hello" + os.write.over(file, content) + Files.setPosixFilePermissions(file.wrapped, PosixFilePermissions.fromString("rw-rw----")) + val rwSig = PathRef(file, quick).sig + val rwSigb = PathRef(file, quick).sig + assert(rwSig == rwSigb) - assert(rwSig != rwxSig) - } - if (isPosixFs()) { - check(quick = true) - check(quick = false) - } + Files.setPosixFilePermissions(file.wrapped, PosixFilePermissions.fromString("rwxrw----")) + val rwxSig = PathRef(file, quick).sig + + assert(rwSig != rwxSig) + } + else "Test Skipped on non-POSIX host" + + test("qref") - check(quick = true) + test("ref") - check(quick = false) } - "symlinks" - { + test("symlinks") { def check(quick: Boolean) = withTmpDir { tmpDir => // invalid symlink os.symlink(tmpDir / "nolink", tmpDir / "nonexistant") @@ -60,8 +75,29 @@ object PathRefTests extends TestSuite { val sig2 = PathRef(tmpDir, quick).sig assert(sig1 == sig2) } - check(quick = true) - check(quick = false) + test("qref") - check(quick = true) + test("ref") - check(quick = false) + } + + test("json") { + def check(quick: Boolean) = withTmpDir { tmpDir => + val file = tmpDir / "foo.txt" + os.write(file, "hello") + val pr = PathRef(file, quick) + val json = upickle.default.write(pr) + if (quick) { + assert(json.startsWith(""""qref:""")) + assert(json.endsWith(s""":${file}"""")) + } else { + val expected = s""""ref:4c7ef487:${file}"""" + assert(json == expected) + } + val pr1 = upickle.default.read[PathRef](json) + assert(pr == pr1) + } + + test("qref") - check(quick = true) + test("ref") - check(quick = false) } } diff --git a/main/testkit/src/testkit/MillTestkit.scala b/main/testkit/src/testkit/MillTestkit.scala index 95b7e097353..e336ee0c7f8 100644 --- a/main/testkit/src/testkit/MillTestkit.scala +++ b/main/testkit/src/testkit/MillTestkit.scala @@ -151,7 +151,10 @@ trait MillTestKit { .flatMap(_.asTarget) .filter(module.millInternal.targets.contains) .filter(!_.isInstanceOf[Input[_]]) - assert(evaluated == expected) + assert( + evaluated == expected, + s"evaluated is not equal expected. evaluated=${evaluated}, expected=${expected}" + ) } } From d3dbf6c27abebbf3c29b41764127e9eb8fcfbaae Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Fri, 18 Nov 2022 20:12:16 +0100 Subject: [PATCH 2/2] Adapt expected test results for Windows --- main/test/src/api/PathRefTests.scala | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/main/test/src/api/PathRefTests.scala b/main/test/src/api/PathRefTests.scala index 04363c568b0..746f08b9d2f 100644 --- a/main/test/src/api/PathRefTests.scala +++ b/main/test/src/api/PathRefTests.scala @@ -2,9 +2,10 @@ package mill.api import java.nio.file.Files import java.nio.file.attribute.PosixFilePermissions - import utest._ +import scala.util.Properties + object PathRefTests extends TestSuite { val tests: Tests = Tests { test("sig") { @@ -84,12 +85,14 @@ object PathRefTests extends TestSuite { val file = tmpDir / "foo.txt" os.write(file, "hello") val pr = PathRef(file, quick) + val prFile = pr.path.toString().replace("\\", "\\\\") val json = upickle.default.write(pr) if (quick) { assert(json.startsWith(""""qref:""")) - assert(json.endsWith(s""":${file}"""")) + assert(json.endsWith(s""":${prFile}"""")) } else { - val expected = s""""ref:4c7ef487:${file}"""" + val hash = if(Properties.isWin) "86df6a6a" else "4c7ef487" + val expected = s""""ref:${hash}:${prFile}"""" assert(json == expected) } val pr1 = upickle.default.read[PathRef](json)