Encode RunCommand's loading failures with FailureDetails RunCommand adds a validation step to the loading phase. Validation failures result in LoadingFailedException. This CL encodes those validation failure modes using FailureDetails. RELNOTES: None. PiperOrigin-RevId: 332878473
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java index 53feadb..9cc3efc 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -433,7 +433,10 @@ } catch (TargetParsingException e) { detailedExitCode = e.getDetailedExitCode(); reportExceptionError(e); - } catch (LoadingFailedException | ViewCreationFailedException e) { + } catch (LoadingFailedException e) { + detailedExitCode = e.getDetailedExitCode(); + reportExceptionError(e); + } catch (ViewCreationFailedException e) { detailedExitCode = DetailedExitCode.justExitCode(ExitCode.PARSING_FAILURE); reportExceptionError(e); } catch (ExitException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/BUILD b/src/main/java/com/google/devtools/build/lib/pkgcache/BUILD index 4ca1895..0cca8da 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/BUILD +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/BUILD
@@ -25,6 +25,7 @@ "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/packages", + "//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code", "//src/main/java/com/google/devtools/build/lib/util:resource_converter",
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingFailedException.java b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingFailedException.java index a5f5fdb..21fcf63 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingFailedException.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingFailedException.java
@@ -13,17 +13,24 @@ // limitations under the License. package com.google.devtools.build.lib.pkgcache; -/** - * An exception indicating that there was a problem during the loading phase for one or more - * targets in such a way that the build cannot proceed (for example because keep_going is disabled). - */ -public class LoadingFailedException extends Exception { +import com.google.devtools.build.lib.skyframe.DetailedException; +import com.google.devtools.build.lib.util.DetailedExitCode; - public LoadingFailedException(String message) { +/** + * An exception indicating that there was a problem during the loading phase for one or more targets + * in such a way that the build cannot proceed (for example because keep_going is disabled). + */ +public class LoadingFailedException extends Exception implements DetailedException { + + private final DetailedExitCode detailedExitCode; + + public LoadingFailedException(String message, DetailedExitCode detailedExitCode) { super(message); + this.detailedExitCode = detailedExitCode; } - public LoadingFailedException(String message, Throwable cause) { - super(message + ": " + cause.getMessage(), cause); + @Override + public DetailedExitCode getDetailedExitCode() { + return detailedExitCode; } }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java index a6c0a01..af50237 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
@@ -76,6 +76,7 @@ import com.google.devtools.build.lib.shell.ShellUtils; import com.google.devtools.build.lib.util.CommandDescriptionForm; import com.google.devtools.build.lib.util.CommandFailureUtils; +import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.util.InterruptedFailureDetails; import com.google.devtools.build.lib.util.OS; @@ -571,11 +572,7 @@ private static BlazeCommandResult reportAndCreateFailureResult( CommandEnvironment env, String message, Code detailedCode) { env.getReporter().handle(Event.error(message)); - return BlazeCommandResult.failureDetail( - FailureDetail.newBuilder() - .setMessage(message) - .setRunCommand(FailureDetails.RunCommand.newBuilder().setCode(detailedCode)) - .build()); + return BlazeCommandResult.failureDetail(createFailureDetail(message, detailedCode)); } /** @@ -672,13 +669,14 @@ makeErrorMessageForNotHavingASingleTarget( targetPatternStrings.get(0), Iterables.transform(targets, t -> t.getLabel().toString())), - keepGoing); + keepGoing, + Code.TOO_MANY_TARGETS_SPECIFIED); singleTargetWarningWasOutput = true; } for (Target target : targets) { - String targetError = validateTarget(target); - if (targetError != null) { - warningOrException(reporter, targetError, keepGoing); + if (!isExecutable(target)) { + warningOrException( + reporter, notExecutableError(target), keepGoing, Code.TARGET_NOT_EXECUTABLE); } if (currentRunUnder != null && target.getLabel().equals(currentRunUnder.getLabel())) { @@ -694,7 +692,8 @@ makeErrorMessageForNotHavingASingleTarget( targetPatternStrings.get(0), Iterables.transform(targets, t -> t.getLabel().toString())), - keepGoing); + keepGoing, + Code.TOO_MANY_TARGETS_SPECIFIED); } return; } @@ -704,18 +703,22 @@ targetToRun = runUnderTarget; } if (targetToRun == null) { - warningOrException(reporter, NO_TARGET_MESSAGE, keepGoing); + warningOrException(reporter, NO_TARGET_MESSAGE, keepGoing, Code.NO_TARGET_SPECIFIED); } } - // If keepGoing, print a warning and return the given collection. - // Otherwise, throw InvalidTargetException. - private void warningOrException(Reporter reporter, String message, - boolean keepGoing) throws LoadingFailedException { + /** + * If keepGoing, print a warning and return the given collection. Otherwise, throw + * InvalidTargetException. + */ + private void warningOrException( + Reporter reporter, String message, boolean keepGoing, Code detailedCode) + throws LoadingFailedException { if (keepGoing) { reporter.handle(Event.warn(message + ". Will continue anyway")); } else { - throw new LoadingFailedException(message); + throw new LoadingFailedException( + message, DetailedExitCode.of(createFailureDetail(message, detailedCode))); } } @@ -723,13 +726,6 @@ return "Cannot run target " + target.getLabel() + ": Not executable"; } - /** Returns null if the target is a runnable rule, or an appropriate error message otherwise. */ - private static String validateTarget(Target target) { - return isExecutable(target) - ? null - : notExecutableError(target); - } - /** * Performs all available validation checks on an individual target. * @@ -754,10 +750,9 @@ throw new IllegalStateException("Failed to find a target to validate", e); } - String targetError = validateTarget(target); - - if (targetError != null) { - return reportAndCreateFailureResult(env, targetError, Code.TARGET_NOT_EXECUTABLE); + if (!isExecutable(target)) { + return reportAndCreateFailureResult( + env, notExecutableError(target), Code.TARGET_NOT_EXECUTABLE); } Artifact executable = @@ -835,6 +830,13 @@ truncateTargetNameList ? "[TRUNCATED]" : ""); } + private static FailureDetail createFailureDetail(String message, Code detailedCode) { + return FailureDetail.newBuilder() + .setMessage(message) + .setRunCommand(FailureDetails.RunCommand.newBuilder().setCode(detailedCode)) + .build(); + } + private static class RunfilesException extends Exception { private final FailureDetails.RunCommand.Code detailedCode; @@ -844,10 +846,7 @@ } private FailureDetail createFailureDetail() { - return FailureDetail.newBuilder() - .setMessage(getMessage()) - .setRunCommand(FailureDetails.RunCommand.newBuilder().setCode(detailedCode)) - .build(); + return RunCommand.createFailureDetail(getMessage(), detailedCode); } } }