Remove undetailed BuildFailedException constructors Encodes execution-phase cycle and non-action failures with FailureDetails. Drive-by detailing of a new CleanCommand failure mode. RELNOTES: None. PiperOrigin-RevId: 318347785
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BuildFailedException.java b/src/main/java/com/google/devtools/build/lib/actions/BuildFailedException.java index ddc3dad..8969094 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BuildFailedException.java +++ b/src/main/java/com/google/devtools/build/lib/actions/BuildFailedException.java
@@ -45,10 +45,6 @@ private final boolean errorAlreadyShown; private final DetailedExitCode detailedExitCode; - public BuildFailedException() { - this(null); - } - public BuildFailedException(String message) { this( message, @@ -67,15 +63,6 @@ detailedExitCode); } - public BuildFailedException(String message, boolean catastrophic) { - this( - message, - catastrophic, - NestedSetBuilder.emptySet(Order.STABLE_ORDER), - /*errorAlreadyShown=*/ false, - DetailedExitCode.justExitCode(ExitCode.BUILD_FAILURE)); - } - public BuildFailedException( String message, boolean catastrophic,
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java index 89490aa..3d99f98 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
@@ -41,6 +41,9 @@ import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.runtime.KeepGoingOption; +import com.google.devtools.build.lib.server.FailureDetails.Execution; +import com.google.devtools.build.lib.server.FailureDetails.Execution.Code; +import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.skyframe.ActionExecutionInactivityWatchdog; import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.lib.skyframe.Builder; @@ -50,7 +53,6 @@ import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.util.DetailedExitCode.DetailedExitCodeComparator; -import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.LoggingUtil; import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.skyframe.CycleInfo; @@ -229,13 +231,9 @@ return; } - if (options.getOptions(KeepGoingOption.class).keepGoing) { - // Use the exit code with the highest priority. - throw new BuildFailedException( - null, Collections.max(detailedExitCodes, DetailedExitCodeComparator.INSTANCE)); - } else { - throw new BuildFailedException(); - } + // Use the exit code with the highest priority. + throw new BuildFailedException( + null, Collections.max(detailedExitCodes, DetailedExitCodeComparator.INSTANCE)); } /** @@ -247,8 +245,8 @@ * <li>{@code null}, if {@code result} had no errors * <li>{@code e} if result had errors and one of them specified a {@link DetailedExitCode} value * {@code e} - * <li>{@code DetailedExitCode.justExitCode(ExitCode.BUILD_FAILURE)} if result had errors but - * none specified a {@link DetailedExitCode} value + * <li>a {@link DetailedExitCode} with {@link Code.NON_ACTION_EXECUTION_FAILURE} if result had + * errors but none specified a {@link DetailedExitCode} value * </ol> * * <p>Throws on catastrophic failures and, if !keepGoing, on any failure. @@ -289,7 +287,9 @@ return detailedExitCode != null ? detailedExitCode - : DetailedExitCode.justExitCode(ExitCode.BUILD_FAILURE); + : createDetailedExitCode( + "keep_going execution failed without an action failure", + Code.NON_ACTION_EXECUTION_FAILURE); } ErrorInfo errorInfo = Preconditions.checkNotNull(result.getError(), result); Exception exception = errorInfo.getException(); @@ -299,7 +299,8 @@ // during evaluation (otherwise, it wouldn't have bothered to find a cycle). So the best // we can do is throw a generic build failure exception, since we've already reported the // cycles above. - throw new BuildFailedException(null, /* catastrophic= */ false); + throw new BuildFailedException( + null, createDetailedExitCode("cycle found during execution", Code.CYCLE)); } else { rethrow(exception); } @@ -355,4 +356,11 @@ return count; } + private static DetailedExitCode createDetailedExitCode(String message, Code detailedCode) { + return DetailedExitCode.of( + FailureDetail.newBuilder() + .setMessage(message) + .setExecution(Execution.newBuilder().setCode(detailedCode)) + .build()); + } }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java index 98cfd82..1a62a69 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java
@@ -35,7 +35,6 @@ import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.shell.CommandException; import com.google.devtools.build.lib.util.CommandBuilder; -import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.InterruptedFailureDetails; import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.Pair; @@ -136,7 +135,8 @@ if (!residue.isEmpty()) { String message = "Unrecognized arguments: " + Joiner.on(' ').join(residue); env.getReporter().handle(Event.error(message)); - return BlazeCommandResult.exitCode(ExitCode.COMMAND_LINE_ERROR); + return BlazeCommandResult.failureDetail( + createFailureDetail(message, Code.ARGUMENTS_NOT_RECOGNIZED)); } Options cleanOptions = options.getOptions(Options.class); @@ -322,10 +322,14 @@ } private FailureDetail getFailureDetail() { - return FailureDetail.newBuilder() - .setMessage(getMessage()) - .setCleanCommand(FailureDetails.CleanCommand.newBuilder().setCode(detailedCode)) - .build(); + return createFailureDetail(getMessage(), detailedCode); } } + + private static FailureDetail createFailureDetail(String message, Code detailedCode) { + return FailureDetail.newBuilder() + .setMessage(message) + .setCleanCommand(FailureDetails.CleanCommand.newBuilder().setCode(detailedCode)) + .build(); + } }
diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index 6147481..fb70bbc 100644 --- a/src/main/protobuf/failure_details.proto +++ b/src/main/protobuf/failure_details.proto
@@ -405,6 +405,8 @@ [(metadata) = { exit_code: 1 }]; ACTION_FS_OUT_ERR_DIRECTORY_CREATION_FAILURE = 33 [(metadata) = { exit_code: 1 }]; + NON_ACTION_EXECUTION_FAILURE = 34 [(metadata) = { exit_code: 1 }]; + CYCLE = 35 [(metadata) = { exit_code: 1 }]; } Code code = 1; @@ -724,6 +726,7 @@ EXECROOT_DELETE_FAILURE = 7 [(metadata) = { exit_code: 36 }]; EXECROOT_TEMP_MOVE_FAILURE = 8 [(metadata) = { exit_code: 36 }]; ASYNC_EXECROOT_DELETE_FAILURE = 9 [(metadata) = { exit_code: 6 }]; + ARGUMENTS_NOT_RECOGNIZED = 10 [(metadata) = { exit_code: 2 }]; } Code code = 1;
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java index 427a132..e3f37ab 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
@@ -73,6 +73,9 @@ import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.remote.options.RemoteOutputsMode; import com.google.devtools.build.lib.runtime.KeepGoingOption; +import com.google.devtools.build.lib.server.FailureDetails.Execution; +import com.google.devtools.build.lib.server.FailureDetails.Execution.Code; +import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; @@ -85,6 +88,7 @@ import com.google.devtools.build.lib.testutil.TestRuleClassProvider; import com.google.devtools.build.lib.testutil.TestUtils; import com.google.devtools.build.lib.util.AbruptExitException; +import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -357,9 +361,11 @@ hasCycles |= !Iterables.isEmpty(cycles); } if (hasCycles) { - throw new BuildFailedException(CYCLE_MSG); + throw new BuildFailedException(CYCLE_MSG, createDetailedExitCode(Code.CYCLE)); } else if (result.errorMap().isEmpty() || keepGoing) { - throw new BuildFailedException(); + // The specific detailed code used here doesn't matter. + throw new BuildFailedException( + null, createDetailedExitCode(Code.NON_ACTION_EXECUTION_FAILURE)); } else { SkyframeBuilder.rethrow(Preconditions.checkNotNull(result.getError().getException())); } @@ -486,6 +492,13 @@ } } + private static DetailedExitCode createDetailedExitCode(Code detailedCode) { + return DetailedExitCode.of( + FailureDetail.newBuilder() + .setExecution(Execution.newBuilder().setCode(detailedCode)) + .build()); + } + /** {@link TestAction} that copies its single input to its single output. */ protected static class CopyingAction extends TestAction { CopyingAction(Runnable effect, Artifact input, Artifact output) {