Skip to content

Commit

Permalink
Separate ExecException.java from main actions target.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 413105213
  • Loading branch information
wilwell authored and copybara-github committed Nov 30, 2021
1 parent 47ad531 commit 56d624b
Show file tree
Hide file tree
Showing 15 changed files with 102 additions and 101 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -115,6 +116,47 @@ private static NestedSet<Cause> 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;
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/com/google/devtools/build/lib/actions/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
)
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,29 +64,29 @@

/**
* A class to create the coverage report generator action.
*
* <p>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.
*
*
* <p>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.
*
* <p>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.
*
* <p>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}.
*
* <p>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}.
*
*
* <p>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 {

Expand Down Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -136,7 +136,7 @@ public void createSymlinks(
actionExecutionContext.getFileOutErr());
}
} catch (ExecException e) {
throw e.toActionExecutionException(action);
throw ActionExecutionException.fromExecException(e, action);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ private NestedSet<Artifact> 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);
}
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
}
}
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
Loading

0 comments on commit 56d624b

Please sign in to comment.