diff options
author | lberki <lberki@google.com> | 2017-11-14 01:28:25 -0800 |
---|---|---|
committer | Copybara-Service <copybara-piper@google.com> | 2017-11-14 01:29:55 -0800 |
commit | 4260c30a03a9b83d48a5e8690aca19cd80be4c38 (patch) | |
tree | 3f0f94b2e277cb9fccb561270e43bd7aec1d4785 /src/main/java/com/google | |
parent | b064d536b7f4b2593406eadc051a0da13d398a43 (diff) |
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
Diffstat (limited to 'src/main/java/com/google')
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) { |