PathRef.sig is now independent of the base path

main/api/src/mill/api/PathRef.scala

@@ -3,44 +3,47 @@ package mill.api
 import java.nio.{file => jnio}
 import java.security.{DigestOutputStream, MessageDigest}
 import scala.util.Using
-
 import upickle.default.{ReadWriter => RW}
 
 /**
  * A wrapper around `os.Path` that calculates it's hashcode based
  * 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)
     }
   )
 }

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 =>

main/test/src/api/PathRefTests.scala

@@ -2,12 +2,13 @@ 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 {
-    "sig" - {
+    test("sig") {
       def check(quick: Boolean) = withTmpDir { tmpDir =>
         val file = tmpDir / "foo.txt"
         os.write.over(file, "hello")
@@ -18,32 +19,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 +76,31 @@ 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 prFile = pr.path.toString().replace("\\", "\\\\")
+        val json = upickle.default.write(pr)
+        if (quick) {
+          assert(json.startsWith(""""qref:"""))
+          assert(json.endsWith(s""":${prFile}""""))
+        } else {
+          val hash = if(Properties.isWin) "86df6a6a" else "4c7ef487"
+          val expected = s""""ref:${hash}:${prFile}""""
+          assert(json == expected)
+        }
+        val pr1 = upickle.default.read[PathRef](json)
+        assert(pr == pr1)
+      }
+
+      test("qref") - check(quick = true)
+      test("ref") - check(quick = false)
     }
   }
 

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}"
+      )
     }
 
   }