From 56d624be4dc79a94ed763c07d302c6270bc8c43b Mon Sep 17 00:00:00 2001 From: wilwell Date: Tue, 30 Nov 2021 03:25:47 -0800 Subject: [PATCH] Separate ExecException.java from main actions target. PiperOrigin-RevId: 413105213 --- .../lib/actions/ActionExecutionException.java | 42 +++++++++++++++++ .../google/devtools/build/lib/actions/BUILD | 6 +++ .../build/lib/actions/ExecException.java | 46 ------------------- .../lib/actions/LostInputsExecException.java | 3 +- .../actions/AbstractFileWriteAction.java | 6 +-- .../lib/analysis/actions/SpawnAction.java | 4 +- .../actions/TemplateExpansionAction.java | 3 +- .../lib/analysis/test/TestRunnerAction.java | 13 +++--- .../coverage/CoverageReportActionBuilder.java | 34 +++++++------- .../build/lib/exec/SymlinkTreeStrategy.java | 6 +-- .../build/lib/rules/cpp/CppCompileAction.java | 11 +++-- .../build/lib/rules/cpp/CppLinkAction.java | 5 +- .../lib/rules/java/JavaCompileAction.java | 18 ++++---- .../lib/skyframe/SkyframeActionExecutor.java | 4 +- .../StandaloneSpawnStrategyTest.java | 2 +- 15 files changed, 102 insertions(+), 101 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java index 0e32500cf9f657..613a15187a6d47 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java @@ -24,6 +24,7 @@ import com.google.devtools.build.lib.skyframe.DetailedException; import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.util.ExitCode; +import javax.annotation.Nullable; import net.starlark.java.syntax.Location; /** @@ -115,6 +116,47 @@ private static NestedSet rootCausesFromAction( detailedExitCode)); } + public static ActionExecutionException fromExecException(ExecException exception, Action action) { + return fromExecException(exception, null, action); + } + + /** + * Returns a new ActionExecutionException given an optional action subtask describing which part + * of the action failed (should be null for standard action failures). When appropriate (we use + * some heuristics to decide), produces an abbreviated message incorporating just the termination + * status if available. + * + * @param exception initial ExecException + * @param actionSubtask additional information about the action + * @param action failed action + * @return ActionExecutionException object describing the action failure + */ + public static ActionExecutionException fromExecException( + ExecException exception, @Nullable String actionSubtask, Action action) { + // Message from ActionExecutionException will be prepended with action.describe() where + // necessary: because not all ActionExecutionExceptions come from this codepath, it is safer + // for consumers to manually prepend. We still put action.describe() in the failure detail + // message argument. + String message = + (actionSubtask == null ? "" : actionSubtask + ": ") + + exception.getMessageForActionExecutionException(); + + DetailedExitCode code = + DetailedExitCode.of(exception.getFailureDetail(action.describe() + " failed: " + message)); + + if (exception instanceof LostInputsExecException) { + return ((LostInputsExecException) exception).fromExecException(message, action, code); + } + + return fromExecException(exception, message, action, code); + } + + public static ActionExecutionException fromExecException( + ExecException exception, String message, Action action, DetailedExitCode code) { + return new ActionExecutionException( + message, exception, action, exception.isCatastrophic(), code); + } + /** Returns the action that failed. */ public ActionAnalysisMetadata getAction() { return action; diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD index 691e701f94ff22..79d5b47bc61feb 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD @@ -346,3 +346,9 @@ java_library( "//third_party:error_prone_annotations", ], ) + +java_library( + name = "exec_exception", + srcs = ["ExecException.java"], + deps = ["//src/main/protobuf:failure_details_java_proto"], +) diff --git a/src/main/java/com/google/devtools/build/lib/actions/ExecException.java b/src/main/java/com/google/devtools/build/lib/actions/ExecException.java index 43ba000e3632f3..c3544877031246 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ExecException.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ExecException.java @@ -15,9 +15,6 @@ package com.google.devtools.build.lib.actions; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; -import com.google.devtools.build.lib.util.DetailedExitCode; -import com.google.errorprone.annotations.ForOverride; -import javax.annotation.Nullable; /** * An exception indication that the execution of an action has failed OR could not be attempted OR @@ -82,52 +79,9 @@ public boolean isCatastrophic() { return catastrophe; } - /** - * Returns a new ActionExecutionException without a message prefix. - * - * @param action failed action - * @return ActionExecutionException object describing the action failure - */ - public final ActionExecutionException toActionExecutionException(Action action) { - return toActionExecutionException(null, action); - } - - /** - * Returns a new ActionExecutionException given an optional action subtask describing which part - * of the action failed (should be null for standard action failures). When appropriate (we use - * some heuristics to decide), produces an abbreviated message incorporating just the termination - * status if available. - * - * @param actionSubtask additional information about the action - * @param action failed action - * @return ActionExecutionException object describing the action failure - */ - public final ActionExecutionException toActionExecutionException( - @Nullable String actionSubtask, Action action) { - // Message from ActionExecutionException will be prepended with action.describe() where - // necessary: because not all ActionExecutionExceptions come from this codepath, it is safer - // for consumers to manually prepend. We still put action.describe() in the failure detail - // message argument. - String message = - (actionSubtask == null ? "" : actionSubtask + ": ") - + getMessageForActionExecutionException(); - return toActionExecutionException( - message, - action, - DetailedExitCode.of(getFailureDetail(action.describe() + " failed: " + message))); - } - - @ForOverride - protected ActionExecutionException toActionExecutionException( - String message, Action action, DetailedExitCode code) { - return new ActionExecutionException(message, this, action, isCatastrophic(), code); - } - - @ForOverride protected String getMessageForActionExecutionException() { return getMessage(); } - @ForOverride protected abstract FailureDetail getFailureDetail(String message); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java b/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java index 0be336183d5bef..f992d5d3543e7f 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java +++ b/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java @@ -64,8 +64,7 @@ public ActionInputDepOwners getOwners() { return owners; } - @Override - protected ActionExecutionException toActionExecutionException( + protected ActionExecutionException fromExecException( String message, Action action, DetailedExitCode code) { return new LostInputsActionExecutionException( message, lostInputs, owners, action, /*cause=*/ this, code); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/AbstractFileWriteAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/AbstractFileWriteAction.java index 170f97cafd2f49..a0974b7813d162 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/AbstractFileWriteAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/AbstractFileWriteAction.java @@ -89,16 +89,14 @@ public ActionContinuationOrResult execute() return this; } } catch (ExecException e) { - throw e.toActionExecutionException( - AbstractFileWriteAction.this); + throw ActionExecutionException.fromExecException(e, AbstractFileWriteAction.this); } afterWrite(actionExecutionContext); return ActionContinuationOrResult.of(ActionResult.create(nextContinuation.get())); } }; } catch (ExecException e) { - throw e.toActionExecutionException( - this); + throw ActionExecutionException.fromExecException(e, this); } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java index 0588543f88d03c..2614bd4da63b14 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java @@ -318,7 +318,7 @@ public final ActionContinuationOrResult beginExecution( beforeExecute(actionExecutionContext); spawn = getSpawn(actionExecutionContext); } catch (ExecException e) { - throw e.toActionExecutionException(this); + throw ActionExecutionException.fromExecException(e, this); } catch (CommandLineExpansionException e) { throw createCommandLineException(e); } @@ -1413,7 +1413,7 @@ public ActionContinuationOrResult execute() } return new SpawnActionContinuation(actionExecutionContext, nextContinuation); } catch (ExecException e) { - throw e.toActionExecutionException(SpawnAction.this); + throw ActionExecutionException.fromExecException(e, SpawnAction.this); } } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java index b84902218fd17f..5ac3c526eb5b78 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java @@ -164,8 +164,7 @@ public ActionContinuationOrResult execute() return this; } } catch (ExecException e) { - throw e.toActionExecutionException( - TemplateExpansionAction.this); + throw ActionExecutionException.fromExecException(e, TemplateExpansionAction.this); } return ActionContinuationOrResult.of(ActionResult.create(nextContinuation.get())); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java index 6fe0ecb56631b9..7ccf134bc01a16 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java @@ -956,10 +956,10 @@ public ActionContinuationOrResult beginExecution( testActionContext.isTestKeepGoing(), cancelFuture); } catch (ExecException e) { - throw e.toActionExecutionException(this); + throw ActionExecutionException.fromExecException(e, this); } catch (IOException e) { - throw new EnvironmentalExecException(e, Code.TEST_RUNNER_IO_EXCEPTION) - .toActionExecutionException(this); + throw ActionExecutionException.fromExecException( + new EnvironmentalExecException(e, Code.TEST_RUNNER_IO_EXCEPTION), this); } } @@ -1256,10 +1256,11 @@ public ActionContinuationOrResult execute() Preconditions.checkState(actualMaxAttempts != 0); return process(result, actualMaxAttempts); } catch (ExecException e) { - throw e.toActionExecutionException(this.testRunnerAction); + throw ActionExecutionException.fromExecException(e, this.testRunnerAction); } catch (IOException e) { - throw new EnvironmentalExecException(e, Code.TEST_RUNNER_IO_EXCEPTION) - .toActionExecutionException(this.testRunnerAction); + throw ActionExecutionException.fromExecException( + new EnvironmentalExecException(e, Code.TEST_RUNNER_IO_EXCEPTION), + this.testRunnerAction); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java b/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java index 50709e4055fce8..a50958b8dacf8c 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java @@ -64,29 +64,29 @@ /** * A class to create the coverage report generator action. - * - *

The coverage report action is created after every test shard action is created, at the - * very end of the analysis phase. There is only one coverage report action per coverage - * command invocation. It can also be viewed as a single sink node of the action graph. - * + * + *

The coverage report action is created after every test shard action is created, at the very + * end of the analysis phase. There is only one coverage report action per coverage command + * invocation. It can also be viewed as a single sink node of the action graph. + * *

Its inputs are the individual coverage.dat files from the test outputs (each shard produces - * one) and the baseline coverage artifacts. Note that each ConfiguredTarget among the - * transitive dependencies of the top level test targets may provide baseline coverage artifacts. - * + * one) and the baseline coverage artifacts. Note that each ConfiguredTarget among the transitive + * dependencies of the top level test targets may provide baseline coverage artifacts. + * *

