diff options
5 files changed, 17 insertions, 44 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java index b6957cfcca..f2adb52bfc 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java @@ -59,11 +59,6 @@ public class ActionCacheChecker { } @Override - public boolean isFile() { - return true; - } - - @Override public byte[] getDigest() { return EMPTY_DIGEST; } 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(); } diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java index f9abca0137..f1d34d0f24 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java @@ -92,7 +92,7 @@ public class SpawnInputExpander { PathFragment location = root.getRelative(mapping.getKey()); Artifact localArtifact = mapping.getValue(); if (localArtifact != null) { - if (strict && !actionFileCache.getMetadata(localArtifact).isFile()) { + if (strict && !actionFileCache.getMetadata(localArtifact).getType().isFile()) { throw new IOException("Not a file: " + localArtifact.getPath().getPathString()); } addMapping(inputMap, location, localArtifact); 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 5a7ed29dc8..06b2aefcc9 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 @@ -58,11 +58,6 @@ public abstract class FileArtifactValue implements SkyValue, Metadata { } @Override - public boolean isFile() { - return false; - } - - @Override public long getSize() { return 0; } @@ -90,11 +85,6 @@ public abstract class FileArtifactValue implements SkyValue, Metadata { } @Override - public boolean isFile() { - throw new UnsupportedOperationException(); - } - - @Override public long getSize() { throw new UnsupportedOperationException(); } @@ -150,11 +140,6 @@ public abstract class FileArtifactValue implements SkyValue, Metadata { } @Override - public boolean isFile() { - return false; - } - - @Override public String toString() { return MoreObjects.toStringHelper(this).add("mtime", mtime).toString(); } @@ -180,11 +165,6 @@ public abstract class FileArtifactValue implements SkyValue, Metadata { } @Override - public boolean isFile() { - return true; - } - - @Override public long getSize() { return size; } @@ -262,7 +242,7 @@ public abstract class FileArtifactValue implements SkyValue, Metadata { } @Override - public abstract boolean isFile(); + public abstract FileStateType getType(); @Nullable @Override @@ -289,7 +269,7 @@ public abstract class FileArtifactValue implements SkyValue, Metadata { if (getType() != m.getType()) { return false; } - if (isFile()) { + if (getDigest() != null) { return Arrays.equals(getDigest(), m.getDigest()) && getSize() == m.getSize(); } else { return getModifiedTime() == m.getModifiedTime(); @@ -302,7 +282,7 @@ public abstract class FileArtifactValue implements SkyValue, Metadata { return System.identityHashCode(this); } // Hash digest by content, not reference. - if (isFile()) { + if (getDigest() != null) { return 37 * Long.hashCode(getSize()) + Arrays.hashCode(getDigest()); } else { return Long.hashCode(getModifiedTime()); |