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

5 files changed, 95 insertions, 19 deletions

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 1a75e3b0de..3fffcd9c00 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 @@ import javax.annotation.Nullable;
  *   <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 @@ public abstract class FileStateValue implements SkyValue {
       new NonexistentFileStateValue();
 
   enum Type {
-    FILE,
+    REGULAR_FILE,
+    SPECIAL_FILE,
     DIRECTORY,
     SYMLINK,
     NONEXISTENT,
@@ -86,7 +87,9 @@ public abstract class FileStateValue implements SkyValue {
           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 @@ public abstract class FileStateValue implements SkyValue {
   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 @@ public abstract class FileStateValue implements SkyValue {
    * 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 @@ public abstract class FileStateValue implements SkyValue {
     @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 @@ public abstract class FileStateValue implements SkyValue {
      * 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 @@ public abstract class FileStateValue implements SkyValue {
           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 @@ public abstract class FileStateValue implements SkyValue {
 
     @Override
     Type getType() {
-      return Type.FILE;
+      return Type.REGULAR_FILE;
     }
 
     @Override
@@ -207,8 +210,8 @@ public abstract class FileStateValue implements SkyValue {
 
     @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 @@ public abstract class FileStateValue implements SkyValue {
     }
   }
 
+  /** 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 8d849e09fd..0c45cf1bcd 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 @@ public abstract class FileValue implements SkyValue {
   }
 
   /**
-   * 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 d8e8230f99..aa922913da 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 @@ public class PackageFunction implements SkyFunction {
    *
    * <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 @@ public class PackageFunction implements SkyFunction {
           }
           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 b1265b4c96..51df3671a4 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 @@ public class PerBuildSyscallCache implements UnixGlob.FilesystemCalls {
     }
 
     @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 cc809502d8..1b5dfd8468 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 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
   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));
   }