Introduce Path#isSpecialFile, FileSystem#isSpecialFile, and FileStatus#isSpecialFile to help disambiguate between a regular file and a special file, since the file size of a special file cannot be trusted.
--
MOS_MIGRATED_REVID=105903622
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 1a75e3b..3fffcd9 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
@@ -41,7 +41,7 @@
* <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.
- * file digest). See {@link FileFileStateValue}.
+ * file digest). See {@link RegularFileStateValue}.
* <ul>
*
* <p>This class is an implementation detail of {@link FileValue} and should not be used outside of
@@ -59,7 +59,8 @@
new NonexistentFileStateValue();
enum Type {
- FILE,
+ REGULAR_FILE,
+ SPECIAL_FILE,
DIRECTORY,
SYMLINK,
NONEXISTENT,
@@ -86,7 +87,9 @@
throws InconsistentFilesystemException, IOException {
Path path = rootedPath.asPath();
if (statNoFollow.isFile()) {
- return FileFileStateValue.fromPath(path, statNoFollow, tsgm);
+ return statNoFollow.isSpecialFile()
+ ? SpecialFileStateValue.fromStat(statNoFollow, tsgm)
+ : RegularFileStateValue.fromPath(path, statNoFollow, tsgm);
} else if (statNoFollow.isDirectory()) {
return DIRECTORY_FILE_STATE_NODE;
} else if (statNoFollow.isSymbolicLink()) {
@@ -125,7 +128,7 @@
abstract String prettyPrint();
/**
- * Implementation of {@link FileStateValue} for files that exist.
+ * Implementation of {@link FileStateValue} for regular files that exist.
*
* <p>A union of (digest, mtime). We use digests only if a fast digest lookup is available from
* the filesystem. If not, we fall back to mtime-based digests. This avoids the case where Blaze
@@ -133,7 +136,7 @@
* where fast digest lookups are not available.
*/
@ThreadSafe
- public static final class FileFileStateValue extends FileStateValue {
+ public static final class RegularFileStateValue extends FileStateValue {
private final long size;
// Only needed for empty-file equality-checking. Otherwise is always -1.
// TODO(bazel-team): Consider getting rid of this special case for empty files.
@@ -141,7 +144,7 @@
@Nullable private final byte[] digest;
@Nullable private final FileContentsProxy contentsProxy;
- private FileFileStateValue(long size, long mtime, byte[] digest,
+ private RegularFileStateValue(long size, long mtime, byte[] digest,
FileContentsProxy contentsProxy) {
Preconditions.checkState((digest == null) != (contentsProxy == null));
this.size = size;
@@ -156,7 +159,7 @@
* Create a FileFileStateValue instance corresponding to the given existing file.
* @param stat must be of type "File". (Not a symlink).
*/
- private static FileFileStateValue fromPath(Path path, FileStatusWithDigest stat,
+ private static RegularFileStateValue fromPath(Path path, FileStatusWithDigest stat,
@Nullable TimestampGranularityMonitor tsgm)
throws InconsistentFilesystemException {
Preconditions.checkState(stat.isFile(), path);
@@ -172,13 +175,13 @@
if (tsgm != null) {
tsgm.notifyDependenceOnFileTime(mtime);
}
- return new FileFileStateValue(stat.getSize(), stat.getLastModifiedTime(), null,
+ return new RegularFileStateValue(stat.getSize(), stat.getLastModifiedTime(), null,
FileContentsProxy.create(mtime, stat.getNodeId()));
} else {
// We are careful here to avoid putting the value ID into FileMetadata if we already have
// a digest. Arbitrary filesystems may do weird things with the value ID; a digest is more
// robust.
- return new FileFileStateValue(stat.getSize(), stat.getLastModifiedTime(), digest, null);
+ return new RegularFileStateValue(stat.getSize(), stat.getLastModifiedTime(), digest, null);
}
} catch (IOException e) {
String errorMessage = e.getMessage() != null
@@ -191,7 +194,7 @@
@Override
Type getType() {
- return Type.FILE;
+ return Type.REGULAR_FILE;
}
@Override
@@ -207,8 +210,8 @@
@Override
public boolean equals(Object obj) {
- if (obj instanceof FileFileStateValue) {
- FileFileStateValue other = (FileFileStateValue) 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);
}
@@ -230,6 +233,61 @@
}
}
+ /** Implementation of {@link FileStateValue} for special files that exist. */
+ public static final class SpecialFileStateValue extends FileStateValue {
+ private final FileContentsProxy contentsProxy;
+
+ private SpecialFileStateValue(FileContentsProxy contentsProxy) {
+ this.contentsProxy = contentsProxy;
+ }
+
+ static SpecialFileStateValue fromStat(FileStatusWithDigest stat,
+ @Nullable TimestampGranularityMonitor tsgm) throws IOException {
+ long mtime = stat.getLastModifiedTime();
+ // Note that TimestampGranularityMonitor#notifyDependenceOnFileTime is a thread-safe
+ // method.
+ if (tsgm != null) {
+ tsgm.notifyDependenceOnFileTime(mtime);
+ }
+ return new SpecialFileStateValue(FileContentsProxy.create(mtime, stat.getNodeId()));
+ }
+
+ @Override
+ Type getType() {
+ return Type.SPECIAL_FILE;
+ }
+
+ @Override
+ long getSize() {
+ return 0;
+ }
+
+ @Override
+ @Nullable
+ byte[] getDigest() {
+ return null;
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj instanceof SpecialFileStateValue) {
+ SpecialFileStateValue other = (SpecialFileStateValue) obj;
+ return Objects.equals(contentsProxy, other.contentsProxy);
+ }
+ return false;
+ }
+
+ @Override
+ public int hashCode() {
+ return contentsProxy.hashCode();
+ }
+
+ @Override
+ public String prettyPrint() {
+ return String.format("special file with %s", contentsProxy.prettyPrint());
+ }
+ }
+
/** Implementation of {@link FileStateValue} for directories that exist. */
public static final class DirectoryFileStateValue extends FileStateValue {
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 8d849e0..0c45cf1 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
@@ -59,11 +59,20 @@
}
/**
- * Returns true if this value corresponds to a file or symlink to an existing file. If so, its
- * parent directory is guaranteed to exist.
+ * Returns true if this value corresponds to a file or symlink to an existing regular or special
+ * file. If so, its parent directory is guaranteed to exist.
*/
public boolean isFile() {
- return realFileStateValue().getType() == Type.FILE;
+ return realFileStateValue().getType() == Type.REGULAR_FILE
+ || realFileStateValue().getType() == Type.SPECIAL_FILE;
+ }
+
+ /**
+ * Returns true if this value corresponds to a file or symlink to an existing special file. If so,
+ * its parent directory is guaranteed to exist.
+ */
+ public boolean isSpecialFile() {
+ return realFileStateValue().getType() == Type.SPECIAL_FILE;
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index d8e8230..aa92291 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -845,7 +845,7 @@
*
* <p>May return null if the computation has to be restarted.
*
- * <p>Exactly one of {@code replacementContents} and {@link buildFileValue} will be
+ * <p>Exactly one of {@code replacementContents} and {@code buildFileValue} will be
* non-{@code null}. The former indicates that we have a faux BUILD file with the given contents
* and the latter indicates that we have a legitimate BUILD file and should actually do
* preprocessing.
@@ -875,13 +875,15 @@
}
Preprocessor.Result preprocessingResult;
if (replacementContents == null) {
- long buildFileSize = Preconditions.checkNotNull(buildFileValue, packageId).getSize();
+ Preconditions.checkNotNull(buildFileValue, packageId);
// Even though we only open and read the file on a cache miss, note that the BUILD is
// still parsed two times. Also, the preprocessor may suboptimally open and read it
// again anyway.
ParserInputSource inputSource;
try {
- inputSource = ParserInputSource.create(buildFilePath, buildFileSize);
+ inputSource = buildFileValue.isSpecialFile()
+ ? ParserInputSource.create(buildFilePath)
+ : ParserInputSource.create(buildFilePath, buildFileValue.getSize());
} catch (IOException e) {
env.getListener().handle(Event.error(Location.fromFile(buildFilePath),
e.getMessage()));
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PerBuildSyscallCache.java b/src/main/java/com/google/devtools/build/lib/skyframe/PerBuildSyscallCache.java
index b1265b4..51df367 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PerBuildSyscallCache.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PerBuildSyscallCache.java
@@ -102,6 +102,11 @@
}
@Override
+ public boolean isSpecialFile() {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
public boolean isSymbolicLink() {
throw new UnsupportedOperationException();
}
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 cc80950..1b5dfd8 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
@@ -756,7 +756,9 @@
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.FILE))
+ 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));
}
