Automated rollback of commit 10b0d8aa6b73a024cc007c5e075cb329add878ef.

*** Reason for rollback ***

Breaks Google-internal targets, sadly.

*** Original change description ***

Ban middlemen from runfiles artifacts.

Previous changes have removed all middlemen from runfiles
artifacts. This CL locks it down and removes various now-redundant
*WithoutMiddlemen() methods from Runfiles.

I put a check for middlemen in ConflictChecker.put, which should be a
chokepoint for runfiles artifacts. It's unfortunate we can't detect
middlemen earlier than execution, but I can't see a way to efficiently
check every runfiles artifact earlier.

Cha...

***

RELNOTES: None.
PiperOrigin-RevId: 175650018

9 files changed, 32 insertions, 16 deletions

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 7392ed4e79..1313ab6c6e 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
@@ -330,6 +330,15 @@ public final class Runfiles {
     return unconditionalArtifacts;
   }
 
+  /**
+   * Returns the artifacts that are unconditionally included in the runfiles (as opposed to
+   * pruning manifest candidates, which may or may not be included). Middleman artifacts are
+   * excluded.
+   */
+  public Iterable<Artifact> getUnconditionalArtifactsWithoutMiddlemen() {
+    return Iterables.filter(unconditionalArtifacts, Artifact.MIDDLEMAN_FILTER);
+  }
+
   public NestedSet<Artifact> getExtraMiddlemen() {
     return extraMiddlemen;
   }
@@ -352,6 +361,14 @@ public final class Runfiles {
     return allArtifacts.build();
   }
 
