Don't treat empty files specially with respect to mtime/digest.

RELNOTES: Bazel no longer regards an empty file as changed if its mtime has changed.

--
MOS_MIGRATED_REVID=127328552

3 files changed, 7 insertions, 30 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/DigestUtils.java b/src/main/java/com/google/devtools/build/lib/actions/cache/DigestUtils.java
index 377ea43fad..c61e943eda 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/cache/DigestUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/cache/DigestUtils.java
@@ -40,17 +40,6 @@ public class DigestUtils {
   private DigestUtils() {}
 
   /**
-   * Returns true iff using MD5 digests is appropriate for an artifact.
-   *
-   * @param isFile whether or not Artifact is a file versus a directory, isFile() on its stat.
-   * @param size size of Artifact on filesystem in bytes, getSize() on its stat.
-   */
-  public static boolean useFileDigest(boolean isFile, long size) {
-    // Use timestamps for directories and empty files. Use digests for everything else.
-    return isFile && size != 0;
-  }
-
-  /**
    * Obtain file's MD5 metadata using synchronized method, ensuring that system
    * is not overloaded in case when multiple threads are requesting MD5
    * calculations and underlying file system cannot provide it via extended
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 a72ef48557..f6a02249d9 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
@@ -24,7 +24,6 @@ import com.google.devtools.build.lib.actions.ActionInputHelper;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
 import com.google.devtools.build.lib.actions.cache.Digest;
-import com.google.devtools.build.lib.actions.cache.DigestUtils;
 import com.google.devtools.build.lib.actions.cache.Metadata;
 import com.google.devtools.build.lib.actions.cache.MetadataHandler;
 import com.google.devtools.build.lib.skyframe.TreeArtifactValue.TreeArtifactException;
@@ -253,10 +252,9 @@ public class ActionMetadataHandler implements MetadataHandler {
       throw new FileNotFoundException(artifact.prettyPrint() + " does not exist");
     }
     if (!artifact.hasParent()) {
-      // Artifacts may use either the "real" digest or the mtime, if the file is size 0.
+      // Artifacts may use either the "real" digest or the mtime, if the file is a directory.
       boolean isFile = data.isFile();
-      boolean useDigest = DigestUtils.useFileDigest(isFile, isFile ? data.getSize() : 0);
-      if (useDigest && data.getDigest() != null) {
+      if (isFile && data.getDigest() != null) {
         // We do not need to store the FileArtifactValue separately -- the digest is in the
         // file value and that is all that is needed for this file's metadata.
         return new Metadata(data.getDigest());
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileArtifactValue.java
index b55639eb94..5125764b63 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileArtifactValue.java
@@ -33,9 +33,6 @@ import javax.annotation.Nullable;
  * </li><li>
  *   a directory, in which case we would expect to see an mtime;
  * </li><li>
- *   an empty file corresponding to an Artifact, where we would expect to see a size (=0), mtime,
- *   and digest;
- * </li><li>
  *   an intentionally omitted file which the build system is aware of but doesn't actually exist,
  *   where all access methods are unsupported;
  * </li><li>
@@ -107,13 +104,10 @@ public class FileArtifactValue extends ArtifactValue {
     if (isFile && digest == null) {
       digest = DigestUtils.getDigestOrFail(artifact.getPath(), size);
     }
-    if (!DigestUtils.useFileDigest(isFile, size)) {
-      // In this case, we need to store the mtime because the action cache uses mtime to determine
-      // if this artifact has changed. This is currently true for empty files and directories. We
-      // do not optimize for this code path (by storing the mtime in a FileValue) because we do not
-      // like it and may remove this special-casing for empty files in the future. We want this code
-      // path to go away somehow too for directories (maybe by implementing FileSet
-      // in Skyframe)
+    if (!isFile) {
+      // In this case, we need to store the mtime because the action cache uses mtime for
+      // directories to determine if this artifact has changed. We want this code path to go away
+      // somehow (maybe by implementing FileSet in Skyframe).
       return new FileArtifactValue(digest, artifact.getPath().getLastModifiedTime(), size);
     }
     Preconditions.checkState(digest != null, artifact);
@@ -158,11 +152,7 @@ public class FileArtifactValue extends ArtifactValue {
     return size;
   }
 
-  /**
-   * Gets last modified time of file. Should only be called if {@link DigestUtils#useFileDigest} was
-   * false for this artifact -- namely, either it is a directory or an empty file. Note that since
-   * we store directory sizes as 0, all files for which this method can be called have size 0.
-   */
+  /** Gets last modified time of file. Should only be called if this is a directory. */
   long getModifiedTime() {
     Preconditions.checkState(size == 0, "%s %s %s", digest, mtime, size);
     return mtime;