Remove mtime field from FileStateValue.

We stopped treating empty files specially in the execution phase after unknown commit. So this code is mostly if not entirely orphaned. The only scenario in which this would lead to a semantic difference that I can think of is if the user is running with --nouse_action_cache and a running Bazel server, and they have an action they want to re-run when an empty input file changes, which isn't something we need to be concerned about.

PiperOrigin-RevId: 182109952

3 files changed, 8 insertions, 35 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileStateValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileStateValue.java
index 2595663fd4..7a3453a243 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileStateValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileStateValue.java
@@ -133,19 +133,12 @@ public abstract class FileStateValue implements SkyValue {
   @ThreadSafe
   public static final class RegularFileStateValue extends FileStateValue {
     private final long size;
-    // Only needed for empty-file equality-checking. Otherwise is always -1.
-    // TODO(bazel-team): Consider getting rid of this special case for empty files.
-    private final long mtime;
     @Nullable private final byte[] digest;
     @Nullable private final FileContentsProxy contentsProxy;
 
-    public RegularFileStateValue(long size, long mtime, byte[] digest,
-        FileContentsProxy contentsProxy) {
+    public RegularFileStateValue(long size, byte[] digest, FileContentsProxy contentsProxy) {
       Preconditions.checkState((digest == null) != (contentsProxy == null));
       this.size = size;
-      // mtime is forced to be -1 so that we do not accidentally depend on it for non-empty files,
-      // which should only be compared using digests.
-      this.mtime = size == 0 ? mtime : -1;
       this.digest = digest;
       this.contentsProxy = contentsProxy;
     }
@@ -168,13 +161,12 @@ public abstract class FileStateValue implements SkyValue {
           if (tsgm != null) {
             tsgm.notifyDependenceOnFileTime(path.asFragment(), mtime);
           }
-          return new RegularFileStateValue(stat.getSize(), stat.getLastModifiedTime(), null,
-              FileContentsProxy.create(stat));
+          return new RegularFileStateValue(stat.getSize(), null, FileContentsProxy.create(stat));
         } else {
           // We are careful here to avoid putting the value ID into FileMetadata if we already have
           // a digest. Arbitrary filesystems may do weird things with the value ID; a digest is more
           // robust.
-          return new RegularFileStateValue(stat.getSize(), stat.getLastModifiedTime(), digest, null);
+          return new RegularFileStateValue(stat.getSize(), digest, null);
         }
       } catch (IOException e) {
         String errorMessage = e.getMessage() != null
@@ -208,10 +200,6 @@ public abstract class FileStateValue implements SkyValue {
       return size;
     }
 
-    public long getMtime() {
-      return mtime;
-    }
-
     @Override
     @Nullable
     public byte[] getDigest() {
@@ -232,14 +220,13 @@ public abstract class FileStateValue implements SkyValue {
       }
       RegularFileStateValue other = (RegularFileStateValue) obj;
       return size == other.size
-          && mtime == other.mtime
           && Arrays.equals(digest, other.digest)
           && Objects.equals(contentsProxy, other.contentsProxy);
     }
 
     @Override
     public int hashCode() {
-      return Objects.hash(size, mtime, Arrays.hashCode(digest), contentsProxy);
+      return Objects.hash(size, Arrays.hashCode(digest), contentsProxy);
     }
 
     @Override
@@ -247,7 +234,6 @@ public abstract class FileStateValue implements SkyValue {
       return MoreObjects.toStringHelper(this)
           .add("digest", digest)
           .add("size", size)
-          .add("mtime", mtime)
           .add("contentsProxy", contentsProxy).toString();
     }
 
@@ -256,8 +242,7 @@ public abstract class FileStateValue implements SkyValue {
       String contents = digest != null
           ? String.format("digest of %s", Arrays.toString(digest))
           : contentsProxy.prettyPrint();
-      String extra = mtime != -1 ? String.format(" and mtime of %d", mtime) : "";
-      return String.format("regular file with size of %d and %s%s", size, contents, extra);
+      return String.format("regular file with size of %d and %s", size, contents);
     }
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryFunctionTest.java b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryFunctionTest.java
index 722067011e..929531ed43 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryFunctionTest.java
@@ -125,12 +125,12 @@ public class RepositoryFunctionTest extends BuildViewTestCase {
     RootedPath path = RootedPath.toRootedPath(rootDirectory, scratch.file("foo", "bar"));
 
     // Digest should be returned if the FileStateValue has it.
-    FileStateValue fsv = new RegularFileStateValue(3, 100, new byte[] {1, 2, 3, 4}, null);
+    FileStateValue fsv = new RegularFileStateValue(3, new byte[] {1, 2, 3, 4}, null);
     FileValue fv = new RegularFileValue(path, fsv);
     assertThat(RepositoryFunction.fileValueToMarkerValue(fv)).isEqualTo("01020304");
 
     // Digest should also be returned if the FileStateValue doesn't have it.
-    fsv = new RegularFileStateValue(3, 100, null, new FileContentsProxy(100, 200));
+    fsv = new RegularFileStateValue(3, null, new FileContentsProxy(100, 200));
     fv = new RegularFileValue(path, fsv);
     String expectedDigest = BaseEncoding.base16().lowerCase().encode(path.asPath().getDigest());
     assertThat(RepositoryFunction.fileValueToMarkerValue(fv)).isEqualTo(expectedDigest);
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
index 052b59105a..0a0b460cd4 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
@@ -452,8 +452,6 @@ public class FileFunctionTest {
     p.setLastModifiedTime(0L);
     FileValue a = valueForPath(p);
     p.setLastModifiedTime(1L);
-    assertThat(valueForPath(p)).isNotEqualTo(a);
-    p.setLastModifiedTime(0L);
     assertThat(valueForPath(p)).isEqualTo(a);
     FileSystemUtils.writeContentAsLatin1(p, "content");
     // Same digest, but now non-empty.
@@ -471,7 +469,7 @@ public class FileFunctionTest {
     assertThat(value.getDigest()).isNull();
 
     p.setLastModifiedTime(10L);
-    assertThat(valueForPath(p)).isNotEqualTo(value);
+    assertThat(valueForPath(p)).isEqualTo(value);
 
     p.setLastModifiedTime(0L);
     assertThat(valueForPath(p)).isEqualTo(value);
@@ -499,16 +497,6 @@ public class FileFunctionTest {
   }
 
   @Test
-  public void testFileModificationModTime() throws Exception {
-    fastDigest = false;
-    Path p = file("file");
-    FileValue a = valueForPath(p);
-    p.setLastModifiedTime(42);
-    FileValue b = valueForPath(p);
-    assertThat(a.equals(b)).isFalse();
-  }
-
-  @Test
   public void testFileModificationDigest() throws Exception {
     fastDigest = true;
     Path p = file("file");