Move FileStateValue.Type to Metadata; add Metadata.getType()
This is in preparation for merging FileArtifactValue and FileStateValue.
Progress on #3360.
PiperOrigin-RevId: 179832948
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 2a8c6e5..4e087db 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
@@ -219,7 +219,7 @@
}
// We do not cache exceptions besides nonexistence here, because it is unlikely that the file
// will be requested from this cache too many times.
- fileValue = constructFileValue(artifact, null);
+ fileValue = constructFileValue(artifact, /*statNoFollow=*/ null);
return maybeStoreAdditionalData(artifact, fileValue, null);
}
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 7f45e7d..6bc9f5a 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
@@ -17,6 +17,7 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.FileStateType;
import com.google.devtools.build.lib.actions.cache.DigestUtils;
import com.google.devtools.build.lib.actions.cache.Metadata;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
@@ -45,6 +46,11 @@
@Immutable @ThreadSafe
public abstract class FileArtifactValue implements SkyValue, Metadata {
private static final class SingletonMarkerValue extends FileArtifactValue implements Singleton {
+ @Override
+ public FileStateType getType() {
+ return FileStateType.NONEXISTENT;
+ }
+
@Nullable
@Override
public byte[] getDigest() {
@@ -74,6 +80,11 @@
private static final class OmittedFileValue extends FileArtifactValue implements Singleton {
@Override
+ public FileStateType getType() {
+ return FileStateType.NONEXISTENT;
+ }
+
+ @Override
public byte[] getDigest() {
throw new UnsupportedOperationException();
}
@@ -117,6 +128,11 @@
this.mtime = mtime;
}
+ @Override
+ public FileStateType getType() {
+ return FileStateType.DIRECTORY;
+ }
+
@Nullable
@Override
public byte[] getDigest() {
@@ -154,6 +170,11 @@
}
@Override
+ public FileStateType getType() {
+ return FileStateType.REGULAR_FILE;
+ }
+
+ @Override
public byte[] getDigest() {
return digest;
}
@@ -255,10 +276,13 @@
return false;
}
Metadata m = (Metadata) o;
+ if (getType() != m.getType()) {
+ return false;
+ }
if (isFile()) {
- return m.isFile() && Arrays.equals(getDigest(), m.getDigest()) && getSize() == m.getSize();
+ return Arrays.equals(getDigest(), m.getDigest()) && getSize() == m.getSize();
} else {
- return !m.isFile() && getModifiedTime() == m.getModifiedTime();
+ return getModifiedTime() == m.getModifiedTime();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java
index acac707..965c4c5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java
@@ -18,8 +18,8 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
+import com.google.devtools.build.lib.actions.FileStateType;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
-import com.google.devtools.build.lib.skyframe.FileStateValue.Type;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -69,7 +69,7 @@
}
realRootedPath = resolvedState.getFirst();
realFileStateValue = resolvedState.getSecond();
- if (realFileStateValue.getType() == Type.NONEXISTENT) {
+ if (realFileStateValue.getType() == FileStateType.NONEXISTENT) {
return FileValue.value(
rootedPath,
FileStateValue.NONEXISTENT_FILE_STATE_NODE,
@@ -95,7 +95,7 @@
ArrayList<RootedPath> symlinkChain = new ArrayList<>();
TreeSet<Path> orderedSeenPaths = Sets.newTreeSet();
- while (realFileStateValue.getType().equals(FileStateValue.Type.SYMLINK)) {
+ while (realFileStateValue.getType().isSymlink()) {
symlinkChain.add(realRootedPath);
orderedSeenPaths.add(realRootedPath.asPath());
Pair<RootedPath, FileStateValue> resolvedState = getSymlinkTargetRootedPath(realRootedPath,
@@ -145,7 +145,7 @@
if (realFileStateValue == null) {
return null;
}
- if (realFileStateValue.getType() != FileStateValue.Type.NONEXISTENT
+ if (realFileStateValue.getType() != FileStateType.NONEXISTENT
&& parentFileValue != null && !parentFileValue.isDirectory()) {
String type = realFileStateValue.getType().toString().toLowerCase();
String message = type + " " + rootedPath.asPath() + " exists but its parent "
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 c309b42..2595663 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
@@ -14,7 +14,9 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
+import com.google.devtools.build.lib.actions.FileStateType;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.FileStatus;
@@ -33,10 +35,10 @@
import javax.annotation.Nullable;
/**
- * Encapsulates the filesystem operations needed to get state for a path. This is at least a
- * 'lstat' to determine what type of file the path is.
+ * Encapsulates the filesystem operations needed to get state for a path. This is equivalent to an
+ * 'lstat' that does not follow symlinks to determine what type of file the path is.
* <ul>
- * <li> For a non-existent file, the non existence is noted.
+ * <li> For a non-existent file, the non-existence is noted.
* <li> For a symlink, the symlink target is noted.
* <li> For a directory, the existence is noted.
* <li> For a file, the existence is noted, along with metadata about the file (e.g.
@@ -58,15 +60,6 @@
public static final NonexistentFileStateValue NONEXISTENT_FILE_STATE_NODE =
new NonexistentFileStateValue();
- /** Type of a path. */
- public enum Type {
- REGULAR_FILE,
- SPECIAL_FILE,
- DIRECTORY,
- SYMLINK,
- NONEXISTENT,
- }
-
protected FileStateValue() {
}
@@ -106,8 +99,9 @@
return LegacySkyKey.create(SkyFunctions.FILE_STATE, rootedPath);
}
- public abstract Type getType();
+ public abstract FileStateType getType();
+ /** Returns the target of the symlink, or throws an exception if this is not a symlink. */
PathFragment getSymlinkTarget() {
throw new IllegalStateException();
}
@@ -205,8 +199,8 @@
}
@Override
- public Type getType() {
- return Type.REGULAR_FILE;
+ public FileStateType getType() {
+ return FileStateType.REGULAR_FILE;
}
@Override
@@ -230,12 +224,17 @@
@Override
public boolean equals(Object obj) {
- if (obj instanceof RegularFileStateValue) {
- RegularFileStateValue other = (RegularFileStateValue) obj;
- return size == other.size && mtime == other.mtime && Arrays.equals(digest, other.digest)
- && Objects.equals(contentsProxy, other.contentsProxy);
+ if (obj == this) {
+ return true;
}
- return false;
+ if (!(obj instanceof RegularFileStateValue)) {
+ return false;
+ }
+ RegularFileStateValue other = (RegularFileStateValue) obj;
+ return size == other.size
+ && mtime == other.mtime
+ && Arrays.equals(digest, other.digest)
+ && Objects.equals(contentsProxy, other.contentsProxy);
}
@Override
@@ -244,6 +243,15 @@
}
@Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("digest", digest)
+ .add("size", size)
+ .add("mtime", mtime)
+ .add("contentsProxy", contentsProxy).toString();
+ }
+
+ @Override
public String prettyPrint() {
String contents = digest != null
? String.format("digest of %s", Arrays.toString(digest))
@@ -261,11 +269,10 @@
this.contentsProxy = contentsProxy;
}
- static SpecialFileStateValue fromStat(PathFragment path, FileStatusWithDigest stat,
+ static SpecialFileStateValue fromStat(PathFragment path, FileStatus stat,
@Nullable TimestampGranularityMonitor tsgm) throws IOException {
long mtime = stat.getLastModifiedTime();
- // Note that TimestampGranularityMonitor#notifyDependenceOnFileTime is a thread-safe
- // method.
+ // Note that TimestampGranularityMonitor#notifyDependenceOnFileTime is a thread-safe method.
if (tsgm != null) {
tsgm.notifyDependenceOnFileTime(path, mtime);
}
@@ -273,8 +280,8 @@
}
@Override
- public Type getType() {
- return Type.SPECIAL_FILE;
+ public FileStateType getType() {
+ return FileStateType.SPECIAL_FILE;
}
@Override
@@ -294,11 +301,14 @@
@Override
public boolean equals(Object obj) {
- if (obj instanceof SpecialFileStateValue) {
- SpecialFileStateValue other = (SpecialFileStateValue) obj;
- return Objects.equals(contentsProxy, other.contentsProxy);
+ if (obj == this) {
+ return true;
}
- return false;
+ if (!(obj instanceof SpecialFileStateValue)) {
+ return false;
+ }
+ SpecialFileStateValue other = (SpecialFileStateValue) obj;
+ return Objects.equals(contentsProxy, other.contentsProxy);
}
@Override
@@ -319,8 +329,8 @@
}
@Override
- public Type getType() {
- return Type.DIRECTORY;
+ public FileStateType getType() {
+ return FileStateType.DIRECTORY;
}
@Override
@@ -350,8 +360,8 @@
}
@Override
- public Type getType() {
- return Type.SYMLINK;
+ public FileStateType getType() {
+ return FileStateType.SYMLINK;
}
@Override
@@ -386,8 +396,8 @@
}
@Override
- public Type getType() {
- return Type.NONEXISTENT;
+ public FileStateType getType() {
+ return FileStateType.NONEXISTENT;
}
@Override
@@ -398,6 +408,9 @@
// This object is normally a singleton, but deserialization produces copies.
@Override
public boolean equals(Object obj) {
+ if (obj == this) {
+ return true;
+ }
return obj instanceof NonexistentFileStateValue;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileValue.java
index 441bf69..7374bf3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileValue.java
@@ -14,9 +14,9 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.base.Preconditions;
+import com.google.devtools.build.lib.actions.FileStateType;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
-import com.google.devtools.build.lib.skyframe.FileStateValue.Type;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.LegacySkyKey;
@@ -48,9 +48,10 @@
public abstract class FileValue implements SkyValue {
public boolean exists() {
- return realFileStateValue().getType() != Type.NONEXISTENT;
+ return realFileStateValue().getType() != FileStateType.NONEXISTENT;
}
+ /** Returns true if the original path is a symlink; the target path can never be a symlink. */
public boolean isSymlink() {
return false;
}
@@ -60,8 +61,8 @@
* file. If so, its parent directory is guaranteed to exist.
*/
public boolean isFile() {
- return realFileStateValue().getType() == Type.REGULAR_FILE
- || realFileStateValue().getType() == Type.SPECIAL_FILE;
+ return realFileStateValue().getType() == FileStateType.REGULAR_FILE
+ || realFileStateValue().getType() == FileStateType.SPECIAL_FILE;
}
/**
@@ -69,7 +70,7 @@
* its parent directory is guaranteed to exist.
*/
public boolean isSpecialFile() {
- return realFileStateValue().getType() == Type.SPECIAL_FILE;
+ return realFileStateValue().getType() == FileStateType.SPECIAL_FILE;
}
/**
@@ -77,7 +78,7 @@
* parent directory is guaranteed to exist.
*/
public boolean isDirectory() {
- return realFileStateValue().getType() == Type.DIRECTORY;
+ return realFileStateValue().getType() == FileStateType.DIRECTORY;
}
/**
@@ -125,12 +126,12 @@
static FileValue value(RootedPath rootedPath, FileStateValue fileStateValue,
RootedPath realRootedPath, FileStateValue realFileStateValue) {
if (rootedPath.equals(realRootedPath)) {
- Preconditions.checkState(fileStateValue.getType() != FileStateValue.Type.SYMLINK,
+ Preconditions.checkState(fileStateValue.getType() != FileStateType.SYMLINK,
"rootedPath: %s, fileStateValue: %s, realRootedPath: %s, realFileStateValue: %s",
rootedPath, fileStateValue, realRootedPath, realFileStateValue);
return new RegularFileValue(rootedPath, fileStateValue);
} else {
- if (fileStateValue.getType() == FileStateValue.Type.SYMLINK) {
+ if (fileStateValue.getType() == FileStateType.SYMLINK) {
return new SymlinkFileValue(realRootedPath, realFileStateValue,
fileStateValue.getSymlinkTarget());
} else {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 3c450d3..5924481 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -50,6 +50,7 @@
import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.actions.Executor;
+import com.google.devtools.build.lib.actions.FileStateType;
import com.google.devtools.build.lib.actions.ResourceManager;
import com.google.devtools.build.lib.actions.Root;
import com.google.devtools.build.lib.analysis.AspectCollection;
@@ -911,12 +912,11 @@
protected abstract void invalidate(Predicate<SkyKey> pred);
- private static boolean compatibleFileTypes(Dirent.Type oldType, FileStateValue.Type newType) {
- return (oldType.equals(Dirent.Type.FILE) && newType.equals(FileStateValue.Type.REGULAR_FILE))
- || (oldType.equals(Dirent.Type.UNKNOWN)
- && newType.equals(FileStateValue.Type.SPECIAL_FILE))
- || (oldType.equals(Dirent.Type.DIRECTORY) && newType.equals(FileStateValue.Type.DIRECTORY))
- || (oldType.equals(Dirent.Type.SYMLINK) && newType.equals(FileStateValue.Type.SYMLINK));
+ private static boolean compatibleFileTypes(Dirent.Type oldType, FileStateType newType) {
+ return (oldType.equals(Dirent.Type.FILE) && newType.equals(FileStateType.REGULAR_FILE))
+ || (oldType.equals(Dirent.Type.UNKNOWN) && newType.equals(FileStateType.SPECIAL_FILE))
+ || (oldType.equals(Dirent.Type.DIRECTORY) && newType.equals(FileStateType.DIRECTORY))
+ || (oldType.equals(Dirent.Type.SYMLINK) && newType.equals(FileStateType.SYMLINK));
}
protected Differencer.Diff getDiff(TimestampGranularityMonitor tsgm,
