Remove the runfiles middleman from py_binary's data runfiles.

This requires polluting Runfiles and RuleConfiguredTargetBuilder with
some custom infrastructure to lift a py_binary runfiles middleman into
the FilesToRunProviders and output groups of its reverse
dependency. That can all go away when the
--experimental_build_transitive_python_runfiles transition is
complete.

This is another step towards https://bazel-review.googlesource.com/c/bazel/+/14010.

Change-Id: Ib750d72d4be42324c8edec485707480690b9fc9c
PiperOrigin-RevId: 173514090

2 files changed, 52 insertions, 14 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
index 44ab47c341..a67e9d0f3b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -88,16 +88,21 @@ public final class RuleConfiguredTargetBuilder {
       return null;
     }
 
+    NestedSetBuilder<Artifact> runfilesMiddlemenBuilder = NestedSetBuilder.stableOrder();
+    if (runfilesSupport != null) {
+      runfilesMiddlemenBuilder.add(runfilesSupport.getRunfilesMiddleman());
+      runfilesMiddlemenBuilder.addTransitive(runfilesSupport.getRunfiles().getExtraMiddlemen());
+    }
+    NestedSet<Artifact> runfilesMiddlemen = runfilesMiddlemenBuilder.build();
     FilesToRunProvider filesToRunProvider =
         new FilesToRunProvider(
-            buildFilesToRun(runfilesSupport, filesToBuild), runfilesSupport, executable);
+            buildFilesToRun(runfilesMiddlemen, filesToBuild), runfilesSupport, executable);
     addProvider(new FileProvider(filesToBuild));
     addProvider(filesToRunProvider);
 
     if (runfilesSupport != null) {
       // If a binary is built, build its runfiles, too
-      addOutputGroup(
-          OutputGroupProvider.HIDDEN_TOP_LEVEL, runfilesSupport.getRunfilesMiddleman());
+      addOutputGroup(OutputGroupProvider.HIDDEN_TOP_LEVEL, runfilesMiddlemen);
     } else if (providersBuilder.contains(RunfilesProvider.class)) {
       // If we don't have a RunfilesSupport (probably because this is not a binary rule), we still
       // want to build the files this rule contributes to runfiles of dependent rules so that we
@@ -142,15 +147,13 @@ public final class RuleConfiguredTargetBuilder {
 
   /**
    * Compute the artifacts to put into the {@link FilesToRunProvider} for this target. These are the
-   * filesToBuild, any artifacts added by the rule with {@link #addFilesToRun}, and the runfiles
-   * middleman if it exists.
+   * filesToBuild, any artifacts added by the rule with {@link #addFilesToRun}, and the runfiles'
+   * middlemen if they exists.
    */
   private NestedSet<Artifact> buildFilesToRun(
-      RunfilesSupport runfilesSupport, NestedSet<Artifact> filesToBuild) {
+      NestedSet<Artifact> runfilesMiddlemen, NestedSet<Artifact> filesToBuild) {
     filesToRunBuilder.addTransitive(filesToBuild);
-    if (runfilesSupport != null) {
-      filesToRunBuilder.add(runfilesSupport.getRunfilesMiddleman());
-    }
+    filesToRunBuilder.addTransitive(runfilesMiddlemen);
     return filesToRunBuilder.build();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
index 65009a3254..f75d29066b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
@@ -201,6 +201,12 @@ public final class Runfiles {
   private final NestedSet<SymlinkEntry> rootSymlinks;
 
   /**
+   * A set of middlemen artifacts. {@link RuleConfiguredTargetBuilder} adds these to the {@link
+   * FilesToRunProvider} of binaries that include this runfiles tree in their runfiles.
+   */
+  private final NestedSet<Artifact> extraMiddlemen;
+
+  /**
    * Interface used for adding empty files to the runfiles at the last minute. Mainly to support
    * python-related rules adding __init__.py files.
    */
@@ -294,6 +300,7 @@ public final class Runfiles {
       NestedSet<SymlinkEntry> symlinks,
       NestedSet<SymlinkEntry> rootSymlinks,
       NestedSet<PruningManifest> pruningManifests,
+      NestedSet<Artifact> extraMiddlemen,
       EmptyFilesSupplier emptyFilesSupplier,
       ConflictPolicy conflictPolicy,
       boolean legacyExternalRunfiles) {
@@ -301,6 +308,7 @@ public final class Runfiles {
     this.unconditionalArtifacts = Preconditions.checkNotNull(artifacts);
     this.symlinks = Preconditions.checkNotNull(symlinks);
     this.rootSymlinks = Preconditions.checkNotNull(rootSymlinks);
+    this.extraMiddlemen = Preconditions.checkNotNull(extraMiddlemen);
     this.pruningManifests = Preconditions.checkNotNull(pruningManifests);
     this.emptyFilesSupplier = Preconditions.checkNotNull(emptyFilesSupplier);
     this.conflictPolicy = conflictPolicy;
@@ -331,6 +339,10 @@ public final class Runfiles {
     return Iterables.filter(unconditionalArtifacts, Artifact.MIDDLEMAN_FILTER);
   }
 
+  public NestedSet<Artifact> getExtraMiddlemen() {
+    return extraMiddlemen;
+  }
+
   /**
    * Returns the collection of runfiles as artifacts, including both unconditional artifacts and
    * pruning manifest candidates.
@@ -641,8 +653,11 @@ public final class Runfiles {
    * Returns if there are no runfiles.
    */
   public boolean isEmpty() {
-    return unconditionalArtifacts.isEmpty() && symlinks.isEmpty() && rootSymlinks.isEmpty()
-        && pruningManifests.isEmpty();
+    return unconditionalArtifacts.isEmpty()
+        && symlinks.isEmpty()
+        && rootSymlinks.isEmpty()
+        && pruningManifests.isEmpty()
+        && extraMiddlemen.isEmpty();
   }
 
   /**
@@ -753,6 +768,7 @@ public final class Runfiles {
         NestedSetBuilder.stableOrder();
     private NestedSetBuilder<PruningManifest> pruningManifestsBuilder =
         NestedSetBuilder.stableOrder();
+    private NestedSetBuilder<Artifact> extraMiddlemenBuilder = NestedSetBuilder.stableOrder();
     private EmptyFilesSupplier emptyFilesSupplier = DUMMY_EMPTY_FILES_SUPPLIER;
 
     /** Build the Runfiles object with this policy */
@@ -804,9 +820,16 @@ public final class Runfiles {
      * Builds a new Runfiles object.
      */
     public Runfiles build() {
-      return new Runfiles(suffix, artifactsBuilder.build(), symlinksBuilder.build(),
-          rootSymlinksBuilder.build(), pruningManifestsBuilder.build(),
-          emptyFilesSupplier, conflictPolicy, legacyExternalRunfiles);
+      return new Runfiles(
+          suffix,
+          artifactsBuilder.build(),
+          symlinksBuilder.build(),
+          rootSymlinksBuilder.build(),
+          pruningManifestsBuilder.build(),
+          extraMiddlemenBuilder.build(),
+          emptyFilesSupplier,
+          conflictPolicy,
+          legacyExternalRunfiles);
     }
 
     /**
@@ -1051,6 +1074,17 @@ public final class Runfiles {
     }
 
     /**
+     * Add extra middlemen artifacts that should be built by reverse dependency binaries. This
+     * method exists solely to support the unfortunate legacy behavior of some rules; new uses
+     * should not be added.
+     */
+    public Builder addLegacyExtraMiddleman(Artifact middleman) {
+      Preconditions.checkArgument(middleman.isMiddlemanArtifact(), middleman);
+      extraMiddlemenBuilder.add(middleman);
+      return this;
+    }
+
+    /**
      * Add the other {@link Runfiles} object transitively.
      */
     public Builder merge(Runfiles runfiles) {
@@ -1097,6 +1131,7 @@ public final class Runfiles {
       if (includePruningManifests) {
         pruningManifestsBuilder.addTransitive(runfiles.getPruningManifests());
       }
+      extraMiddlemenBuilder.addTransitive(runfiles.getExtraMiddlemen());
       if (emptyFilesSupplier == DUMMY_EMPTY_FILES_SUPPLIER) {
         emptyFilesSupplier = runfiles.getEmptyFilesProvider();
       } else {