Automated rollback of commit 4260c30a03a9b83d48a5e8690aca19cd80be4c38.
*** Reason for rollback ***
Try again with fixes.
*** Original change description ***
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 arti...
***
PiperOrigin-RevId: 184661375
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 1313ab6..d27ea32 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 @@
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 @@
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 @@
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 @@
// 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 @@
* @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);
@@ -837,6 +822,7 @@
*/
public Builder addArtifact(Artifact artifact) {
Preconditions.checkNotNull(artifact);
+ Preconditions.checkArgument(!artifact.isMiddlemanArtifact());
artifactsBuilder.add(artifact);
return this;
}
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 a863234..c609d3d 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.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 @@
@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 2090dde..5302e52 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
@@ -236,12 +236,12 @@
}
/**
- * 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 3d9d547..b7a420a 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
@@ -211,7 +211,7 @@
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 701a293..0227f75 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
@@ -277,7 +277,7 @@
// 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 676a9ee..9996d47 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 @@
.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 24ec6ae..a74487f 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 @@
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 798b186..41182b2 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
@@ -97,7 +97,7 @@
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 876bc1d..fe9cb4e 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
@@ -81,7 +81,7 @@
ruleContext
.getExecutablePrerequisite("$android_resources_busybox", Mode.HOST)
.getRunfilesSupport()
- .getRunfilesArtifactsWithoutMiddlemen());
+ .getRunfilesArtifacts());
List<Artifact> outs = new ArrayList<>();
if (primary.getRTxt() != null) {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesSupplierImplTest.java b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesSupplierImplTest.java
index 952d013..33e3daf 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesSupplierImplTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesSupplierImplTest.java
@@ -17,7 +17,6 @@
import static com.google.common.truth.Truth.assertThat;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
@@ -61,18 +60,6 @@
}
@Test
- public void testGetArtifactsFilterMiddlemen() {
- List<Artifact> artifacts = mkArtifacts(rootDir, "thing1", "thing2");
- Artifact middleman = new Artifact(PathFragment.create("middleman"), middlemanRoot);
- Runfiles runfiles = mkRunfiles(Iterables.concat(artifacts, ImmutableList.of(middleman)));
-
- RunfilesSupplier underTest =
- new RunfilesSupplierImpl(PathFragment.create("notimportant"), runfiles);
-
- assertThat(underTest.getArtifacts()).containsExactlyElementsIn(artifacts);
- }
-
- @Test
public void testGetManifestsWhenNone() {
RunfilesSupplier underTest =
new RunfilesSupplierImpl(PathFragment.create("ignored"), Runfiles.EMPTY, null);
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidHostServiceFixtureTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidHostServiceFixtureTest.java
index 55718d4..aefd956 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidHostServiceFixtureTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidHostServiceFixtureTest.java
@@ -65,7 +65,7 @@
hostServiceFixture
.getProvider(RunfilesProvider.class)
.getDefaultRunfiles()
- .getArtifactsWithoutMiddlemen()))
+ .getArtifacts()))
.containsExactlyElementsIn(
ActionsTestUtil.prettyArtifactNames(
getConfiguredTarget("//java/com/server")
