More SpawnResult-related plumbing changes to Actions, Strategies, ActionContexts, etc., so that SpawnResult metadata is returned upwards.
Note that the TODOs mostly refer to changes that will appear in a subsequent CL (a CL to return SpawnResults, contained in ActionResults, from Actions/AbstractActions). I split off the remaining SpawnResult-related changes into this CL and kept the ActionResult-related changes separate.
RELNOTES: None.
PiperOrigin-RevId: 171355611
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionContext.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionContext.java
index bff6530..4a2a19b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionContext.java
@@ -16,6 +16,8 @@
import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.SpawnResult;
+import java.util.Set;
/**
* The action context for {@link AbstractFileWriteAction} instances (technically instances of
@@ -25,7 +27,10 @@
/**
* Performs all the setup and then calls back into the action to write the data.
+ *
+ * @return a set of SpawnResults created during execution, if any
*/
- void exec(AbstractFileWriteAction action, ActionExecutionContext actionExecutionContext)
+ Set<SpawnResult> exec(
+ AbstractFileWriteAction action, ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException;
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeActionContext.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeActionContext.java
index 1ece4b0..7ea2e2b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeActionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeActionContext.java
@@ -17,6 +17,8 @@
import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionExecutionException;
+import com.google.devtools.build.lib.actions.SpawnResult;
+import java.util.Set;
/**
* Action context for symlink tree actions (an action that creates a tree of symlinks).
@@ -25,8 +27,10 @@
/**
* Creates the symlink tree.
+ *
+ * @return a set of SpawnResults created during symlink creation, if any
*/
- void createSymlinks(
+ Set<SpawnResult> createSymlinks(
SymlinkTreeAction action,
ActionExecutionContext actionExecutionContext,
ImmutableMap<String, String> shellEnvironment,
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionContext.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionContext.java
index cc483f6..44ae134 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionContext.java
@@ -16,9 +16,11 @@
import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.view.test.TestStatus.TestResultData;
import java.io.IOException;
+import java.util.Set;
/**
* A context for the execution of test actions ({@link TestRunnerAction}).
@@ -26,11 +28,13 @@
public interface TestActionContext extends ActionContext {
/**
- * Executes the test command, directing standard out / err to {@code outErr}. The status of
- * the test should be communicated by posting a {@link TestResult} object to the eventbus.
+ * Executes the test command, directing standard out / err to {@code outErr}. The status of the
+ * test should be communicated by posting a {@link TestResult} object to the eventbus.
+ *
+ * @return a set of SpawnResults created during execution of the test command, if any
*/
- void exec(TestRunnerAction action,
- ActionExecutionContext actionExecutionContext) throws ExecException, InterruptedException;
+ Set<SpawnResult> exec(TestRunnerAction action, ActionExecutionContext actionExecutionContext)
+ throws ExecException, InterruptedException;
/**
* Creates a cached test result.
diff --git a/src/main/java/com/google/devtools/build/lib/exec/FileWriteStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/FileWriteStrategy.java
index 2ada360..74df515 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/FileWriteStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/FileWriteStrategy.java
@@ -14,11 +14,13 @@
package com.google.devtools.build.lib.exec;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ExecutionStrategy;
+import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction;
import com.google.devtools.build.lib.analysis.actions.FileWriteActionContext;
import com.google.devtools.build.lib.profiler.AutoProfiler;
@@ -26,6 +28,7 @@
import java.io.BufferedOutputStream;
import java.io.IOException;
import java.io.OutputStream;
+import java.util.Set;
import java.util.logging.Logger;
/**
@@ -40,7 +43,8 @@
}
@Override
- public void exec(AbstractFileWriteAction action, ActionExecutionContext actionExecutionContext)
+ public Set<SpawnResult> exec(
+ AbstractFileWriteAction action, ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException {
// TODO(ulfjack): Consider acquiring local resources here before trying to write the file.
try (AutoProfiler p =
@@ -60,5 +64,6 @@
+ "' due to I/O error: " + e.getMessage(), e);
}
}
+ return ImmutableSet.of();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestResult.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestResult.java
new file mode 100644
index 0000000..b4fab1f
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestResult.java
@@ -0,0 +1,68 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.exec;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.actions.SpawnResult;
+import com.google.devtools.build.lib.view.test.TestStatus.TestResultData;
+import java.util.Set;
+
+/** Contains information about the results of test execution. */
+@AutoValue
+public abstract class StandaloneTestResult {
+
+ /** Returns the SpawnResults created by the test, if any. */
+ public abstract Set<SpawnResult> spawnResults();
+
+ /** Returns the TestResultData for the test. */
+ public abstract TestResultData testResultData();
+
+ /** Returns a builder that can be used to construct a {@link StandaloneTestResult} object. */
+ public static Builder builder() {
+ return new AutoValue_StandaloneTestResult.Builder();
+ }
+
+ /** Creates a StandaloneTestResult, given spawnResults and testResultData. */
+ public static StandaloneTestResult create(
+ Set<SpawnResult> spawnResults, TestResultData testResultData) {
+ return builder().setSpawnResults(spawnResults).setTestResultData(testResultData).build();
+ }
+
+ /** Builder for a {@link StandaloneTestResult} instance, which is immutable once built. */
+ @AutoValue.Builder
+ public abstract static class Builder {
+
+ /** Returns the SpawnResults for the test, if any. */
+ abstract Set<SpawnResult> spawnResults();
+
+ /** Sets the SpawnResults for the test. */
+ public abstract Builder setSpawnResults(Set<SpawnResult> spawnResults);
+
+ /** Sets the TestResultData for the test. */
+ public abstract Builder setTestResultData(TestResultData testResultData);
+
+ abstract StandaloneTestResult realBuild();
+
+ /**
+ * Returns an immutable StandaloneTestResult object.
+ *
+ * <p>The set of SpawnResults is also made immutable here.
+ */
+ public StandaloneTestResult build() {
+ return this.setSpawnResults(ImmutableSet.copyOf(spawnResults())).realBuild();
+ }
+ }
+}
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 1ff1e06..d668a02 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
@@ -16,6 +16,7 @@
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.ActionExecutionContext;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.EnvironmentalExecException;
@@ -26,6 +27,7 @@
import com.google.devtools.build.lib.actions.SimpleSpawn;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnActionContext;
+import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.TestExecException;
import com.google.devtools.build.lib.analysis.RunfilesSupplierImpl;
import com.google.devtools.build.lib.analysis.config.BinTools;
@@ -50,6 +52,7 @@
import java.io.Closeable;
import java.io.IOException;
import java.util.Map;
+import java.util.Set;
/** Runs TestRunnerAction actions. */
@ExecutionStrategy(
@@ -85,7 +88,8 @@
}
@Override
- public void exec(TestRunnerAction action, ActionExecutionContext actionExecutionContext)
+ public Set<SpawnResult> exec(
+ TestRunnerAction action, ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException {
Path execRoot = actionExecutionContext.getExecRoot();
Path coverageDir = execRoot.getRelative(action.getCoverageDirectory());
@@ -140,7 +144,7 @@
try {
int maxAttempts = getTestAttempts(action);
- TestResultData data =
+ StandaloneTestResult standaloneTestResult =
executeTestAttempt(
action,
spawn,
@@ -151,11 +155,16 @@
workingDirectory);
int attempt;
for (attempt = 1;
- data.getStatus() != BlazeTestStatus.PASSED && attempt < maxAttempts;
+ standaloneTestResult.testResultData().getStatus() != BlazeTestStatus.PASSED
+ && attempt < maxAttempts;
attempt++) {
processFailedTestAttempt(
- attempt, actionExecutionContext, action, dataBuilder, data);
- data =
+ attempt,
+ actionExecutionContext,
+ action,
+ dataBuilder,
+ standaloneTestResult.testResultData());
+ standaloneTestResult =
executeTestAttempt(
action,
spawn,
@@ -165,7 +174,7 @@
tmpDir,
workingDirectory);
}
- processLastTestAttempt(attempt, dataBuilder, data);
+ processLastTestAttempt(attempt, dataBuilder, standaloneTestResult.testResultData());
ImmutableList.Builder<Pair<String, Path>> testOutputsBuilder = new ImmutableList.Builder<>();
if (action.getTestLog().getPath().exists()) {
testOutputsBuilder.add(
@@ -182,13 +191,17 @@
new TestAttempt(
action,
attempt,
- data.getStatus(),
- data.getStartTimeMillisEpoch(),
- data.getRunDurationMillis(),
+ standaloneTestResult.testResultData().getStatus(),
+ standaloneTestResult.testResultData().getStartTimeMillisEpoch(),
+ standaloneTestResult.testResultData().getRunDurationMillis(),
testOutputsBuilder.build(),
- data.getWarningList(),
+ standaloneTestResult.testResultData().getWarningList(),
true));
finalizeTest(actionExecutionContext, action, dataBuilder.build());
+
+ // TODO(b/62588075): Should we accumulate SpawnResults across test attempts instead of only
+ // returning the last set?
+ return standaloneTestResult.spawnResults();
} catch (IOException e) {
actionExecutionContext.getEventHandler().handle(Event.error("Caught I/O exception: " + e));
throw new EnvironmentalExecException("unexpected I/O exception", e);
@@ -287,7 +300,7 @@
}
}
- private TestResultData executeTestAttempt(
+ private StandaloneTestResult executeTestAttempt(
TestRunnerAction action,
Spawn spawn,
ActionExecutionContext actionExecutionContext,
@@ -301,13 +314,10 @@
try (FileOutErr fileOutErr =
new FileOutErr(
action.getTestLog().getPath(), action.resolve(execRoot).getTestStderr())) {
- TestResultData data =
- executeTest(
- action,
- spawn,
- actionExecutionContext.withFileOutErr(fileOutErr));
+ StandaloneTestResult standaloneTestResult =
+ executeTest(action, spawn, actionExecutionContext.withFileOutErr(fileOutErr));
appendStderr(fileOutErr.getOutputPath(), fileOutErr.getErrorPath());
- return data;
+ return standaloneTestResult;
}
}
@@ -328,11 +338,9 @@
relativeTmpDir);
}
- protected TestResultData executeTest(
- TestRunnerAction action,
- Spawn spawn,
- ActionExecutionContext actionExecutionContext)
- throws ExecException, InterruptedException, IOException {
+ protected StandaloneTestResult executeTest(
+ TestRunnerAction action, Spawn spawn, ActionExecutionContext actionExecutionContext)
+ throws ExecException, InterruptedException, IOException {
Closeable streamed = null;
Path testLogPath = action.getTestLog().getPath();
TestResultData.Builder builder = TestResultData.newBuilder();
@@ -340,6 +348,7 @@
long startTime = actionExecutionContext.getClock().currentTimeMillis();
SpawnActionContext spawnActionContext =
actionExecutionContext.getSpawnActionContext(action.getMnemonic());
+ Set<SpawnResult> spawnResults = ImmutableSet.of();
try {
try {
if (executionOptions.testOutput.equals(TestOutputFormat.STREAMED)) {
@@ -348,7 +357,7 @@
Reporter.outErrForReporter(actionExecutionContext.getEventHandler()),
testLogPath);
}
- spawnActionContext.exec(spawn, actionExecutionContext);
+ spawnResults = spawnActionContext.exec(spawn, actionExecutionContext);
builder
.setTestPassed(true)
@@ -386,7 +395,7 @@
builder.setHasCoverage(true);
}
- return builder.build();
+ return StandaloneTestResult.create(spawnResults, builder.build());
} catch (IOException e) {
throw new TestExecException(e.getMessage());
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
index 2299fae..60fbf2d 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java
@@ -16,12 +16,14 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.devtools.build.lib.actions.AbstractAction;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.BaseSpawn;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ResourceSet;
+import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.analysis.config.BinTools;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -33,6 +35,7 @@
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import java.util.List;
+import java.util.Set;
/**
* Helper class responsible for the symlink tree creation.
@@ -96,18 +99,19 @@
}
/**
- * Creates symlink tree using appropriate method. At this time tree
- * always created using build-runfiles helper application.
+ * Creates symlink tree using appropriate method. At this time tree always created using
+ * build-runfiles helper application.
*
- * Note: method may try to acquire resources - meaning that it would
- * block for undetermined period of time. If it is interrupted during
- * that wait, ExecException will be thrown but interrupted bit will be
- * preserved.
+ * <p>Note: method may try to acquire resources - meaning that it would block for undetermined
+ * period of time. If it is interrupted during that wait, ExecException will be thrown but
+ * interrupted bit will be preserved.
+ *
* @param action action instance that requested symlink tree creation
* @param actionExecutionContext Services that are in the scope of the action.
* @param enableRunfiles
+ * @return a set of SpawnResults created during symlink creation, if any
*/
- public void createSymlinks(
+ public Set<SpawnResult> createSymlinks(
AbstractAction action,
ActionExecutionContext actionExecutionContext,
BinTools binTools,
@@ -118,9 +122,11 @@
List<String> args =
getSpawnArgumentList(
actionExecutionContext.getExecRoot(), binTools);
- actionExecutionContext.getSpawnActionContext(action.getMnemonic()).exec(
- new BaseSpawn.Local(args, shellEnvironment, action, RESOURCE_SET),
- actionExecutionContext);
+ return actionExecutionContext
+ .getSpawnActionContext(action.getMnemonic())
+ .exec(
+ new BaseSpawn.Local(args, shellEnvironment, action, RESOURCE_SET),
+ actionExecutionContext);
} else {
// Pretend we created the runfiles tree by copying the manifest
try {
@@ -129,6 +135,7 @@
} catch (IOException e) {
throw new UserExecException(e.getMessage(), e);
}
+ return ImmutableSet.of();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java
index 80a5c1b..617b8f1 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java
@@ -14,15 +14,18 @@
package com.google.devtools.build.lib.exec;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionExecutionException;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ExecutionStrategy;
+import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.analysis.actions.SymlinkTreeAction;
import com.google.devtools.build.lib.analysis.actions.SymlinkTreeActionContext;
import com.google.devtools.build.lib.analysis.config.BinTools;
import com.google.devtools.build.lib.profiler.AutoProfiler;
import com.google.devtools.build.lib.skyframe.OutputService;
+import java.util.Set;
import java.util.logging.Logger;
/**
@@ -42,7 +45,7 @@
}
@Override
- public void createSymlinks(
+ public Set<SpawnResult> createSymlinks(
SymlinkTreeAction action,
ActionExecutionContext actionExecutionContext,
ImmutableMap<String, String> shellEnvironment,
@@ -60,13 +63,10 @@
action.getOutputManifest().getPath(),
action.isFilesetTree(),
action.getOutputManifest().getExecPath().getParentDirectory());
+ return ImmutableSet.of();
} else {
- helper.createSymlinks(
- action,
- actionExecutionContext,
- binTools,
- shellEnvironment,
- enableRunfiles);
+ return helper.createSymlinks(
+ action, actionExecutionContext, binTools, shellEnvironment, enableRunfiles);
}
} catch (ExecException e) {
throw e.toActionExecutionException(
diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
index 56bde84..dda6046 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
@@ -25,6 +25,7 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.analysis.config.BinTools;
import com.google.devtools.build.lib.analysis.test.TestActionContext;
@@ -55,6 +56,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import javax.annotation.Nullable;
/** A strategy for executing a {@link TestRunnerAction}. */
@@ -146,7 +148,8 @@
}
@Override
- public abstract void exec(TestRunnerAction action, ActionExecutionContext actionExecutionContext)
+ public abstract Set<SpawnResult> exec(
+ TestRunnerAction action, ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException;
/**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 8c09e6c..1ac5cf2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -1164,8 +1164,12 @@
actionExecutionContext.getFileOutErr().setErrorFilter(showIncludesFilterForStderr);
}
try {
- reply = actionExecutionContext.getContext(actionContext)
- .execWithReply(this, actionExecutionContext);
+ CppCompileActionResult cppCompileActionResult =
+ actionExecutionContext
+ .getContext(actionContext)
+ .execWithReply(this, actionExecutionContext);
+ // TODO(b/62588075) Save spawnResults from cppCompileActionResult and return them upwards.
+ reply = cppCompileActionResult.contextReply();
} catch (ExecException e) {
throw e.toActionExecutionException(
"C++ compilation of rule '" + getOwner().getLabel() + "'",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionContext.java
index 1f2e536..5e890d6 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionContext.java
@@ -51,7 +51,10 @@
/**
* Executes the given action and return the reply of the executor.
+ *
+ * @return a CppCompileActionResult with information resulting from the action's execution
*/
- Reply execWithReply(CppCompileAction action,
- ActionExecutionContext actionExecutionContext) throws ExecException, InterruptedException;
+ CppCompileActionResult execWithReply(
+ CppCompileAction action, ActionExecutionContext actionExecutionContext)
+ throws ExecException, InterruptedException;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionResult.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionResult.java
new file mode 100644
index 0000000..93e3ae4
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionResult.java
@@ -0,0 +1,68 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.rules.cpp;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.actions.SpawnResult;
+import java.util.Set;
+import javax.annotation.Nullable;
+
+/** Contains information about the result of a CppCompileAction's execution. */
+@AutoValue
+public abstract class CppCompileActionResult {
+
+ /** Returns the SpawnResults created by the action, if any. */
+ public abstract Set<SpawnResult> spawnResults();
+
+ /**
+ * Gets the optional CppCompileActionContext.Reply for the action.
+ *
+ * <p>Could be null if there is no reply (e.g. if there is no .d file documenting which #include
+ * statements are actually required.)
+ */
+ @Nullable
+ public abstract CppCompileActionContext.Reply contextReply();
+
+ /** Returns a builder that can be used to construct a {@link CppCompileActionResult} object. */
+ public static Builder builder() {
+ return new AutoValue_CppCompileActionResult.Builder();
+ }
+
+ /** Builder for a {@link CppCompileActionResult} instance, which is immutable once built. */
+ @AutoValue.Builder
+ public abstract static class Builder {
+
+ /** Returns the SpawnResults for the action, if any. */
+ abstract Set<SpawnResult> spawnResults();
+
+ /** Sets the SpawnResults for the action. */
+ public abstract Builder setSpawnResults(Set<SpawnResult> spawnResults);
+
+ /** Sets the CppCompileActionContext.Reply for the action. */
+ public abstract Builder setContextReply(CppCompileActionContext.Reply reply);
+
+ abstract CppCompileActionResult realBuild();
+
+ /**
+ * Returns an immutable CppCompileActionResult object.
+ *
+ * <p>The set of SpawnResults is also made immutable here.
+ */
+ public CppCompileActionResult build() {
+ return this.setSpawnResults(ImmutableSet.copyOf(spawnResults())).realBuild();
+ }
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
index 1daec43..dc36e72 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
@@ -129,7 +129,10 @@
CppCompileActionContext context = actionExecutionContext.getContext(actionContext);
CppCompileActionContext.Reply reply = null;
try {
- reply = context.execWithReply(this, actionExecutionContext);
+ CppCompileActionResult cppCompileActionResult =
+ context.execWithReply(this, actionExecutionContext);
+ // TODO(b/62588075) Save spawnResults from cppCompileActionResult and return them upwards.
+ reply = cppCompileActionResult.contextReply();
} catch (ExecException e) {
throw e.toActionExecutionException(
"C++ compilation of rule '" + getOwner().getLabel() + "'",
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 b53d38b..2192c3f 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
@@ -161,12 +161,6 @@
}
@Override
- public void execute(ActionExecutionContext actionExecutionContext)
- throws ActionExecutionException, InterruptedException {
- super.execute(actionExecutionContext);
- }
-
- @Override
protected String computeKey() {
Fingerprint f = new Fingerprint();
f.addString(GUID);
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 f24be92..a5a82f1 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
@@ -25,7 +25,9 @@
import com.google.devtools.build.lib.actions.SimpleSpawn;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnActionContext;
+import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.UserExecException;
+import java.util.Set;
/**
* A context for C++ compilation that calls into a {@link SpawnActionContext}.
@@ -45,7 +47,7 @@
}
@Override
- public CppCompileActionContext.Reply execWithReply(
+ public CppCompileActionResult execWithReply(
CppCompileAction action, ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException {
if (action.getDotdFile() != null && action.getDotdFile().artifact() == null) {
@@ -65,9 +67,10 @@
action.getOutputs().asList(),
action.estimateResourceConsumptionLocal());
- actionExecutionContext
- .getSpawnActionContext(action.getMnemonic())
- .exec(spawn, actionExecutionContext);
- return null;
+ Set<SpawnResult> spawnResults =
+ actionExecutionContext
+ .getSpawnActionContext(action.getMnemonic())
+ .exec(spawn, actionExecutionContext);
+ return CppCompileActionResult.builder().setSpawnResults(spawnResults).build();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/ExclusiveTestStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/test/ExclusiveTestStrategy.java
index 8932660..dd127ca 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/ExclusiveTestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/ExclusiveTestStrategy.java
@@ -16,12 +16,14 @@
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ExecutionStrategy;
+import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.analysis.test.TestActionContext;
import com.google.devtools.build.lib.analysis.test.TestResult;
import com.google.devtools.build.lib.analysis.test.TestRunnerAction;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.view.test.TestStatus.TestResultData;
import java.io.IOException;
+import java.util.Set;
/**
* Test strategy wrapper called 'exclusive'. It should delegate to a test strategy for local
@@ -39,9 +41,10 @@
}
@Override
- public void exec(TestRunnerAction action,
- ActionExecutionContext actionExecutionContext) throws ExecException, InterruptedException {
- parent.exec(action, actionExecutionContext);
+ public Set<SpawnResult> exec(
+ TestRunnerAction action, ActionExecutionContext actionExecutionContext)
+ throws ExecException, InterruptedException {
+ return parent.exec(action, actionExecutionContext);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java
index d247d4e..94d1c11 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java
@@ -20,14 +20,11 @@
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
-import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.analysis.test.TestConfiguration;
import com.google.devtools.build.lib.exec.SpawnInputExpander;
import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy;
import com.google.devtools.build.lib.rules.fileset.FilesetActionContext;
-import com.google.devtools.build.lib.standalone.StandaloneSpawnStrategy;
-import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.common.options.OptionsProvider;
@@ -40,15 +37,6 @@
/** Helper methods that are shared by the different sandboxing strategies in this package. */
public final class SandboxHelpers {
- static void fallbackToNonSandboxedExecution(
- Spawn spawn, ActionExecutionContext actionExecutionContext)
- throws ExecException, InterruptedException {
- StandaloneSpawnStrategy standaloneStrategy =
- Preconditions.checkNotNull(
- actionExecutionContext.getContext(StandaloneSpawnStrategy.class));
- standaloneStrategy.exec(spawn, actionExecutionContext);
- }
-
/**
* Returns the inputs of a Spawn as a map of PathFragments relative to an execRoot to paths in the
* host filesystem where the input files can be found.
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java
index e9d7dee..e06b6c4 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java
@@ -30,6 +30,7 @@
import com.google.devtools.build.lib.analysis.test.TestRunnerAction;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.exec.ExecutionOptions;
+import com.google.devtools.build.lib.exec.StandaloneTestResult;
import com.google.devtools.build.lib.exec.StandaloneTestStrategy;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.vfs.Path;
@@ -76,10 +77,8 @@
}
@Override
- protected TestResultData executeTest(
- TestRunnerAction action,
- Spawn spawn,
- ActionExecutionContext actionExecutionContext)
+ protected StandaloneTestResult executeTest(
+ TestRunnerAction action, Spawn spawn, ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException, IOException {
if (!action.getConfiguration().compatibleWithStrategy("experimental_worker")) {
throw new UserExecException(
@@ -108,7 +107,7 @@
actionExecutionContext.getExecRoot());
}
- private TestResultData execInWorker(
+ private StandaloneTestResult execInWorker(
TestRunnerAction action,
Spawn spawn,
ActionExecutionContext actionExecutionContext,
@@ -214,7 +213,7 @@
builder.setTestCase(details);
}
- return builder.build();
+ return StandaloneTestResult.create(ImmutableSet.of(), builder.build());
} catch (IOException | InterruptedException e) {
if (worker != null) {
workerPool.invalidateObject(key, worker);
diff --git a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
new file mode 100644
index 0000000..e4a945c
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
@@ -0,0 +1,132 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.exec;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.when;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.actions.ActionExecutionContext;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.SpawnActionContext;
+import com.google.devtools.build.lib.actions.SpawnResult;
+import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.config.BinTools;
+import com.google.devtools.build.lib.analysis.test.TestProvider;
+import com.google.devtools.build.lib.analysis.test.TestResult;
+import com.google.devtools.build.lib.analysis.test.TestRunnerAction;
+import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
+import com.google.devtools.build.lib.clock.BlazeClock;
+import com.google.devtools.build.lib.vfs.Path;
+import java.io.IOException;
+import java.util.List;
+import java.util.Set;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+/** Unit tests for {@link StandaloneTestStrategy}. */
+@RunWith(JUnit4.class)
+public final class StandaloneTestStrategyTest extends BuildViewTestCase {
+
+ private static class TestedStandaloneTestStrategy extends StandaloneTestStrategy {
+ public TestedStandaloneTestStrategy(
+ ExecutionOptions executionOptions, BinTools binTools, Path tmpDirRoot) {
+ super(executionOptions, binTools, tmpDirRoot);
+ }
+
+ @Override
+ protected void postTestResult(ActionExecutionContext actionExecutionContext, TestResult result)
+ throws IOException {
+ // Make postTestResult a no-op for testing purposes
+ }
+ }
+
+ @Mock private ActionExecutionContext actionExecutionContext;
+
+ @Mock private SpawnActionContext spawnActionContext;
+
+ @Before
+ public final void setUp() throws Exception {
+ MockitoAnnotations.initMocks(this);
+ }
+
+ @Test
+ public void testSpawnResultsAreReturned() throws Exception {
+
+ // setup a StandaloneTestStrategy
+
+ ExecutionOptions executionOptions = ExecutionOptions.DEFAULTS;
+ Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
+ TestedStandaloneTestStrategy standaloneTestStrategy =
+ new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot);
+
+ // setup a test action
+
+ scratch.file(
+ "standalone/simple_test.sh", "echo \"All tests passed, you are awesome!\"", "exit 0");
+
+ scratch.file(
+ "standalone/BUILD",
+ "sh_test(",
+ " name = \"simple_test\",",
+ " size = \"small\",",
+ " srcs = [\"simple_test.sh\"],",
+ ")");
+
+ ConfiguredTarget configuredTarget = getConfiguredTarget("//standalone:simple_test");
+ List<Artifact> testStatusArtifacts =
+ configuredTarget.getProvider(TestProvider.class).getTestParams().getTestStatusArtifacts();
+ Artifact testStatusArtifact = Iterables.getOnlyElement(testStatusArtifacts);
+ TestRunnerAction testRunnerAction = (TestRunnerAction) getGeneratingAction(testStatusArtifact);
+
+ // setup a mock ActionExecutionContext
+
+ when(actionExecutionContext.getClock()).thenReturn(BlazeClock.instance());
+ when(actionExecutionContext.withFileOutErr(any())).thenReturn(actionExecutionContext);
+ when(actionExecutionContext.getExecRoot()).thenReturn(outputBase.getRelative("execroot"));
+ when(actionExecutionContext.getClientEnv()).thenReturn(ImmutableMap.of());
+ when(actionExecutionContext.getEventHandler()).thenReturn(reporter);
+ when(actionExecutionContext.getEventBus()).thenReturn(eventBus);
+
+ long expectedWallTimeMillis = 10;
+ SpawnResult expectedSpawnResult =
+ new SpawnResult.Builder()
+ .setStatus(SpawnResult.Status.SUCCESS)
+ .setWallTimeMillis(expectedWallTimeMillis)
+ .build();
+ when(spawnActionContext.exec(any(), any())).thenReturn(ImmutableSet.of(expectedSpawnResult));
+
+ when(actionExecutionContext.getSpawnActionContext(any())).thenReturn(spawnActionContext);
+
+ // actual StandaloneTestStrategy execution
+
+ Set<SpawnResult> spawnResults =
+ standaloneTestStrategy.exec(testRunnerAction, actionExecutionContext);
+
+ // check that the rigged SpawnResult was returned
+
+ assertThat(spawnResults).containsExactly(expectedSpawnResult);
+ SpawnResult spawnResult = Iterables.getOnlyElement(spawnResults);
+ assertThat(spawnResult.status()).isEqualTo(SpawnResult.Status.SUCCESS);
+ assertThat(spawnResult.getWallTimeMillis()).isEqualTo(expectedWallTimeMillis);
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
index 85fd065..5372dcd 100644
--- a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
@@ -32,6 +32,7 @@
import com.google.devtools.build.lib.actions.ResourceSet;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnActionContext;
+import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ServerDirectories;
@@ -59,6 +60,7 @@
import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Set;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -167,9 +169,10 @@
assertThat(err()).isEmpty();
}
- private void run(Spawn spawn) throws Exception {
- executor.getSpawnActionContext(spawn.getMnemonic()).exec(spawn, createContext());
+ private Set<SpawnResult> run(Spawn spawn) throws Exception {
+ return executor.getSpawnActionContext(spawn.getMnemonic()).exec(spawn, createContext());
}
+
private ActionExecutionContext createContext() {
Path execRoot = executor.getExecRoot();
return new ActionExecutionContext(
