Switch to RunfilesSuppliers for communicating runfiles
ActionSpawn/SpawnAction now deal exclusively in RunfilesSuppliers, manifests
maps are no more.
There is some lingering awkwardness, in particular:
- Manifests still need to be tracked in some places, we can work out if this is
still necessary on a case by case basis.
- Skylark makes actions' runfiles available via 'resolve_command' where they are
consumed by 'action'. I've updated the documentation, though the name isn't
entirely accurate anymore. That being said these interfaces _are_ marked as
experimental, so we _should_ be able to be flexible here.
Overall, I think the benefits consolidating runfiles into suppliers, from both
code cleanliness and performance perspectives (no longer needing to parse
manifests), outweights the awkwardnesses.
RELNOTES: resolve_command/action's input_manifest return/parameter is now list
--
PiperOrigin-RevId: 145817429
MOS_MIGRATED_REVID=145817429
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
index dd2d5de..6208db6 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.actions;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -37,21 +36,15 @@
private final ImmutableList<String> arguments;
private final ImmutableMap<String, String> environment;
private final ImmutableMap<String, String> executionInfo;
- private final ImmutableMap<PathFragment, Artifact> runfilesManifests;
private final ImmutableSet<PathFragment> optionalOutputFiles;
private final RunfilesSupplier runfilesSupplier;
private final ActionExecutionMetadata action;
private final ResourceSet localResources;
- // TODO(bazel-team): When we migrate ActionSpawn to use this constructor decide on and enforce
- // policy on runfilesManifests and runfilesSupplier being non-empty (ie: are overlapping mappings
- // allowed?).
- @VisibleForTesting
- BaseSpawn(
+ public BaseSpawn(
List<String> arguments,
Map<String, String> environment,
Map<String, String> executionInfo,
- Map<PathFragment, Artifact> runfilesManifests,
RunfilesSupplier runfilesSupplier,
ActionExecutionMetadata action,
ResourceSet localResources,
@@ -59,7 +52,6 @@
this.arguments = ImmutableList.copyOf(arguments);
this.environment = ImmutableMap.copyOf(environment);
this.executionInfo = ImmutableMap.copyOf(executionInfo);
- this.runfilesManifests = ImmutableMap.copyOf(runfilesManifests);
this.runfilesSupplier = runfilesSupplier;
this.action = action;
this.localResources = localResources;
@@ -81,35 +73,12 @@
arguments,
environment,
executionInfo,
- ImmutableMap.<PathFragment, Artifact>of(),
runfilesSupplier,
action,
localResources,
ImmutableSet.<PathFragment>of());
}
- /**
- * Returns a new Spawn. The caller must not modify the parameters after the call; neither will
- * this method.
- */
- public BaseSpawn(
- List<String> arguments,
- Map<String, String> environment,
- Map<String, String> executionInfo,
- Map<PathFragment, Artifact> runfilesManifests,
- ActionExecutionMetadata action,
- ResourceSet localResources) {
- this(
- arguments,
- environment,
- executionInfo,
- runfilesManifests,
- EmptyRunfilesSupplier.INSTANCE,
- action,
- localResources,
- ImmutableSet.<PathFragment>of());
- }
-
/** Returns a new Spawn. */
public BaseSpawn(
List<String> arguments,
@@ -121,30 +90,11 @@
arguments,
environment,
executionInfo,
- ImmutableMap.<PathFragment, Artifact>of(),
+ EmptyRunfilesSupplier.INSTANCE,
action,
localResources);
}
- public BaseSpawn(
- List<String> arguments,
- Map<String, String> environment,
- Map<String, String> executionInfo,
- RunfilesSupplier runfilesSupplier,
- ActionExecutionMetadata action,
- ResourceSet localResources,
- Collection<PathFragment> optionalOutputFiles) {
- this(
- arguments,
- environment,
- executionInfo,
- ImmutableMap.<PathFragment, Artifact>of(),
- runfilesSupplier,
- action,
- localResources,
- optionalOutputFiles);
- }
-
public static PathFragment runfilesForFragment(PathFragment pathFragment) {
return pathFragment.getParentDirectory().getChild(pathFragment.getBaseName() + ".runfiles");
}
@@ -170,11 +120,6 @@
}
@Override
- public ImmutableMap<PathFragment, Artifact> getRunfilesManifests() {
- return runfilesManifests;
- }
-
- @Override
public RunfilesSupplier getRunfilesSupplier() {
return runfilesSupplier;
}
@@ -234,10 +179,8 @@
/** @return the runfiles directory if there is only one, otherwise null */
private PathFragment getRunfilesRoot() {
Set<PathFragment> runfilesSupplierRoots = runfilesSupplier.getRunfilesDirs();
- if (runfilesSupplierRoots.size() == 1 && runfilesManifests.isEmpty()) {
+ if (runfilesSupplierRoots.size() == 1) {
return Iterables.getOnlyElement(runfilesSupplierRoots);
- } else if (runfilesManifests.size() == 1 && runfilesSupplierRoots.isEmpty()) {
- return Iterables.getOnlyElement(runfilesManifests.keySet());
} else {
return null;
}
