Fix Fileset incrementality bug when Fileset consumes a generated file. The native skyframe implementation was actually quite incorrect in this case: It was adding a skyframe dependency on a FileValue for the output file. Without a transitive dependency on the source files and actions that determine the output file's state, this could never work (and explains why the incremental build would fail). Instead, we now depend on the Artifact corresponding to the output file instead.

This change updates the business logic RecursiveFilesystemTraversalFunction. This approach keeps the business logic of Fileset filesystem traversal centralized in RFTF.

To avoid making weird recursive Skyframe nodes in the output tree, we inline Skyframe dependencies and do direct filesystem operations over the output tree.

There are now three states we can be in when looking up a file:
1. Source file: As before, make a skyframe dep on the corresponding file
2. Top-level file of an output tree: Make a dep on the corresponding Artifact
3. Recursive file under an output directory: Do direct filesystem operations. It doesn't make sense to make Skyframe nodes corresponding to these files. In the future, I think we should consider failing fast on this case.

RELNOTES: None
PiperOrigin-RevId: 184556044

1 files changed, 17 insertions, 2 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java b/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java
index 75e0da2423..7569544b9d 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java
@@ -23,6 +23,7 @@ import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.Root;
 import com.google.devtools.build.lib.vfs.RootedPath;
 import java.util.Set;
+import javax.annotation.Nullable;
 
 /**
  * Parameters of a filesystem traversal requested by a Fileset rule.
@@ -71,6 +72,13 @@ public interface FilesetTraversalParams {
   abstract class DirectTraversalRoot {
 
     /**
+     * Returns the output Artifact corresponding to this traversal, if present. Only present when
+     * traversing a generated output.
+     */
+    @Nullable
+    public abstract Artifact getOutputArtifact();
+
+    /**
      * Returns the root part of the full path.
      *
      * <p>This is typically the workspace root or some output tree's root (e.g. genfiles, binfiles).
@@ -94,15 +102,22 @@ public interface FilesetTraversalParams {
     @Override
     public abstract int hashCode();
 
-    static DirectTraversalRoot forPackage(Artifact buildFile) {
+    public static DirectTraversalRoot forPackage(Artifact buildFile) {
       return new AutoValue_FilesetTraversalParams_DirectTraversalRoot(
+          null,
           buildFile.getRoot().getRoot(), buildFile.getRootRelativePath().getParentDirectory());
     }
 
-    static DirectTraversalRoot forFileOrDirectory(Artifact fileOrDirectory) {
+    public static DirectTraversalRoot forFileOrDirectory(Artifact fileOrDirectory) {
       return new AutoValue_FilesetTraversalParams_DirectTraversalRoot(
+          fileOrDirectory.isSourceArtifact() ? null : fileOrDirectory,
           fileOrDirectory.getRoot().getRoot(), fileOrDirectory.getRootRelativePath());
     }
+
+    public static DirectTraversalRoot forRootedPath(RootedPath newPath) {
+      return new AutoValue_FilesetTraversalParams_DirectTraversalRoot(null,
+          newPath.getRoot(), newPath.getRootRelativePath());
+    }
   }
 
   /**