Skip to content

Commit

Permalink
[7.0.0] Remove RunfilesSupplier.getManifest(). (#20357)
Browse files Browse the repository at this point in the history 
It appears to be always empty.

RELNOTES: None.
PiperOrigin-RevId: 585627763
Change-Id: I7f39896c7f3a4ffb1040211265793b7f64d8d64d

Co-authored-by: Googler <[email protected]>
  • Loading branch information
@tjgq @lberki
tjgq and lberki authored Nov 29, 2023
1 parent a9747c1 commit 2b700b5
Show file tree
Hide file tree
Showing 14 changed files with 14 additions and 134 deletions.
9 changes: 0 additions & 9 deletions src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,15 +93,6 @@ public ImmutableMap<PathFragment, Map<PathFragment, Artifact>> getMappings() {
return ImmutableMap.copyOf(result);
}

@Override
public ImmutableList<Artifact> getManifests() {
ImmutableList.Builder<Artifact> result = ImmutableList.builder();
for (RunfilesSupplier supplier : suppliers) {
result.addAll(supplier.getManifests());
}
return result.build();
}

@Override
@Nullable
public RunfileSymlinksMode getRunfileSymlinksMode(PathFragment runfilesDir) {
Expand Down
6 changes: 0 additions & 6 deletions src/main/java/com/google/devtools/build/lib/actions/EmptyRunfilesSupplier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -49,11 +48,6 @@ public ImmutableMap<PathFragment, Map<PathFragment, Artifact>> getMappings() {
return ImmutableMap.of();
}

@Override
public ImmutableList<Artifact> getManifests() {
return ImmutableList.of();
}

@Override
@Nullable
public RunfileSymlinksMode getRunfileSymlinksMode(PathFragment runfilesDir) {
Expand Down
4 changes: 0 additions & 4 deletions src/main/java/com/google/devtools/build/lib/actions/RunfilesSupplier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -41,9 +40,6 @@ public interface RunfilesSupplier extends StarlarkValue {
/** Returns mappings from runfiles directories to artifact mappings in that directory. */
ImmutableMap<PathFragment, Map<PathFragment, Artifact>> getMappings();

/** Returns the runfiles manifest artifacts, if any. */
ImmutableList<Artifact> getManifests();

/**
* Returns the {@link RunfileSymlinksMode} for the given {@code runfilesDir}, or {@code null} if
* the {@link RunfilesSupplier} doesn't know about the directory.
Expand Down
6 changes: 0 additions & 6 deletions src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -554,19 +554,13 @@ public ImmutableMap<PathFragment, Map<PathFragment, Artifact>> getMappings() {
/* eventHandler= */ null, /* location= */ null, repoMappingManifest));
}

@Override
public ImmutableList<Artifact> getManifests() {
return ImmutableList.of();
}

@Override
public RunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfilesDir) {
return newRunfilesDir.equals(getRunfilesDirectoryExecPath())
? this
: new SingleRunfilesSupplier(
newRunfilesDir,
runfiles,
/* manifest= */ null,
repoMappingManifest,
runfileSymlinksMode,
buildRunfileLinks);
Expand Down
24 changes: 3 additions & 21 deletions src/main/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -38,15 +37,14 @@ public final class SingleRunfilesSupplier implements RunfilesSupplier {
private final PathFragment runfilesDir;
private final Runfiles runfiles;
private final Supplier<Map<PathFragment, Artifact>> 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}.
*
* <p>The runfiles inputs are computed lazily and softly cached. Caching is shared across
* instances created via {@link #withOverriddenRunfilesDir}.
Expand All @@ -61,7 +59,6 @@ public static SingleRunfilesSupplier createCaching(
runfilesDir,
runfiles,
/* runfilesCachingEnabled= */ true,
/* manifest= */ null,
repoMappingManifest,
runfileSymlinksMode,
buildRunfileLinks);
Expand All @@ -72,25 +69,20 @@ 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
*/
@AutoCodec.Instantiator
public SingleRunfilesSupplier(
PathFragment runfilesDir,
Runfiles runfiles,
@Nullable Artifact manifest,
@Nullable Artifact repoMappingManifest,
RunfileSymlinksMode runfileSymlinksMode,
boolean buildRunfileLinks) {
this(
runfilesDir,
runfiles,
/* runfilesCachingEnabled= */ false,
manifest,
repoMappingManifest,
runfileSymlinksMode,
buildRunfileLinks);
Expand All @@ -100,7 +92,6 @@ private SingleRunfilesSupplier(
PathFragment runfilesDir,
Runfiles runfiles,
boolean runfilesCachingEnabled,
@Nullable Artifact manifest,
@Nullable Artifact repoMappingManifest,
RunfileSymlinksMode runfileSymlinksMode,
boolean buildRunfileLinks) {
Expand All @@ -112,7 +103,6 @@ private SingleRunfilesSupplier(
: () ->
runfiles.getRunfilesInputs(
/* eventHandler= */ null, /* location= */ null, repoMappingManifest),
manifest,
repoMappingManifest,
runfileSymlinksMode,
buildRunfileLinks);
Expand All @@ -122,15 +112,13 @@ private SingleRunfilesSupplier(
PathFragment runfilesDir,
Runfiles runfiles,
Supplier<Map<PathFragment, Artifact>> runfilesInputs,
@Nullable Artifact manifest,
@Nullable Artifact repoMappingManifest,
RunfileSymlinksMode runfileSymlinksMode,
boolean buildRunfileLinks) {
checkArgument(!runfilesDir.isAbsolute());
this.runfilesDir = checkNotNull(runfilesDir);
this.runfiles = checkNotNull(runfiles);
this.runfilesInputs = checkNotNull(runfilesInputs);
this.manifest = manifest;
this.repoMappingManifest = repoMappingManifest;
this.runfileSymlinksMode = runfileSymlinksMode;
this.buildRunfileLinks = buildRunfileLinks;
Expand All @@ -151,11 +139,6 @@ public ImmutableMap<PathFragment, Map<PathFragment, Artifact>> getMappings() {
return ImmutableMap.of(runfilesDir, runfilesInputs.get());
}

@Override
public ImmutableList<Artifact> getManifests() {
return manifest != null ? ImmutableList.of(manifest) : ImmutableList.of();
}

@Override
@Nullable
public RunfileSymlinksMode getRunfileSymlinksMode(PathFragment runfilesDir) {
Expand All @@ -178,7 +161,6 @@ public SingleRunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfiles
newRunfilesDir,
runfiles,
runfilesInputs,
manifest,
repoMappingManifest,
runfileSymlinksMode,
buildRunfileLinks);
Expand Down
8 changes: 1 addition & 7 deletions src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Artifact> 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);
Expand Down Expand Up @@ -522,9 +517,8 @@ private ActionSpawn(
parent,
parent.resourceSetOrBuilder);
NestedSetBuilder<ActionInput> inputsBuilder = NestedSetBuilder.stableOrder();
ImmutableList<Artifact> manifests = getRunfilesSupplier().getManifests();
for (Artifact input : inputs.toList()) {
if (!input.isFileset() && !manifests.contains(input)) {
if (!input.isFileset()) {
inputsBuilder.add(input);
}
}
Expand Down
4 changes: 0 additions & 4 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,10 +261,6 @@ protected void computeKey(
}
fp.addString(getMnemonic());
fp.addPaths(getRunfilesSupplier().getRunfilesDirs());
ImmutableList<Artifact> runfilesManifests = getRunfilesSupplier().getManifests();
for (Artifact runfilesManifest : runfilesManifests) {
fp.addPath(runfilesManifest.getExecPath());
}
for (Artifact input : mandatoryInputs.toList()) {
fp.addPath(input.getExecPath());
}
Expand Down
5 changes: 0 additions & 5 deletions src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Artifact> runfilesManifests = getRunfilesSupplier().getManifests();
fp.addInt(runfilesManifests.size());
for (Artifact runfilesManifest : runfilesManifests) {
fp.addPath(runfilesManifest.getExecPath());
}
getEnvironment().addTo(fp);
fp.addStringMap(executionInfo);
PathMappers.addToFingerprint(
Expand Down
1 change: 0 additions & 1 deletion src/main/java/com/google/devtools/build/lib/rules/python/PyBuiltins.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
31 changes: 0 additions & 31 deletions src/test/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplierTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,48 +56,19 @@ public void testGetArtifactsWithSingleMapping() {
new SingleRunfilesSupplier(
PathFragment.create("notimportant"),
mkRunfiles(artifacts),
/* manifest= */ null,
/* repoMappingManifest= */ null,
RunfileSymlinksMode.SKIP,
/* buildRunfileLinks= */ false);

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);
Expand All @@ -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
Expand All @@ -119,7 +89,6 @@ public void withOverriddenRunfilesDir_noChange_sameObject() {
new SingleRunfilesSupplier(
dir,
Runfiles.EMPTY,
ActionsTestUtil.createArtifact(rootDir, "manifest"),
/* repoMappingManifest= */ null,
RunfileSymlinksMode.SKIP,
/* buildRunfileLinks= */ false);
Expand Down
17 changes: 3 additions & 14 deletions src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -394,7 +391,6 @@ private enum KeyAttributes {
EXECUTABLE_PATH,
EXECUTABLE,
MNEMONIC,
RUNFILES_SUPPLIER,
RUNFILES_SUPPLIER_PATH,
ENVIRONMENT
}
Expand Down Expand Up @@ -425,16 +421,10 @@ public Action generate(ImmutableSet<KeyAttributes> 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<String, String> env = new HashMap<>();
Expand Down Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,6 @@ public static RunfilesSupplier createRunfilesSupplier(
return new SingleRunfilesSupplier(
runfilesDir,
runfiles,
/* manifest= */ null,
/* repoMappingManifest= */ null,
RunfileSymlinksMode.SKIP,
/* buildRunfileLinks= */ false);
Expand Down
5 changes: 0 additions & 5 deletions src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2516,11 +2516,6 @@ public ImmutableMap<PathFragment, Map<PathFragment, Artifact>> getMappings() {
artifacts.stream().collect(toImmutableMap(Artifact::getExecPath, a -> a)));
}

@Override
public ImmutableList<Artifact> getManifests() {
throw new UnsupportedOperationException();
}

@Override
public RunfileSymlinksMode getRunfileSymlinksMode(PathFragment runfilesDir) {
throw new UnsupportedOperationException();
Expand Down
Loading

0 comments on commit 2b700b5

Please sign in to comment.