Fix bug in SkyQuery's 'rbuildfiles' implementation where we were incorrectly using FileValue, rather than FileStateValue, nodes as the roots of the rdep traversal.

This is incorrect e.g. when a .bzl file is a target of a symlink that is load'd from a BUILD file.

RELNOTES: None
PiperOrigin-RevId: 177018264

3 files changed, 16 insertions, 15 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/query2/ParallelSkyQueryUtils.java b/src/main/java/com/google/devtools/build/lib/query2/ParallelSkyQueryUtils.java
index 1948cb0c53..57485d9147 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/ParallelSkyQueryUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/ParallelSkyQueryUtils.java
@@ -87,7 +87,7 @@ class ParallelSkyQueryUtils {
     Uniquifier<SkyKey> keyUniquifier = env.createSkyKeyUniquifier();
     RBuildFilesVisitor visitor =
         new RBuildFilesVisitor(env, keyUniquifier, callback);
-    visitor.visitAndWaitForCompletion(env.getSkyKeysForFileFragments(fileIdentifiers));
+    visitor.visitAndWaitForCompletion(env.getFileStateKeysForFileFragments(fileIdentifiers));
   }
 
   /** A helper class that computes 'rbuildfiles(<blah>)' via BFS. */
diff --git a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
index af7f275430..35c412ffc9 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
@@ -88,7 +88,7 @@ import com.google.devtools.build.lib.query2.engine.Uniquifier;
 import com.google.devtools.build.lib.query2.engine.VariableContext;
 import com.google.devtools.build.lib.skyframe.BlacklistedPackagePrefixesValue;
 import com.google.devtools.build.lib.skyframe.ContainingPackageLookupFunction;
-import com.google.devtools.build.lib.skyframe.FileValue;
+import com.google.devtools.build.lib.skyframe.FileStateValue;
 import com.google.devtools.build.lib.skyframe.GraphBackedRecursivePackageProvider;
 import com.google.devtools.build.lib.skyframe.PackageLookupValue;
 import com.google.devtools.build.lib.skyframe.PackageValue;
@@ -965,10 +965,10 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
 
   /**
    * Returns package lookup keys for looking up the package root for which there may be a relevant
-   * (from the perspective of {@link #getRBuildFiles}) {@link FileValue} node in the graph for
+   * (from the perspective of {@link #getRBuildFiles}) {@link FileStateValue} node in the graph for
    * {@code originalFileFragment}, which is assumed to be a file path.
    *
-   * <p>This is a helper function for {@link #getSkyKeysForFileFragments}.
+   * <p>This is a helper function for {@link #getFileStateKeysForFileFragments}.
    */
   private static Iterable<SkyKey> getPkgLookupKeysForFile(PathFragment originalFileFragment,
       PathFragment currentPathFragment) {
@@ -989,18 +989,18 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
   }
 
   /**
-   * Returns FileValue keys for which there may be relevant (from the perspective of {@link
-   * #getRBuildFiles}) FileValues in the graph corresponding to the given {@code pathFragments},
-   * which are assumed to be file paths.
+   * Returns FileStateValue keys for which there may be relevant (from the perspective of {@link
+   * #getRBuildFiles}) FileStateValues in the graph corresponding to the given
+   * {@code pathFragments}, which are assumed to be file paths.
    *
    * <p>To do this, we emulate the {@link ContainingPackageLookupFunction} logic: for each given
    * file path, we look for the nearest ancestor directory (starting with its parent directory), if
    * any, that has a package. The {@link PackageLookupValue} for this package tells us the package
-   * root that we should use for the {@link RootedPath} for the {@link FileValue} key.
+   * root that we should use for the {@link RootedPath} for the {@link FileStateValue} key.
    *
    * <p>Note that there may not be nodes in the graph corresponding to the returned SkyKeys.
    */
-  Collection<SkyKey> getSkyKeysForFileFragments(Iterable<PathFragment> pathFragments)
+  Collection<SkyKey> getFileStateKeysForFileFragments(Iterable<PathFragment> pathFragments)
       throws InterruptedException {
     Set<SkyKey> result = new HashSet<>();
     Multimap<PathFragment, PathFragment> currentToOriginal = ArrayListMultimap.create();
@@ -1028,8 +1028,8 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
               packageLookupKeysToOriginal.get(packageLookupKey);
           Preconditions.checkState(!originalFiles.isEmpty(), entry);
           for (PathFragment fileName : originalFiles) {
-            result.add(
-                FileValue.key(RootedPath.toRootedPath(packageLookupValue.getRoot(), fileName)));
+            result.add(FileStateValue.key(
+                RootedPath.toRootedPath(packageLookupValue.getRoot(), fileName)));
           }
           for (PathFragment current : packageLookupKeysToCurrent.get(packageLookupKey)) {
             currentToOriginal.removeAll(current);
@@ -1096,7 +1096,7 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
   QueryTaskFuture<Void> getRBuildFiles(
       Collection<PathFragment> fileIdentifiers, Callback<Target> callback) {
     try {
-      Collection<SkyKey> files = getSkyKeysForFileFragments(fileIdentifiers);
+      Collection<SkyKey> files = getFileStateKeysForFileFragments(fileIdentifiers);
       Uniquifier<SkyKey> keyUniquifier =
           new UniquifierImpl<>(SkyKeyKeyExtractor.INSTANCE, /*concurrencyLevel=*/ 1);
       Collection<SkyKey> current = keyUniquifier.unique(graph.getSuccessfulValues(files).keySet());
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 2cf3638c00..c309b427ad 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
@@ -43,9 +43,10 @@ import javax.annotation.Nullable;
  *        file digest). See {@link RegularFileStateValue}.
  * <ul>
  *
- * <p>This class is an implementation detail of {@link FileValue} and should not be used outside of
- * {@link FileFunction}. Instead, {@link FileValue} should be used by consumers that care about
- * files.
+ * <p>This class is an implementation detail of {@link FileValue} and should not be used by
+ * {@link com.google.devtools.build.skyframe.SkyFunction}s other than {@link FileFunction}. Instead,
+ * {@link FileValue} should be used by {@link com.google.devtools.build.skyframe.SkyFunction}
+ * consumers that care about files.
  *
  * <p>All subclasses must implement {@link #equals} and {@link #hashCode} properly.
  */