Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability for a Spawn to declare a required local output #7271

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public final class SimpleSpawn implements Spawn {
private final RunfilesSupplier runfilesSupplier;
private final Map<Artifact, ImmutableList<FilesetOutputSymlink>> filesetMappings;
private final ImmutableList<? extends ActionInput> outputs;
private final ImmutableList<? extends ActionInput> requiredLocalOutputs;
private final ResourceSet localResources;

public SimpleSpawn(
Expand All @@ -49,6 +50,7 @@ public SimpleSpawn(
ImmutableList<? extends ActionInput> inputs,
ImmutableList<? extends ActionInput> tools,
ImmutableList<? extends ActionInput> outputs,
ImmutableList<? extends ActionInput> requiredLocalOutputs,
ResourceSet localResources) {
this.owner = Preconditions.checkNotNull(owner);
this.arguments = Preconditions.checkNotNull(arguments);
Expand All @@ -60,6 +62,7 @@ public SimpleSpawn(
runfilesSupplier == null ? EmptyRunfilesSupplier.INSTANCE : runfilesSupplier;
this.filesetMappings = filesetMappings;
this.outputs = Preconditions.checkNotNull(outputs);
this.requiredLocalOutputs = Preconditions.checkNotNull(requiredLocalOutputs);
this.localResources = Preconditions.checkNotNull(localResources);
}

Expand All @@ -81,6 +84,7 @@ public SimpleSpawn(
inputs,
ImmutableList.<Artifact>of(),
outputs,
/* requiredLocalOutputs= */ ImmutableList.of(),
localResources);
}

Expand Down
12 changes: 12 additions & 0 deletions src/main/java/com/google/devtools/build/lib/actions/Spawn.java
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,18 @@ public interface Spawn {
*/
Collection<? extends ActionInput> getOutputFiles();

/**
* Returns the collection of files that this command must write and make available via
* Bazel's {@link com.google.devtools.build.lib.vfs.FileSystem}. The returned outputs
* are a subset of the spawns's output {@link #getOutputFiles()}.
*
* <p>This is for use with remote execution, where as an optimization we don't want to
* stage (i.e. download) all output files of a spawn on disk.
*/
default Collection<? extends ActionInput> getRequiredLocalOutputs() {
return ImmutableList.of();
}

/**
* Returns the resource owner for local fallback.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ public TestRunnerSpawn createTestRunnerSpawn(
/*inputs=*/ ImmutableList.copyOf(action.getInputs()),
/*tools=*/ ImmutableList.<Artifact>of(),
ImmutableList.copyOf(action.getSpawnOutputs()),
/*requiredLocalOutputs=*/ ImmutableList.of(),
localResourceUsage);
return new StandaloneTestRunnerSpawn(
action, actionExecutionContext, spawn, tmpDir, coverageDir, workingDirectory, execRoot);
Expand Down Expand Up @@ -435,6 +436,7 @@ private Spawn createXmlGeneratingSpawn(TestRunnerAction action, SpawnResult resu
/*inputs=*/ ImmutableList.of(action.getTestXmlGeneratorScript(), action.getTestLog()),
/*tools=*/ ImmutableList.<Artifact>of(),
/*outputs=*/ ImmutableList.of(ActionInputHelper.fromPath(action.getXmlOutputPath())),
/*requiredLocalOutputs=*/ ImmutableList.of(),
SpawnAction.DEFAULT_RESOURCE_SET);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier;
import com.google.devtools.build.lib.actions.ExecException;
Expand All @@ -28,13 +29,10 @@
import com.google.devtools.build.lib.actions.SpawnResult;
import java.util.List;

/**
* A context for C++ compilation that calls into a {@link SpawnActionContext}.
*/
/** A context for C++ compilation that calls into a {@link SpawnActionContext}. */
@ExecutionStrategy(
contextType = CppCompileActionContext.class,
name = {"spawn"}
)
contextType = CppCompileActionContext.class,
name = {"spawn"})
public class SpawnGccStrategy implements CppCompileActionContext {
@Override
public CppCompileActionResult execWithReply(
Expand All @@ -48,6 +46,14 @@ public CppCompileActionResult execWithReply(
* not needed for execution.
*/
action.getMandatoryInputs(), action.getAdditionalInputs());

ImmutableList<? extends ActionInput> requiredLocalOutputs = ImmutableList.of();
if (action.getDotdFile() != null) {
// .d file scanning happens locally even if the compiler was run remotely. We thus need
// to ensure that the .d file is staged on the local fileystem.
requiredLocalOutputs = ImmutableList.of(action.getDotdFile().artifact());
}

Spawn spawn =
new SimpleSpawn(
action,
Expand All @@ -59,10 +65,12 @@ public CppCompileActionResult execWithReply(
ImmutableList.copyOf(inputs),
/* tools= */ ImmutableList.of(),
action.getOutputs().asList(),
requiredLocalOutputs,
action.estimateResourceConsumptionLocal());

List<SpawnResult> spawnResults =
actionExecutionContext.getContext(SpawnActionContext.class)
actionExecutionContext
.getContext(SpawnActionContext.class)
.exec(spawn, actionExecutionContext);
return CppCompileActionResult.builder().setSpawnResults(spawnResults).build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public Spawn build() {
ImmutableList.copyOf(inputs),
/*tools=*/ ImmutableList.<Artifact>of(),
ImmutableList.copyOf(outputs),
/*requiredLocalOutputs=*/ ImmutableList.of(),
ResourceSet.ZERO);
}

Expand Down