Don't bother getting the not-real FileStateValue for a path whose parent doesn't exist.

In addition to saving a filesystem operation, this removes a source of a potential filesystem inconsistency.

--
MOS_MIGRATED_REVID=136355008

src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java

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 8404f53..617baad 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,6 +18,7 @@
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Sets;
 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.util.Preconditions;
 import com.google.devtools.build.lib.vfs.Path;
@@ -80,34 +81,36 @@
       }
       realRootedPath = resolvedState.getFirst();
       realFileStateValue = resolvedState.getSecond();
+      if (realFileStateValue.getType() == Type.NONEXISTENT) {
+        return FileValue.value(
+            rootedPath,
+            FileStateValue.NONEXISTENT_FILE_STATE_NODE,
+            realRootedPath,
+            realFileStateValue);
+      }
     }
 
-    FileStateValue fileStateValue = null;
-
-    try {
-      fileStateValue =
-          (FileStateValue)
-              env.getValueOrThrow(
-                  FileStateValue.key(rootedPath), FileOutsidePackageRootsException.class);
-    } catch (FileOutsidePackageRootsException e) {
-      throw new FileFunctionException(
-          new FileOutsidePackageRootsException(rootedPath), Transience.PERSISTENT);
+    FileStateValue fileStateValue;
+    if (rootedPath.equals(realRootedPath)) {
+      fileStateValue = Preconditions.checkNotNull(realFileStateValue, rootedPath);
+    } else {
+      try {
+        fileStateValue =
+            (FileStateValue)
+                env.getValueOrThrow(
+                    FileStateValue.key(rootedPath), FileOutsidePackageRootsException.class);
+      } catch (FileOutsidePackageRootsException e) {
+        throw new FileFunctionException(
+            new FileOutsidePackageRootsException(rootedPath), Transience.PERSISTENT);
+      }
+      if (fileStateValue == null) {
+        return null;
+      }
     }
 
-    if (fileStateValue == null) {
-      return null;
-    }
     if (realFileStateValue == null) {
       realRootedPath = rootedPath;
       realFileStateValue = fileStateValue;
-    } else if (rootedPath.equals(realRootedPath) && !fileStateValue.equals(realFileStateValue)) {
-      String message = String.format(
-          "Some filesystem operations implied %s was a %s but others made us think it was a %s",
-          rootedPath.asPath().getPathString(),
-          fileStateValue.prettyPrint(),
-          realFileStateValue.prettyPrint());
-      throw new FileFunctionException(new InconsistentFilesystemException(message),
-          Transience.TRANSIENT);
     }
 
     ArrayList<RootedPath> symlinkChain = new ArrayList<>();

src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
index 5257324..6308953 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
@@ -888,22 +888,13 @@
   }
 
   @Test
-  public void testFilesystemInconsistencies_ParentDoesntExistAndChildIsSymlink() throws Exception {
-    symlink("a/b", "doesntmatter");
-    // Our custom filesystem says "a/b" exists but "a" does not exist.
+  public void testDoesntStatChildIfParentDoesntExist() throws Exception {
+    // Our custom filesystem says "a" does not exist, so FileFunction shouldn't bother trying to
+    // think about "a/b". Test for this by having a stat of "a/b" fail with an io error, and
+    // observing that we don't encounter the error.
     fs.stubStat(path("a"), null);
-    SequentialBuildDriver driver = makeDriver();
-    SkyKey skyKey = skyKey("a/b");
-    EvaluationResult<FileValue> result =
-        driver.evaluate(
-            ImmutableList.of(skyKey), false, DEFAULT_THREAD_COUNT, NullEventHandler.INSTANCE);
-    assertTrue(result.hasError());
-    ErrorInfo errorInfo = result.getError(skyKey);
-    assertThat(errorInfo.getException()).isInstanceOf(InconsistentFilesystemException.class);
-    assertThat(errorInfo.getException().getMessage())
-        .contains(
-            "/root/a/b was a symlink to doesntmatter but others made us think it was a "
-                + "nonexistent path");
+    fs.stubStatError(path("a/b"), new IOException("ouch!"));
+    assertThat(valueForPath(path("a/b")).exists()).isFalse();
   }
 
   @Test
@@ -1639,8 +1630,9 @@
 
   private class CustomInMemoryFs extends InMemoryFileSystem {
 
-    private Map<Path, FileStatus> stubbedStats = Maps.newHashMap();
-    private Map<Path, IOException> stubbedFastDigestErrors = Maps.newHashMap();
+    private final Map<Path, FileStatus> stubbedStats = Maps.newHashMap();
+    private final Map<Path, IOException> stubbedStatErrors = Maps.newHashMap();
+    private final Map<Path, IOException> stubbedFastDigestErrors = Maps.newHashMap();
 
     public CustomInMemoryFs(ManualClock manualClock) {
       super(manualClock);
@@ -1667,8 +1659,15 @@
       stubbedStats.put(path, stubbedResult);
     }
 
+    public void stubStatError(Path path, IOException error) {
+      stubbedStatErrors.put(path, error);
+    }
+
     @Override
     public FileStatus stat(Path path, boolean followSymlinks) throws IOException {
+      if (stubbedStatErrors.containsKey(path)) {
+        throw stubbedStatErrors.get(path);
+      }
       if (stubbedStats.containsKey(path)) {
         return stubbedStats.get(path);
       }

src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
index 9175145..3c960df 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
@@ -252,7 +252,6 @@
     Path bazDir = bazFile.getParentDirectory();
     Path barDir = bazDir.getParentDirectory();
 
-    long bazFileNodeId = bazFile.stat().getNodeId();
     // Our custom filesystem says "foo/bar/baz" does not exist but it also says that "foo/bar"
     // has a child directory "baz".
     fs.stubStat(bazDir, null);
@@ -265,9 +264,7 @@
             new Dirent("baz", Dirent.Type.DIRECTORY))));
     differencer.inject(ImmutableMap.of(DirectoryListingValue.key(barDirRootedPath), barDirListing));
     SkyKey skyKey = PackageValue.key(PackageIdentifier.parse("@//foo"));
-    String expectedMessage = "Some filesystem operations implied /workspace/foo/bar/baz/baz.sh was "
-        + "a regular file with size of 0 and mtime of 0 and nodeId of " + bazFileNodeId + " and "
-        + "mtime of 0 but others made us think it was a nonexistent path";
+    String expectedMessage = "/workspace/foo/bar/baz is no longer an existing directory";
     EvaluationResult<PackageValue> result = SkyframeExecutorTestUtils.evaluate(
         getSkyframeExecutor(), skyKey, /*keepGoing=*/false, reporter);
     assertTrue(result.hasError());