From 4050a89a1e3fb6a9a9c6bd080cf4e3081a9f2012 Mon Sep 17 00:00:00 2001 From: ruperts Date: Sat, 7 Oct 2017 00:46:20 +0200 Subject: 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 --- .../analysis/actions/FileWriteActionContext.java | 7 ++- .../analysis/actions/SymlinkTreeActionContext.java | 6 +- .../build/lib/analysis/test/TestActionContext.java | 12 ++-- .../devtools/build/lib/exec/FileWriteStrategy.java | 7 ++- .../build/lib/exec/StandaloneTestResult.java | 68 ++++++++++++++++++++++ .../build/lib/exec/StandaloneTestStrategy.java | 57 ++++++++++-------- .../devtools/build/lib/exec/SymlinkTreeHelper.java | 27 +++++---- .../build/lib/exec/SymlinkTreeStrategy.java | 14 ++--- .../devtools/build/lib/exec/TestStrategy.java | 5 +- .../build/lib/rules/cpp/CppCompileAction.java | 8 ++- .../lib/rules/cpp/CppCompileActionContext.java | 7 ++- .../lib/rules/cpp/CppCompileActionResult.java | 68 ++++++++++++++++++++++ .../build/lib/rules/cpp/FakeCppCompileAction.java | 5 +- .../build/lib/rules/cpp/LtoBackendAction.java | 6 -- .../build/lib/rules/cpp/SpawnGccStrategy.java | 13 +++-- .../lib/rules/test/ExclusiveTestStrategy.java | 9 ++- .../devtools/build/lib/sandbox/SandboxHelpers.java | 12 ---- .../build/lib/worker/WorkerTestStrategy.java | 11 ++-- 18 files changed, 256 insertions(+), 86 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/exec/StandaloneTestResult.java create mode 100644 src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionResult.java (limited to 'src/main/java/com/google/devtools/build/lib') 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 bff6530f3b..4a2a19b1ae 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 @@ package com.google.devtools.build.lib.analysis.actions; 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 @@ public interface FileWriteActionContext extends ActionContext { /** * 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 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 1ece4b0066..7ea2e2bbe2 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.common.collect.ImmutableMap; 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 @@ public interface SymlinkTreeActionContext extends ActionContext { /** * Creates the symlink tree. + * + * @return a set of SpawnResults created during symlink creation, if any */ - void createSymlinks( + Set createSymlinks( SymlinkTreeAction action, ActionExecutionContext actionExecutionContext, ImmutableMap 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 cc483f6ab6..44ae134d9d 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 @@ package com.google.devtools.build.lib.analysis.test; 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 @@ import java.io.IOException; 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 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 2ada360baa..74df515739 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 com.google.devtools.build.lib.vfs.Path; 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 @@ public final class FileWriteStrategy implements FileWriteActionContext { } @Override - public void exec(AbstractFileWriteAction action, ActionExecutionContext actionExecutionContext) + public Set 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 @@ public final class FileWriteStrategy implements FileWriteActionContext { + "' 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 0000000000..b4fab1fff7 --- /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 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 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 spawnResults(); + + /** Sets the SpawnResults for the test. */ + public abstract Builder setSpawnResults(Set spawnResults); + + /** Sets the TestResultData for the test. */ + public abstract Builder setTestResultData(TestResultData testResultData); + + abstract StandaloneTestResult realBuild(); + + /** + * Returns an immutable StandaloneTestResult object. + * + *

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 1ff1e06abd..d668a02f01 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 @@ package com.google.devtools.build.lib.exec; 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.ResourceSet; 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 com.google.devtools.build.lib.view.test.TestStatus.TestResultData.Builder import java.io.Closeable; import java.io.IOException; import java.util.Map; +import java.util.Set; /** Runs TestRunnerAction actions. */ @ExecutionStrategy( @@ -85,7 +88,8 @@ public class StandaloneTestStrategy extends TestStrategy { } @Override - public void exec(TestRunnerAction action, ActionExecutionContext actionExecutionContext) + public Set exec( + TestRunnerAction action, ActionExecutionContext actionExecutionContext) throws ExecException, InterruptedException { Path execRoot = actionExecutionContext.getExecRoot(); Path coverageDir = execRoot.getRelative(action.getCoverageDirectory()); @@ -140,7 +144,7 @@ public class StandaloneTestStrategy extends TestStrategy { try { int maxAttempts = getTestAttempts(action); - TestResultData data = + StandaloneTestResult standaloneTestResult = executeTestAttempt( action, spawn, @@ -151,11 +155,16 @@ public class StandaloneTestStrategy extends TestStrategy { 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 @@ public class StandaloneTestStrategy extends TestStrategy { tmpDir, workingDirectory); } - processLastTestAttempt(attempt, dataBuilder, data); + processLastTestAttempt(attempt, dataBuilder, standaloneTestResult.testResultData()); ImmutableList.Builder> testOutputsBuilder = new ImmutableList.Builder<>(); if (action.getTestLog().getPath().exists()) { testOutputsBuilder.add( @@ -182,13 +191,17 @@ public class StandaloneTestStrategy extends TestStrategy { 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 @@ public class StandaloneTestStrategy extends TestStrategy { } } - private TestResultData executeTestAttempt( + private StandaloneTestResult executeTestAttempt( TestRunnerAction action, Spawn spawn, ActionExecutionContext actionExecutionContext, @@ -301,13 +314,10 @@ public class StandaloneTestStrategy extends TestStrategy { 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 @@ public class StandaloneTestStrategy extends TestStrategy { 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 @@ public class StandaloneTestStrategy extends TestStrategy { long startTime = actionExecutionContext.getClock().currentTimeMillis(); SpawnActionContext spawnActionContext = actionExecutionContext.getSpawnActionContext(action.getMnemonic()); + Set spawnResults = ImmutableSet.of(); try { try { if (executionOptions.testOutput.equals(TestOutputFormat.STREAMED)) { @@ -348,7 +357,7 @@ public class StandaloneTestStrategy extends TestStrategy { Reporter.outErrForReporter(actionExecutionContext.getEventHandler()), testLogPath); } - spawnActionContext.exec(spawn, actionExecutionContext); + spawnResults = spawnActionContext.exec(spawn, actionExecutionContext); builder .setTestPassed(true) @@ -386,7 +395,7 @@ public class StandaloneTestStrategy extends TestStrategy { 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 2299faee87..60fbf2dfac 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 @@ package com.google.devtools.build.lib.exec; 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.Path; 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 @@ public final class SymlinkTreeHelper { } /** - * 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. * - * 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 createSymlinks( AbstractAction action, ActionExecutionContext actionExecutionContext, BinTools binTools, @@ -118,9 +122,11 @@ public final class SymlinkTreeHelper { List 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 @@ public final class SymlinkTreeHelper { } 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 80a5c1bda1..617b8f1d84 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 @@ public final class SymlinkTreeStrategy implements SymlinkTreeActionContext { } @Override - public void createSymlinks( + public Set createSymlinks( SymlinkTreeAction action, ActionExecutionContext actionExecutionContext, ImmutableMap shellEnvironment, @@ -60,13 +63,10 @@ public final class SymlinkTreeStrategy implements SymlinkTreeActionContext { 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 56bde845bc..dda60463c6 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.ActionExecutionContext; 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.Arrays; 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 @@ public abstract class TestStrategy implements TestActionContext { } @Override - public abstract void exec(TestRunnerAction action, ActionExecutionContext actionExecutionContext) + public abstract Set 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 8c09e6c8cb..1ac5cf2ecc 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 @@ public class CppCompileAction extends AbstractAction 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 1f2e5369b8..5e890d6dcd 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 @@ public interface CppCompileActionContext extends ActionContext { /** * 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 0000000000..93e3ae4ef8 --- /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 spawnResults(); + + /** + * Gets the optional CppCompileActionContext.Reply for the action. + * + *

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 spawnResults(); + + /** Sets the SpawnResults for the action. */ + public abstract Builder setSpawnResults(Set spawnResults); + + /** Sets the CppCompileActionContext.Reply for the action. */ + public abstract Builder setContextReply(CppCompileActionContext.Reply reply); + + abstract CppCompileActionResult realBuild(); + + /** + * Returns an immutable CppCompileActionResult object. + * + *

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 1daec4300b..dc36e72f69 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 @@ public class FakeCppCompileAction extends CppCompileAction { 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 b53d38bbfd..2192c3f8cb 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 @@ -160,12 +160,6 @@ public final class LtoBackendAction extends SpawnAction { return bitcodeFiles.values(); } - @Override - public void execute(ActionExecutionContext actionExecutionContext) - throws ActionExecutionException, InterruptedException { - super.execute(actionExecutionContext); - } - @Override protected String computeKey() { Fingerprint f = new Fingerprint(); 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 f24be92101..a5a82f1b13 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.ExecutionStrategy; 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 @@ public class SpawnGccStrategy implements CppCompileActionContext { } @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 @@ public class SpawnGccStrategy implements CppCompileActionContext { action.getOutputs().asList(), action.estimateResourceConsumptionLocal()); - actionExecutionContext - .getSpawnActionContext(action.getMnemonic()) - .exec(spawn, actionExecutionContext); - return null; + Set 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 8932660a56..dd127ca8ec 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 @@ package com.google.devtools.build.lib.rules.test; 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 @@ public class ExclusiveTestStrategy implements TestActionContext { } @Override - public void exec(TestRunnerAction action, - ActionExecutionContext actionExecutionContext) throws ExecException, InterruptedException { - parent.exec(action, actionExecutionContext); + public Set 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 d247d4ed5a..94d1c1111c 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.ActionExecutionContext; 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 @@ import java.util.TreeMap; /** 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 e9d7dee66f..e06b6c4f1b 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.TestActionContext; 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 @@ public class WorkerTestStrategy extends StandaloneTestStrategy { } @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 @@ public class WorkerTestStrategy extends StandaloneTestStrategy { actionExecutionContext.getExecRoot()); } - private TestResultData execInWorker( + private StandaloneTestResult execInWorker( TestRunnerAction action, Spawn spawn, ActionExecutionContext actionExecutionContext, @@ -214,7 +213,7 @@ public class WorkerTestStrategy extends StandaloneTestStrategy { builder.setTestCase(details); } - return builder.build(); + return StandaloneTestResult.create(ImmutableSet.of(), builder.build()); } catch (IOException | InterruptedException e) { if (worker != null) { workerPool.invalidateObject(key, worker); -- cgit v1.2.3