From b72a43869fe8d8366b56a31fc7593f7282f33b27 Mon Sep 17 00:00:00 2001 From: Jakob Buchgraber Date: Fri, 1 Mar 2019 13:04:02 +0100 Subject: [PATCH] Add ability for a Spawn to declare a required local output Some native actions run both remotely and locally. For example, the CppCompileAction runs compilation remotely but does .d file pruning locally. This change adds the necessary infrastructure for an action to tell a Spawn which of its outputs it needs on the local filesytem. Progress towards #6862 --- .../build/lib/actions/SimpleSpawn.java | 4 ++++ .../devtools/build/lib/actions/Spawn.java | 12 ++++++++++ .../lib/exec/StandaloneTestStrategy.java | 2 ++ .../build/lib/rules/cpp/SpawnGccStrategy.java | 24 ++++++++++++------- .../build/lib/exec/util/SpawnBuilder.java | 1 + 5 files changed, 35 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java index a9b6e547153ddc..8f45134de63f65 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java @@ -37,6 +37,7 @@ public final class SimpleSpawn implements Spawn { private final RunfilesSupplier runfilesSupplier; private final Map> filesetMappings; private final ImmutableList outputs; + private final ImmutableList requiredLocalOutputs; private final ResourceSet localResources; public SimpleSpawn( @@ -49,6 +50,7 @@ public SimpleSpawn( ImmutableList inputs, ImmutableList tools, ImmutableList outputs, + ImmutableList requiredLocalOutputs, ResourceSet localResources) { this.owner = Preconditions.checkNotNull(owner); this.arguments = Preconditions.checkNotNull(arguments); @@ -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); } @@ -81,6 +84,7 @@ public SimpleSpawn( inputs, ImmutableList.of(), outputs, + /* requiredLocalOutputs= */ ImmutableList.of(), localResources); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java index 3f7aa20a67cba7..5b0bb69353e139 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java @@ -96,6 +96,18 @@ public interface Spawn { */ Collection 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()}. + * + *

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 getRequiredLocalOutputs() { + return ImmutableList.of(); + } + /** * Returns the resource owner for local fallback. */ diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java index 63c384d8e048a1..26595f230d0929 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java @@ -136,6 +136,7 @@ public TestRunnerSpawn createTestRunnerSpawn( /*inputs=*/ ImmutableList.copyOf(action.getInputs()), /*tools=*/ ImmutableList.of(), ImmutableList.copyOf(action.getSpawnOutputs()), + /*requiredLocalOutputs=*/ ImmutableList.of(), localResourceUsage); return new StandaloneTestRunnerSpawn( action, actionExecutionContext, spawn, tmpDir, coverageDir, workingDirectory, execRoot); @@ -435,6 +436,7 @@ private Spawn createXmlGeneratingSpawn(TestRunnerAction action, SpawnResult resu /*inputs=*/ ImmutableList.of(action.getTestXmlGeneratorScript(), action.getTestLog()), /*tools=*/ ImmutableList.of(), /*outputs=*/ ImmutableList.of(ActionInputHelper.fromPath(action.getXmlOutputPath())), + /*requiredLocalOutputs=*/ ImmutableList.of(), SpawnAction.DEFAULT_RESOURCE_SET); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java index 0ba465a6b8a1ed..f25a1b6cc8923f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java @@ -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; @@ -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( @@ -48,6 +46,14 @@ public CppCompileActionResult execWithReply( * not needed for execution. */ action.getMandatoryInputs(), action.getAdditionalInputs()); + + ImmutableList 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, @@ -59,10 +65,12 @@ public CppCompileActionResult execWithReply( ImmutableList.copyOf(inputs), /* tools= */ ImmutableList.of(), action.getOutputs().asList(), + requiredLocalOutputs, action.estimateResourceConsumptionLocal()); - + List spawnResults = - actionExecutionContext.getContext(SpawnActionContext.class) + actionExecutionContext + .getContext(SpawnActionContext.class) .exec(spawn, actionExecutionContext); return CppCompileActionResult.builder().setSpawnResults(spawnResults).build(); } diff --git a/src/test/java/com/google/devtools/build/lib/exec/util/SpawnBuilder.java b/src/test/java/com/google/devtools/build/lib/exec/util/SpawnBuilder.java index b425dedef2f852..2941d47f25a4bd 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/util/SpawnBuilder.java +++ b/src/test/java/com/google/devtools/build/lib/exec/util/SpawnBuilder.java @@ -65,6 +65,7 @@ public Spawn build() { ImmutableList.copyOf(inputs), /*tools=*/ ImmutableList.of(), ImmutableList.copyOf(outputs), + /*requiredLocalOutputs=*/ ImmutableList.of(), ResourceSet.ZERO); }