diff --git a/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java b/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java index 36776d62fb2626..7a6823af5ce3e0 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java +++ b/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java @@ -93,15 +93,6 @@ public ImmutableMap> getMappings() { return ImmutableMap.copyOf(result); } - @Override - public ImmutableList getManifests() { - ImmutableList.Builder result = ImmutableList.builder(); - for (RunfilesSupplier supplier : suppliers) { - result.addAll(supplier.getManifests()); - } - return result.build(); - } - @Override @Nullable public RunfileSymlinksMode getRunfileSymlinksMode(PathFragment runfilesDir) { diff --git a/src/main/java/com/google/devtools/build/lib/actions/EmptyRunfilesSupplier.java b/src/main/java/com/google/devtools/build/lib/actions/EmptyRunfilesSupplier.java index c7847cb6fbdc0a..7ba71666df9740 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/EmptyRunfilesSupplier.java +++ b/src/main/java/com/google/devtools/build/lib/actions/EmptyRunfilesSupplier.java @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.actions; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue.RunfileSymlinksMode; @@ -49,11 +48,6 @@ public ImmutableMap> getMappings() { return ImmutableMap.of(); } - @Override - public ImmutableList getManifests() { - return ImmutableList.of(); - } - @Override @Nullable public RunfileSymlinksMode getRunfileSymlinksMode(PathFragment runfilesDir) { diff --git a/src/main/java/com/google/devtools/build/lib/actions/RunfilesSupplier.java b/src/main/java/com/google/devtools/build/lib/actions/RunfilesSupplier.java index ded16ef000e6db..a57e1452365326 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/RunfilesSupplier.java +++ b/src/main/java/com/google/devtools/build/lib/actions/RunfilesSupplier.java @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.actions; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue.RunfileSymlinksMode; @@ -41,9 +40,6 @@ public interface RunfilesSupplier extends StarlarkValue { /** Returns mappings from runfiles directories to artifact mappings in that directory. */ ImmutableMap> getMappings(); - /** Returns the runfiles manifest artifacts, if any. */ - ImmutableList getManifests(); - /** * Returns the {@link RunfileSymlinksMode} for the given {@code runfilesDir}, or {@code null} if * the {@link RunfilesSupplier} doesn't know about the directory. 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 30b8fd54867873..56abd4d70c014a 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 @@ -554,11 +554,6 @@ public ImmutableMap> getMappings() { /* eventHandler= */ null, /* location= */ null, repoMappingManifest)); } - @Override - public ImmutableList getManifests() { - return ImmutableList.of(); - } - @Override public RunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfilesDir) { return newRunfilesDir.equals(getRunfilesDirectoryExecPath()) @@ -566,7 +561,6 @@ public RunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfilesDir) { : new SingleRunfilesSupplier( newRunfilesDir, runfiles, - /* manifest= */ null, repoMappingManifest, runfileSymlinksMode, buildRunfileLinks); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplier.java b/src/main/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplier.java index 4d9ac6ee2a2772..26fca6a7c45a07 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplier.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplier.java @@ -17,7 +17,6 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.Artifact; @@ -38,15 +37,14 @@ public final class SingleRunfilesSupplier implements RunfilesSupplier { private final PathFragment runfilesDir; private final Runfiles runfiles; private final Supplier> runfilesInputs; - @Nullable private final Artifact manifest; @Nullable private final Artifact repoMappingManifest; private final RunfileSymlinksMode runfileSymlinksMode; private final boolean buildRunfileLinks; /** - * Same as {@link SingleRunfilesSupplier#SingleRunfilesSupplier(PathFragment, Runfiles, Artifact, - * Artifact, boolean, boolean)}, except adds caching for {@linkplain Runfiles#getRunfilesInputs - * runfiles inputs}. + * Same as {@link SingleRunfilesSupplier#SingleRunfilesSupplier(PathFragment, Runfiles, boolean, + * Artifact, RunfileSymlinksMode, boolean)}, except adds caching for {@linkplain + * Runfiles#getRunfilesInputs runfiles inputs}. * *

The runfiles inputs are computed lazily and softly cached. Caching is shared across * instances created via {@link #withOverriddenRunfilesDir}. @@ -61,7 +59,6 @@ public static SingleRunfilesSupplier createCaching( runfilesDir, runfiles, /* runfilesCachingEnabled= */ true, - /* manifest= */ null, repoMappingManifest, runfileSymlinksMode, buildRunfileLinks); @@ -72,9 +69,6 @@ public static SingleRunfilesSupplier createCaching( * * @param runfilesDir the desired runfiles directory. Should be relative. * @param runfiles the runfiles for runilesDir. - * @param manifest runfiles' associated runfiles manifest artifact, if present. Important: this - * parameter will be used to filter the resulting spawn's inputs to not poison downstream - * caches. * @param runfileSymlinksMode how to create runfile symlinks * @param buildRunfileLinks whether runfile symlinks should be created during the build */ @@ -82,7 +76,6 @@ public static SingleRunfilesSupplier createCaching( public SingleRunfilesSupplier( PathFragment runfilesDir, Runfiles runfiles, - @Nullable Artifact manifest, @Nullable Artifact repoMappingManifest, RunfileSymlinksMode runfileSymlinksMode, boolean buildRunfileLinks) { @@ -90,7 +83,6 @@ public SingleRunfilesSupplier( runfilesDir, runfiles, /* runfilesCachingEnabled= */ false, - manifest, repoMappingManifest, runfileSymlinksMode, buildRunfileLinks); @@ -100,7 +92,6 @@ private SingleRunfilesSupplier( PathFragment runfilesDir, Runfiles runfiles, boolean runfilesCachingEnabled, - @Nullable Artifact manifest, @Nullable Artifact repoMappingManifest, RunfileSymlinksMode runfileSymlinksMode, boolean buildRunfileLinks) { @@ -112,7 +103,6 @@ private SingleRunfilesSupplier( : () -> runfiles.getRunfilesInputs( /* eventHandler= */ null, /* location= */ null, repoMappingManifest), - manifest, repoMappingManifest, runfileSymlinksMode, buildRunfileLinks); @@ -122,7 +112,6 @@ private SingleRunfilesSupplier( PathFragment runfilesDir, Runfiles runfiles, Supplier> runfilesInputs, - @Nullable Artifact manifest, @Nullable Artifact repoMappingManifest, RunfileSymlinksMode runfileSymlinksMode, boolean buildRunfileLinks) { @@ -130,7 +119,6 @@ private SingleRunfilesSupplier( this.runfilesDir = checkNotNull(runfilesDir); this.runfiles = checkNotNull(runfiles); this.runfilesInputs = checkNotNull(runfilesInputs); - this.manifest = manifest; this.repoMappingManifest = repoMappingManifest; this.runfileSymlinksMode = runfileSymlinksMode; this.buildRunfileLinks = buildRunfileLinks; @@ -151,11 +139,6 @@ public ImmutableMap> getMappings() { return ImmutableMap.of(runfilesDir, runfilesInputs.get()); } - @Override - public ImmutableList getManifests() { - return manifest != null ? ImmutableList.of(manifest) : ImmutableList.of(); - } - @Override @Nullable public RunfileSymlinksMode getRunfileSymlinksMode(PathFragment runfilesDir) { @@ -178,7 +161,6 @@ public SingleRunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfiles newRunfilesDir, runfiles, runfilesInputs, - manifest, repoMappingManifest, runfileSymlinksMode, buildRunfileLinks); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java index a79834e25da8c2..f44ed8ae4db3c9 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java @@ -378,11 +378,6 @@ protected void computeKey( // definition and the output of an action shouldn't change whether something is considered a // tool or not. fp.addPaths(runfilesSupplier.getRunfilesDirs()); - ImmutableList runfilesManifests = runfilesSupplier.getManifests(); - fp.addInt(runfilesManifests.size()); - for (Artifact runfilesManifest : runfilesManifests) { - fp.addPath(runfilesManifest.getExecPath()); - } env.addTo(fp); fp.addStringMap(getExecutionInfo()); PathMappers.addToFingerprint(getMnemonic(), getExecutionInfo(), outputPathsMode, fp); @@ -522,9 +517,8 @@ private ActionSpawn( parent, parent.resourceSetOrBuilder); NestedSetBuilder inputsBuilder = NestedSetBuilder.stableOrder(); - ImmutableList manifests = getRunfilesSupplier().getManifests(); for (Artifact input : inputs.toList()) { - if (!input.isFileset() && !manifests.contains(input)) { + if (!input.isFileset()) { inputsBuilder.add(input); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java index ba1edb355304d7..2ae7cd8314b3e6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java @@ -261,10 +261,6 @@ protected void computeKey( } fp.addString(getMnemonic()); fp.addPaths(getRunfilesSupplier().getRunfilesDirs()); - ImmutableList runfilesManifests = getRunfilesSupplier().getManifests(); - for (Artifact runfilesManifest : runfilesManifests) { - fp.addPath(runfilesManifest.getExecPath()); - } for (Artifact input : mandatoryInputs.toList()) { fp.addPath(input.getExecPath()); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java index 8a8508910404c4..c0439855a55dcc 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java @@ -237,11 +237,6 @@ protected void computeKey( // definition and the output of an action shouldn't change whether something is considered a // tool or not. fp.addPaths(getRunfilesSupplier().getRunfilesDirs()); - ImmutableList runfilesManifests = getRunfilesSupplier().getManifests(); - fp.addInt(runfilesManifests.size()); - for (Artifact runfilesManifest : runfilesManifests) { - fp.addPath(runfilesManifest.getExecPath()); - } getEnvironment().addTo(fp); fp.addStringMap(executionInfo); PathMappers.addToFingerprint( diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyBuiltins.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyBuiltins.java index ecc78aaefa0cfb..7434d644147c4f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyBuiltins.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyBuiltins.java @@ -150,7 +150,6 @@ public Object addEnv(StarlarkRuleContext ruleContext, String runfilesStr, Runfil return new SingleRunfilesSupplier( PathFragment.create(runfilesStr), runfiles, - /* manifest= */ null, /* repoMappingManifest= */ null, ruleContext.getConfiguration().getRunfileSymlinksMode(), ruleContext.getConfiguration().buildRunfileLinks()); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplierTest.java b/src/test/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplierTest.java index 65a6277fa301e5..edbc789a828d66 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplierTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplierTest.java @@ -56,7 +56,6 @@ public void testGetArtifactsWithSingleMapping() { new SingleRunfilesSupplier( PathFragment.create("notimportant"), mkRunfiles(artifacts), - /* manifest= */ null, /* repoMappingManifest= */ null, RunfileSymlinksMode.SKIP, /* buildRunfileLinks= */ false); @@ -64,40 +63,12 @@ public void testGetArtifactsWithSingleMapping() { assertThat(underTest.getArtifacts().toList()).containsExactlyElementsIn(artifacts); } - @Test - public void testGetManifestsWhenNone() { - RunfilesSupplier underTest = - new SingleRunfilesSupplier( - PathFragment.create("ignored"), - Runfiles.EMPTY, - /* manifest= */ null, - /* repoMappingManifest= */ null, - RunfileSymlinksMode.SKIP, - /* buildRunfileLinks= */ false); - assertThat(underTest.getManifests()).isEmpty(); - } - - @Test - public void testGetManifestsWhenSupplied() { - Artifact manifest = ActionsTestUtil.createArtifact(rootDir, "manifest"); - RunfilesSupplier underTest = - new SingleRunfilesSupplier( - PathFragment.create("ignored"), - Runfiles.EMPTY, - manifest, - /* repoMappingManifest= */ null, - RunfileSymlinksMode.SKIP, - /* buildRunfileLinks= */ false); - assertThat(underTest.getManifests()).containsExactly(manifest); - } - @Test public void withOverriddenRunfilesDir() { SingleRunfilesSupplier original = new SingleRunfilesSupplier( PathFragment.create("old"), Runfiles.EMPTY, - ActionsTestUtil.createArtifact(rootDir, "manifest"), /* repoMappingManifest= */ null, RunfileSymlinksMode.SKIP, /* buildRunfileLinks= */ false); @@ -109,7 +80,6 @@ public void withOverriddenRunfilesDir() { assertThat(overridden.getMappings()) .containsExactly(newDir, Iterables.getOnlyElement(original.getMappings().values())); assertThat(overridden.getArtifacts()).isEqualTo(original.getArtifacts()); - assertThat(overridden.getManifests()).isEqualTo(original.getManifests()); } @Test @@ -119,7 +89,6 @@ public void withOverriddenRunfilesDir_noChange_sameObject() { new SingleRunfilesSupplier( dir, Runfiles.EMPTY, - ActionsTestUtil.createArtifact(rootDir, "manifest"), /* repoMappingManifest= */ null, RunfileSymlinksMode.SKIP, /* buildRunfileLinks= */ false); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java index bb0743a6b849f0..e90c2183fb71b0 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java @@ -369,15 +369,12 @@ public void testExtraActionInfoEnvironmentVariables() throws Exception { @Test public void testInputManifestsRemovedIfSupplied() throws Exception { - Artifact manifest = getSourceArtifact("MANIFEST"); SpawnAction action = builder() - .addInput(manifest) .addRunfilesSupplier( new SingleRunfilesSupplier( PathFragment.create("destination"), Runfiles.EMPTY, - manifest, /* repoMappingManifest= */ null, RunfileSymlinksMode.SKIP, /* buildRunfileLinks= */ false)) @@ -394,7 +391,6 @@ private enum KeyAttributes { EXECUTABLE_PATH, EXECUTABLE, MNEMONIC, - RUNFILES_SUPPLIER, RUNFILES_SUPPLIER_PATH, ENVIRONMENT } @@ -425,16 +421,10 @@ public Action generate(ImmutableSet attributesToFlip) { builder.setMnemonic(attributesToFlip.contains(KeyAttributes.MNEMONIC) ? "a" : "b"); - if (attributesToFlip.contains(KeyAttributes.RUNFILES_SUPPLIER)) { - builder.addRunfilesSupplier(runfilesSupplier(artifactA, PathFragment.create("a"))); - } else { - builder.addRunfilesSupplier(runfilesSupplier(artifactB, PathFragment.create("a"))); - } - if (attributesToFlip.contains(KeyAttributes.RUNFILES_SUPPLIER_PATH)) { - builder.addRunfilesSupplier(runfilesSupplier(artifactA, PathFragment.create("aa"))); + builder.addRunfilesSupplier(runfilesSupplier(PathFragment.create("aa"))); } else { - builder.addRunfilesSupplier(runfilesSupplier(artifactA, PathFragment.create("ab"))); + builder.addRunfilesSupplier(runfilesSupplier(PathFragment.create("ab"))); } Map env = new HashMap<>(); @@ -585,11 +575,10 @@ public void testWorkerMnemonicOverride() throws Exception { .isEqualTo("ToolPoolMnemonic"); } - private static RunfilesSupplier runfilesSupplier(Artifact manifest, PathFragment dir) { + private static RunfilesSupplier runfilesSupplier(PathFragment dir) { return new SingleRunfilesSupplier( dir, Runfiles.EMPTY, - manifest, /* repoMappingManifest= */ null, RunfileSymlinksMode.SKIP, /* buildRunfileLinks= */ false); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java index c741281e45433a..a892023d0a4e31 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java @@ -545,7 +545,6 @@ public static RunfilesSupplier createRunfilesSupplier( return new SingleRunfilesSupplier( runfilesDir, runfiles, - /* manifest= */ null, /* repoMappingManifest= */ null, RunfileSymlinksMode.SKIP, /* buildRunfileLinks= */ false); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java index f3aae430d80d8f..00cd51e3c17255 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java @@ -2516,11 +2516,6 @@ public ImmutableMap> getMappings() { artifacts.stream().collect(toImmutableMap(Artifact::getExecPath, a -> a))); } - @Override - public ImmutableList getManifests() { - throw new UnsupportedOperationException(); - } - @Override public RunfileSymlinksMode getRunfileSymlinksMode(PathFragment runfilesDir) { throw new UnsupportedOperationException(); diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java index 08164de32c87d1..7d7a4dfe0604d8 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java @@ -173,7 +173,6 @@ private enum KeyAttributes { EXECUTABLE, IMPORTS_INFO, MNEMONIC, - RUNFILES_SUPPLIER, INPUT, FIXED_ENVIRONMENT } @@ -211,25 +210,13 @@ public Action generate(ImmutableSet attributesToFlip) { builder.setMnemonic(attributesToFlip.contains(KeyAttributes.MNEMONIC) ? "a" : "b"); - if (attributesToFlip.contains(KeyAttributes.RUNFILES_SUPPLIER)) { - builder.addRunfilesSupplier( - new SingleRunfilesSupplier( - PathFragment.create("a"), - Runfiles.EMPTY, - artifactA, - /* repoMappingManifest= */ null, - RunfileSymlinksMode.SKIP, - /* buildRunfileLinks= */ false)); - } else { - builder.addRunfilesSupplier( - new SingleRunfilesSupplier( - PathFragment.create("a"), - Runfiles.EMPTY, - artifactB, - /* repoMappingManifest= */ null, - RunfileSymlinksMode.SKIP, - /* buildRunfileLinks= */ false)); - } + builder.addRunfilesSupplier( + new SingleRunfilesSupplier( + PathFragment.create("a"), + Runfiles.EMPTY, + /* repoMappingManifest= */ null, + RunfileSymlinksMode.SKIP, + /* buildRunfileLinks= */ false)); if (attributesToFlip.contains(KeyAttributes.INPUT)) { builder.addInput(artifactA);