From ff179a3b3a97cd91037a2d3fb8a9792b1cc96d36 Mon Sep 17 00:00:00 2001 From: ulfjack Date: Tue, 6 Feb 2018 04:16:34 -0800 Subject: 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 --- .../devtools/build/lib/analysis/Runfiles.java | 24 +++++----------------- .../build/lib/analysis/RunfilesSupplierImpl.java | 3 +-- .../build/lib/analysis/RunfilesSupport.java | 10 ++++----- .../build/lib/analysis/SourceManifestAction.java | 2 +- .../bazel/rules/python/BazelPythonSemantics.java | 2 +- .../rules/android/AndroidHostServiceFixture.java | 2 +- .../android/LibraryRGeneratorActionBuilder.java | 2 +- .../rules/android/ManifestMergerActionBuilder.java | 2 +- .../android/RClassGeneratorActionBuilder.java | 2 +- 9 files changed, 17 insertions(+), 32 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib') 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..d27ea32781 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 getUnconditionalArtifactsWithoutMiddlemen() { - return Iterables.filter(unconditionalArtifacts, Artifact.MIDDLEMAN_FILTER); - } - public NestedSet 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 getArtifactsWithoutMiddlemen() { - return Iterables.filter(getArtifacts(), Artifact.MIDDLEMAN_FILTER); - } - /** Returns the symlinks. */ @SkylarkCallable(name = "symlinks", doc = "Returns the set of symlinks.", structField = true) public NestedSet getSymlinks() { @@ -456,7 +439,7 @@ public final class Runfiles { ConflictChecker checker = new ConflictChecker(conflictPolicy, eventHandler, location); Map 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 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 final class Runfiles { */ 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 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 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 2090ddedbb..5302e5297e 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 @@ 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 getRunfilesArtifactsWithoutMiddlemen() { - return runfiles.getArtifactsWithoutMiddlemen(); + public Iterable 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 3d9d547659..b7a420a03e 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 @@ 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 701a29327d..0227f75ab2 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 @@ 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 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 798b1867a3..41182b285f 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 @@ 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 876bc1d9a8..fe9cb4e979 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 @@ public class RClassGeneratorActionBuilder { ruleContext .getExecutablePrerequisite("$android_resources_busybox", Mode.HOST) .getRunfilesSupport() - .getRunfilesArtifactsWithoutMiddlemen()); + .getRunfilesArtifacts()); List outs = new ArrayList<>(); if (primary.getRTxt() != null) { -- cgit v1.2.3