+  /**
+   * Returns the collection of runfiles as artifacts, including both unconditional artifacts
+   * and pruning manifest candidates. Middleman artifacts are excluded.
+   */
+  public Iterable<Artifact> getArtifactsWithoutMiddlemen() {
+    return Iterables.filter(getArtifacts(), Artifact.MIDDLEMAN_FILTER);
+  }
+
   /** Returns the symlinks. */
   @SkylarkCallable(name = "symlinks", doc = "Returns the set of symlinks.", structField = true)
   public NestedSet<SymlinkEntry> getSymlinks() {
@@ -439,7 +456,7 @@ public final class Runfiles {
     ConflictChecker checker = new ConflictChecker(conflictPolicy, eventHandler, location);
     Map<PathFragment, Artifact> manifest = getSymlinksAsMap(checker);
     // Add unconditional artifacts (committed to inclusion on construction of runfiles).
-    for (Artifact artifact : getUnconditionalArtifacts()) {
+    for (Artifact artifact : getUnconditionalArtifactsWithoutMiddlemen()) {
       checker.put(manifest, artifact.getRootRelativePath(), artifact);
     }
 
@@ -592,7 +609,7 @@ public final class Runfiles {
     // If multiple artifacts have the same root-relative path, the last one in the list will win.
     // That is because the runfiles tree cannot contain the same artifact for different
     // configurations, because it only uses root-relative paths.
-    for (Artifact artifact : unconditionalArtifacts) {
+    for (Artifact artifact : Iterables.filter(unconditionalArtifacts, Artifact.MIDDLEMAN_FILTER)) {
       result.put(artifact.getRootRelativePath(), artifact);
     }
     return result;
@@ -712,8 +729,6 @@ public final class Runfiles {
      * @param artifact Artifact to store in map. This may be null to indicate an empty file.
      */
     public void put(Map<PathFragment, Artifact> map, PathFragment path, Artifact artifact) {
-      Preconditions.checkArgument(
-          artifact == null || !artifact.isMiddlemanArtifact(), "%s", artifact);
       if (policy != ConflictPolicy.IGNORE && map.containsKey(path)) {
         // Previous and new entry might have value of null
         Artifact previous = map.get(path);
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupplierImpl.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupplierImpl.java
index c609d3d787..a86323496d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupplierImpl.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupplierImpl.java
@@ -18,6 +18,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.BaseSpawn;
 import com.google.devtools.build.lib.actions.RunfilesSupplier;
@@ -72,7 +73,7 @@ public class RunfilesSupplierImpl implements RunfilesSupplier {
 
   @Override
   public Iterable<Artifact> getArtifacts() {
-    return runfiles.getAllArtifacts();
+    return Iterables.filter(runfiles.getAllArtifacts(), Artifact.MIDDLEMAN_FILTER);
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
index a58a020ee3..75f0a1d9c0 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
@@ -233,12 +233,12 @@ public final class RunfilesSupport {
   }
 
   /**
-   * Returns both runfiles artifacts and "conditional" artifacts that may be part of a Runfiles
-   * PruningManifest. This means the returned set may be an overapproximation of the actual set of
-   * runfiles (see {@link Runfiles.PruningManifest}).
+   * Returns both runfiles artifacts and "conditional" artifacts that may be part of a
+   * Runfiles PruningManifest. This means the returned set may be an overapproximation of the
+   * actual set of runfiles (see {@link Runfiles.PruningManifest}).
    */
-  public Iterable<Artifact> getRunfilesArtifacts() {
-    return runfiles.getArtifacts();
+  public Iterable<Artifact> getRunfilesArtifactsWithoutMiddlemen() {
+    return runfiles.getArtifactsWithoutMiddlemen();
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java b/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java
index 864f2cfe81..0fe1ba736e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java
@@ -210,7 +210,7 @@ public final class SourceManifestAction extends AbstractFileWriteAction {
       f.addPath(rootSymlink.getValue().getPath());
     }
 
-    for (Artifact artifact : runfiles.getArtifacts()) {
+    for (Artifact artifact : runfiles.getArtifactsWithoutMiddlemen()) {
       f.addPath(artifact.getRootRelativePath());
       f.addPath(artifact.getPath());
     }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
index 8b0cfaf5d3..52d839c4d9 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
@@ -276,7 +276,7 @@ public class BazelPythonSemantics implements PythonSemantics {
 
     // Read each runfile from execute path, add them into zip file at the right runfiles path.
     // Filter the executable file, cause we are building it.
-    for (Artifact artifact : runfilesSupport.getRunfilesArtifacts()) {
+    for (Artifact artifact : runfilesSupport.getRunfilesArtifactsWithoutMiddlemen()) {
       if (!artifact.equals(executable) && !artifact.equals(zipFile)) {
         argv.addDynamicString(
             getZipRunfilesPath(artifact.getRunfilesPath(), workspaceName)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidHostServiceFixture.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidHostServiceFixture.java
index 9996d47e5d..676a9ee6df 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidHostServiceFixture.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidHostServiceFixture.java
@@ -43,7 +43,7 @@ public class AndroidHostServiceFixture implements RuleConfiguredTargetFactory {
             .build();
     Runfiles runfiles =
         new Runfiles.Builder(ruleContext.getWorkspaceName())
-            .addTransitiveArtifacts(supportApks)
+            .addTransitiveArtifacts(filesToBuild)
             .merge(executable.getRunfilesSupport())
             .build();
     return ruleBuilder
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/LibraryRGeneratorActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/LibraryRGeneratorActionBuilder.java
index a74487fe61..24ec6aec33 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/LibraryRGeneratorActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/LibraryRGeneratorActionBuilder.java
@@ -63,7 +63,7 @@ public class LibraryRGeneratorActionBuilder {
     NestedSetBuilder<Artifact> inputs = NestedSetBuilder.naiveLinkOrder();
     FilesToRunProvider executable =
         ruleContext.getExecutablePrerequisite("$android_resources_busybox", Mode.HOST);
-    inputs.addAll(executable.getRunfilesSupport().getRunfilesArtifacts());
+    inputs.addAll(executable.getRunfilesSupport().getRunfilesArtifactsWithoutMiddlemen());
 
     builder.add("--tool").add("GENERATE_LIBRARY_R").add("--");
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ManifestMergerActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/ManifestMergerActionBuilder.java
index a4d43ed382..0df89bf3a8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ManifestMergerActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ManifestMergerActionBuilder.java
@@ -99,7 +99,7 @@ public class ManifestMergerActionBuilder {
         ruleContext
             .getExecutablePrerequisite("$android_resources_busybox", Mode.HOST)
             .getRunfilesSupport()
-            .getRunfilesArtifacts());
+            .getRunfilesArtifactsWithoutMiddlemen());
 
     builder.addExecPath("--manifest", manifest);
     inputs.add(manifest);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
index 16505b92b8..f6cf02e841 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
@@ -85,7 +85,7 @@ public class RClassGeneratorActionBuilder {
         ruleContext
             .getExecutablePrerequisite("$android_resources_busybox", Mode.HOST)
             .getRunfilesSupport()
-            .getRunfilesArtifacts());
+            .getRunfilesArtifactsWithoutMiddlemen());
 
     List<Artifact> outs = new ArrayList<>();
     if (primary.getRTxt() != null) {