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.

Change-Id: I57fcb51b02e40adf3ebc4591c26bf5492c89352f
PiperOrigin-RevId: 175206157

9 files changed, 16 insertions, 32 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 1313ab6c6e..7392ed4e79 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,15 +330,6 @@ 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;
   }
@@ -361,14 +352,6 @@ 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() {
@@ -456,7 +439,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 : getUnconditionalArtifactsWithoutMiddlemen()) {
+    for (Artifact artifact : getUnconditionalArtifacts()) {
       checker.put(manifest, artifact.getRootRelativePath(), artifact);
     }
 
@@ -609,7 +592,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 : Iterables.filter(unconditionalArtifacts, Artifact.MIDDLEMAN_FILTER)) {
+    for (Artifact artifact : unconditionalArtifacts) {
       result.put(artifact.getRootRelativePath(), artifact);
     }
     return result;
@@ -729,6 +712,8 @@ 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 a86323496d..c609d3d787 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,7 +18,6 @@ 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;
@@ -73,7 +72,7 @@ public class RunfilesSupplierImpl implements RunfilesSupplier {
 
   @Override
   public Iterable<Artifact> getArtifacts() {
-    return Iterables.filter(runfiles.getAllArtifacts(), Artifact.MIDDLEMAN_FILTER);
+    return runfiles.getAllArtifacts();
   }
 
   @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 75f0a1d9c0..a58a020ee3 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> getRunfilesArtifactsWithoutMiddlemen() {
-    return runfiles.getArtifactsWithoutMiddlemen();
+  public Iterable<Artifact> getRunfilesArtifacts() {
+    return runfiles.getArtifacts();
   }
 
   /**
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 0fe1ba736e..864f2cfe81 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.getArtifactsWithoutMiddlemen()) {
+    for (Artifact artifact : runfiles.getArtifacts()) {
       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 52d839c4d9..8b0cfaf5d3 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.getRunfilesArtifactsWithoutMiddlemen()) {
+    for (Artifact artifact : runfilesSupport.getRunfilesArtifacts()) {
       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 676a9ee6df..9996d47e5d 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(filesToBuild)
+            .addTransitiveArtifacts(supportApks)
             .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 24ec6aec33..a74487fe61 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().getRunfilesArtifactsWithoutMiddlemen());
+    inputs.addAll(executable.getRunfilesSupport().getRunfilesArtifacts());
 
     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 0df89bf3a8..a4d43ed382 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()
-            .getRunfilesArtifactsWithoutMiddlemen());
+            .getRunfilesArtifacts());
 
     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 f6cf02e841..16505b92b8 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()
-            .getRunfilesArtifactsWithoutMiddlemen());
+            .getRunfilesArtifacts());
 
     List<Artifact> outs = new ArrayList<>();
     if (primary.getRTxt() != null) {