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

2 files changed, 19 insertions, 92 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/InjectedStat.java b/src/main/java/com/google/devtools/build/lib/actions/cache/InjectedStat.java
deleted file mode 100644
index 7ed0e2e406..0000000000
--- a/src/main/java/com/google/devtools/build/lib/actions/cache/InjectedStat.java
+++ /dev/null
@@ -1,75 +0,0 @@
-// Copyright 2014 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-package com.google.devtools.build.lib.actions.cache;
-
-import com.google.devtools.build.lib.vfs.FileStatus;
-
-/**
- * A FileStatus corresponding to a file that is not determined by querying the file system.
- *
- * <p>Do not use this in combination with MetadataHandler or FileContentsProxy! FileContentsProxy
- * may use ctime, which this class does not support.
- */
-public class InjectedStat implements FileStatus {
-
-  private final long mtime;
-  private final long size;
-  private final long nodeId;
-
-  public InjectedStat(long mtime, long size, long nodeId) {
-    this.mtime = mtime;
-    this.size = size;
-    this.nodeId = nodeId;
-  }
-
-  @Override
-  public boolean isFile() {
-    return true;
-  }
-
-  @Override
-  public boolean isSpecialFile() {
-    return false;
-  }
-
-  @Override
-  public boolean isDirectory() {
-    return false;
-  }
-
-  @Override
-  public boolean isSymbolicLink() {
-    return false;
-  }
-
-  @Override
-  public long getSize() {
-    return size;
-  }
-
-  @Override
-  public long getLastModifiedTime() {
-    return mtime;
-  }
-
-  @Override
-  public long getLastChangeTime() {
-    return getLastModifiedTime();
-  }
-
-  @Override
-  public long getNodeId() {
-    return nodeId;
-  }
-}
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);
   }
 }