Adjust semantics of Metadata interface, remove isFile (use getType)

After some consideration, I think it makes sense to always allow a getDigest call, instead of specifying it as disallowed based on type. This is a follow-up CL for a previous CL introducing the getType method, which increased the complexity of the specification.

I have a follow-up CL, which is related, namely unknown commit. After that CL, Metadata instances for directories (but not Filesets) also have digests rather than using mtime, which is compatible with the documentation changes made here. Said CL is solving a correctness issue with directory dependencies, which I think we want, and using the digest in the Metadata is a natural way to get correct action cache lookups.

PiperOrigin-RevId: 181440548

2 files changed, 13 insertions, 15 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 b3adbd8800..d16d1bae89 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
@@ -320,7 +320,7 @@ public class DigestUtils {
 
     if (md == null) {
       // Move along, nothing to see here.
-    } else if (md.isFile()) {
+    } else if (md.getDigest() != null) {
       fp.addBytes(md.getDigest());
     } else {
       // Use the timestamp if the digest is not present, but not both. Modifying a timestamp while
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/Metadata.java b/src/main/java/com/google/devtools/build/lib/actions/cache/Metadata.java
index c14cb03a57..c79241be82 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/cache/Metadata.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/cache/Metadata.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.actions.cache;
 
 import com.google.devtools.build.lib.actions.FileStateType;
+import javax.annotation.Nullable;
 
 /**
  * An interface to represent the state of a file system object for the execution phase. This is not
@@ -33,23 +34,20 @@ public interface Metadata {
    * The type of the underlying file system object. If it is a regular file, then it is
    * guaranteed to have a digest. Otherwise it does not have a digest.
    */
-  default FileStateType getType() {
-    return isFile() ? FileStateType.REGULAR_FILE : FileStateType.DIRECTORY;
-  }
+  FileStateType getType();
 
   /**
-   * Whether the underlying file system object is a file or a symlink to a file, rather than a
-   * directory. All files are guaranteed to have a digest, and {@link #getDigest} must only be
-   * called on files.
-   */
-  boolean isFile();
-
-  /**
-   * Returns the file's digest; must only be called on objects for which {@link #isFile} returns
-   * true.
+   * Returns a digest of the content of the underlying file system object; must always return a
+   * non-null value for instances of type {@link FileStateType#REGULAR_FILE}. Otherwise may return
+   * null.
+   *
+   * <p>All instances of this interface must either have a digest or return a last-modified time.
+   * Clients should prefer using the digest for content identification (e.g., for caching), and only
+   * fall back to the last-modified time if no digest is available.
    *
    * <p>The return value is owned by this object and must not be modified.
    */
+  @Nullable
   byte[] getDigest();
 
   /** Returns the file's size, or 0 if the underlying file system object is not a file. */
@@ -57,8 +55,8 @@ public interface Metadata {
   long getSize();
 
   /**
-   * Returns the last modified time; must only be called on objects for which {@link #isFile}
-   * returns false.
+   * Returns the last modified time; see the documentation of {@link #getDigest} for when this can
+   * and should be called.
    */
   long getModifiedTime();
 }