Change FileContentsProxy to use ctime instead of mtime

This gives us better reliability for detecting file changes; especially in
cases where tools intentionally do not update mtime.

Fixes #1525.

PiperOrigin-RevId: 155490849

1 files changed, 19 insertions, 17 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileContentsProxy.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileContentsProxy.java
index 43e5f8de7d..d8905881be 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileContentsProxy.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileContentsProxy.java
@@ -20,38 +20,40 @@ import java.util.Objects;
 
 /**
  * In case we can't get a fast digest from the filesystem, we store this metadata as a proxy to
- * the file contents. Currently it is a pair of the mtime and "value id" (which is right now just
- * the inode number). We may wish to augment this object with the following data:
- * a. the device number
- * b. the ctime, which cannot be tampered with in userspace
+ * the file contents. Currently it is a pair of the mtime and "value id", for which we xor the
+ * inode (on Linux) and the ctime. We might want to add the device number in the future.
  *
  * <p>For an example of why mtime alone is insufficient, note that 'mv' preserves timestamps. So if
  * files 'a' and 'b' initially have the same timestamp, then we would think 'b' is unchanged after
  * the user executes `mv a b` between two builds.
  */
 public final class FileContentsProxy implements Serializable {
-  private final long mtime;
-  private final long valueId;
+  private final long ctime;
+  private final long nodeId;
 
   /**
    * Visible for serialization / deserialization. Do not use this method, but call {@link #create}
    * instead.
    */
-  public FileContentsProxy(long mtime, long valueId) {
-    this.mtime = mtime;
-    this.valueId = valueId;
+  public FileContentsProxy(long ctime, long nodeId) {
+    this.ctime = ctime;
+    this.nodeId = nodeId;
   }
 
   public static FileContentsProxy create(FileStatus stat) throws IOException {
-    return new FileContentsProxy(stat.getLastModifiedTime(), stat.getNodeId());
+    // Note: there are file systems that return mtime for this call instead of ctime, such as the
+    // WindowsFileSystem.
+    return new FileContentsProxy(stat.getLastChangeTime(), stat.getNodeId());
   }
 
-  public long getMtime() {
-    return mtime;
+  /** Visible for serialization / deserialization. Do not use this method; use {@link #equals}. */
+  public long getCTime() {
+    return ctime;
   }
 
-  public long getValueId() {
-    return valueId;
+  /** Visible for serialization / deserialization. Do not use this method; use {@link #equals}. */
+  public long getNodeId() {
+    return nodeId;
   }
 
   @Override
@@ -65,12 +67,12 @@ public final class FileContentsProxy implements Serializable {
     }
 
     FileContentsProxy that = (FileContentsProxy) other;
-    return mtime == that.mtime && valueId == that.valueId;
+    return ctime == that.ctime && nodeId == that.nodeId;
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(mtime, valueId);
+    return Objects.hash(ctime, nodeId);
   }
 
   @Override
@@ -79,7 +81,7 @@ public final class FileContentsProxy implements Serializable {
   }
 
   public String prettyPrint() {
-    return String.format("mtime of %d and valueId of %d", mtime, valueId);
+    return String.format("ctime of %d and nodeId of %d", ctime, nodeId);
   }
 }