commit 406c087d7097f76f39e80827d331cc655f2ddd73
author Janak Ramakrishnan <janakr@google.com> Thu Jul 28 21:32:01 2016 +0000
committer Damien Martin-Guillerez <dmarting@google.com> Fri Jul 29 10:11:35 2016 +0000
tree a29bb9fc93d2972d29d868caece2b2a92b70a258
parent 84e3092e3d3e14a8801452d3aaefeb5fa594e307

Fix leftover "size" check when deciding whether to use digest or mtime. Since we no longer stored mtime for empty files, this bug meant that we always compared empty files equal (which is good). But we shouldn't be using Metadata based on mtime for them.

A follow-up change will do a refactor to make this impossible.

--
MOS_MIGRATED_REVID=128742054

src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
index f6a0224..0b10ed4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
@@ -36,7 +36,6 @@
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.RootedPath;
 import com.google.devtools.build.lib.vfs.Symlinks;
-
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.util.Arrays;
@@ -46,7 +45,6 @@
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
-
 import javax.annotation.Nullable;
 
 /**
@@ -137,14 +135,10 @@
         || value == FileArtifactValue.OMITTED_FILE_MARKER) {
       throw new FileNotFoundException();
     }
-    // If the file is empty or a directory, we need to return the mtime because the action cache
-    // uses mtime to determine if this artifact has changed.  We do not optimize for this code
-    // path (by storing the mtime somewhere) because we eventually may be switching to use digests
-    // for empty files. We want this code path to go away somehow too for directories (maybe by
-    // implementing FileSet in Skyframe).
-    return value.getSize() > 0
-        ? new Metadata(value.getDigest())
-        : new Metadata(value.getModifiedTime());
+    // If the file is a directory, we need to return the mtime because the action cache uses mtime
+    // to determine if this artifact has changed. We want this code path to go away somehow
+    // for directories (maybe by implementing FileSet in Skyframe).
+    return value.isFile() ? new Metadata(value.getDigest()) : new Metadata(value.getModifiedTime());
   }
 
   @Override