The coverage report generation can have two phases, though they both run in the same action. * The source code of the coverage report tool {@code lcov_merger} is in the {@code - * testing/coverage/lcov_merger} directory. The deployed binaries used by Blaze are under - * {@code tools/coverage}. - * + * testing/coverage/lcov_merger} directory. The deployed binaries used by Blaze are under {@code + * tools/coverage}. + * *

The first phase is merging the individual coverage files into a single report file. The * location of this file is reported by Blaze. This phase always happens if the {@code * --combined_report=lcov} or {@code --combined_report=html}. - * + * *

The second phase is generating an html report. It only happens if {@code - * --combined_report=html}. The action generates an html output file potentially for every - * tested source file into the report. Since this set of files is unknown in the analysis - * phase (the tool figures it out from the contents of the merged coverage report file) - * the action always runs locally when {@code --combined_report=html}. + * --combined_report=html}. The action generates an html output file potentially for every tested + * source file into the report. Since this set of files is unknown in the analysis phase (the tool + * figures it out from the contents of the merged coverage report file) the action always runs + * locally when {@code --combined_report=html}. */ public final class CoverageReportActionBuilder { @@ -147,7 +147,7 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext) actionExecutionContext.getEventHandler().handle(Event.info(locationMessage)); return ActionResult.create(spawnResults); } catch (ExecException e) { - throw e.toActionExecutionException(this); + throw ActionExecutionException.fromExecException(e, this); } } 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 de90cc6e754810..fc5dc61676466f 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 @@ -116,8 +116,8 @@ public void createSymlinks( .createSymlinksDirectly( action.getOutputManifest().getPath().getParentDirectory(), runfiles); } catch (IOException e) { - throw new EnvironmentalExecException(e, Code.SYMLINK_TREE_CREATION_IO_EXCEPTION) - .toActionExecutionException(action); + throw ActionExecutionException.fromExecException( + new EnvironmentalExecException(e, Code.SYMLINK_TREE_CREATION_IO_EXCEPTION), action); } Path inputManifest = @@ -136,7 +136,7 @@ public void createSymlinks( actionExecutionContext.getFileOutErr()); } } catch (ExecException e) { - throw e.toActionExecutionException(action); + throw ActionExecutionException.fromExecException(e, action); } } } 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 d3d33f7109f72c..f723e20833c5dd 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 @@ -454,7 +454,7 @@ private NestedSet findUsedHeaders( createFailureDetail("Find used headers failure", Code.FIND_USED_HEADERS_IO_EXCEPTION)); } } catch (ExecException e) { - throw e.toActionExecutionException("include scanning", this); + throw ActionExecutionException.fromExecException(e, "include scanning", this); } } @@ -1814,7 +1814,7 @@ public ActionContinuationOrResult execute() dotDContents = getDotDContents(spawnResults.get(0)); } catch (ExecException e) { copyTempOutErrToActionOutErr(); - throw e.toActionExecutionException(CppCompileAction.this); + throw ActionExecutionException.fromExecException(e, CppCompileAction.this); } catch (InterruptedException e) { copyTempOutErrToActionOutErr(); throw e; @@ -1892,9 +1892,10 @@ private void copyTempOutErrToActionOutErr() throws ActionExecutionException { } } } catch (IOException e) { - throw new EnvironmentalExecException( - e, createFailureDetail("OutErr copy failure", Code.COPY_OUT_ERR_FAILURE)) - .toActionExecutionException(CppCompileAction.this); + throw ActionExecutionException.fromExecException( + new EnvironmentalExecException( + e, createFailureDetail("OutErr copy failure", Code.COPY_OUT_ERR_FAILURE)), + CppCompileAction.this); } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java index f46dd86ff193fd..383a8e743051f8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java @@ -109,7 +109,7 @@ public Artifact create( }; private static final String LINK_GUID = "58ec78bd-1176-4e36-8143-439f656b181d"; - + @Nullable private final String mnemonic; private final LibraryToLink outputLibrary; private final Artifact linkOutput; @@ -451,8 +451,7 @@ public ActionContinuationOrResult execute() } return ActionContinuationOrResult.of(ActionResult.create(nextContinuation.get())); } catch (ExecException e) { - throw e.toActionExecutionException( - CppLinkAction.this); + throw ActionExecutionException.fromExecException(e, CppLinkAction.this); } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java index c58892f74a5738..bd97fac22ba6c0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java @@ -664,9 +664,10 @@ private Deps.Dependencies readFullOutputDeps( return createFullOutputDeps( Iterables.getOnlyElement(results), outputDepsProto, getInputs(), actionExecutionContext); } catch (IOException e) { - throw new EnvironmentalExecException( - e, createFailureDetail(".jdeps read IOException", Code.JDEPS_READ_IO_EXCEPTION)) - .toActionExecutionException(this); + throw ActionExecutionException.fromExecException( + new EnvironmentalExecException( + e, createFailureDetail(".jdeps read IOException", Code.JDEPS_READ_IO_EXCEPTION)), + this); } } @@ -742,12 +743,13 @@ public ActionContinuationOrResult execute() // We don't create any tree artifacts anyway. /*cleanupArchivedArtifacts=*/ false); } catch (IOException e) { - throw new EnvironmentalExecException( + throw ActionExecutionException.fromExecException( + new EnvironmentalExecException( e, createFailureDetail( "Failed to delete reduced action outputs", - Code.REDUCED_CLASSPATH_FALLBACK_CLEANUP_FAILURE)) - .toActionExecutionException(JavaCompileAction.this); + Code.REDUCED_CLASSPATH_FALLBACK_CLEANUP_FAILURE)), + JavaCompileAction.this); } actionExecutionContext.getMetadataHandler().resetOutputs(getOutputs()); Spawn spawn; @@ -764,7 +766,7 @@ public ActionContinuationOrResult execute() return new JavaFallbackActionContinuation( actionExecutionContext, results, fallbackContinuation); } catch (ExecException e) { - throw e.toActionExecutionException(JavaCompileAction.this); + throw ActionExecutionException.fromExecException(e, JavaCompileAction.this); } } } @@ -808,7 +810,7 @@ public ActionContinuationOrResult execute() ActionResult.create( ImmutableList.copyOf(Iterables.concat(primaryResults, fallbackResults)))); } catch (ExecException e) { - throw e.toActionExecutionException(JavaCompileAction.this); + throw ActionExecutionException.fromExecException(e, JavaCompileAction.this); } } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index 3183f0b00af2a7..c8ee039fda029f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java @@ -617,7 +617,7 @@ Token checkActionCache( remoteDefaultProperties, isRemoteCacheEnabled); } catch (UserExecException e) { - throw e.toActionExecutionException(action); + throw ActionExecutionException.fromExecException(e, action); } if (token == null) { boolean eventPosted = false; @@ -709,7 +709,7 @@ void updateActionCache( ? remoteOptions.getRemoteDefaultExecProperties() : ImmutableSortedMap.of(); } catch (UserExecException e) { - throw e.toActionExecutionException(action); + throw ActionExecutionException.fromExecException(e, action); } try { 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 a94299c3b43296..2bdfb5a52f978d 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 @@ -321,7 +321,7 @@ public void testStandardError() throws Exception { public void testVerboseFailures() { ExecException e = assertThrows(ExecException.class, () -> run(createSpawn(getFalseCommand()))); ActionExecutionException actionExecutionException = - e.toActionExecutionException(new NullAction()); + ActionExecutionException.fromExecException(e, new NullAction()); assertWithMessage("got: " + actionExecutionException.getMessage()) .that(actionExecutionException.getMessage().contains("failed: error executing command")) .isTrue();