Encode BlazeCommandDispatcher failures with FailureDetails A failure in Starlark option parsing resulted in different numerical exit codes depending on what command was used. For consistency, it now returns the same exit code as native option parsing failures, 2, everywhere. RELNOTES: None. PiperOrigin-RevId: 313622009
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java index 34eff00..2d4d6b1 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
@@ -296,8 +296,7 @@ // Provide the options parser so that we can cache OptionsData here. createOptionsParser(command), invocationPolicy); - DetailedExitCode earlyExitCode = - DetailedExitCode.justExitCode(optionHandler.parseOptions(args, storedEventHandler)); + DetailedExitCode earlyExitCode = optionHandler.parseOptions(args, storedEventHandler); OptionsParsingResult options = optionHandler.getOptionsResult(); CommandLineEvent originalCommandLineEvent = @@ -567,9 +566,7 @@ } // Parse starlark options. - earlyExitCode = - DetailedExitCode.justExitCode( - optionHandler.parseStarlarkOptions(env, storedEventHandler)); + earlyExitCode = optionHandler.parseStarlarkOptions(env, storedEventHandler); if (!earlyExitCode.isSuccess()) { replayEarlyExitEvents( outErr,
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java index 8e91928..7a83102 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java
@@ -23,7 +23,10 @@ import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy; -import com.google.devtools.build.lib.util.ExitCode; +import com.google.devtools.build.lib.server.FailureDetails; +import com.google.devtools.build.lib.server.FailureDetails.Command.Code; +import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; +import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.common.options.InvocationPolicyEnforcer; @@ -97,34 +100,34 @@ * Only some commands work if cwd != workspaceSuffix in Blaze. In that case, also check if Blaze * was called from the output directory and fail if it was. */ - private ExitCode checkCwdInWorkspace(EventHandler eventHandler) { + private DetailedExitCode checkCwdInWorkspace(EventHandler eventHandler) { if (!commandAnnotation.mustRunInWorkspace()) { - return ExitCode.SUCCESS; + return DetailedExitCode.success(); } if (!workspace.getDirectories().inWorkspace()) { - eventHandler.handle( - Event.error( - "The '" - + commandAnnotation.name() - + "' command is only supported from within a workspace" - + " (below a directory having a WORKSPACE file).\n" - + "See documentation at" - + " https://docs.bazel.build/versions/master/build-ref.html#workspace")); - return ExitCode.COMMAND_LINE_ERROR; + String message = + "The '" + + commandAnnotation.name() + + "' command is only supported from within a workspace" + + " (below a directory having a WORKSPACE file).\n" + + "See documentation at" + + " https://docs.bazel.build/versions/master/build-ref.html#workspace"; + eventHandler.handle(Event.error(message)); + return createDetailedExitCode(message, Code.NOT_IN_WORKSPACE); } Path workspacePath = workspace.getWorkspace(); // TODO(kchodorow): Remove this once spaces are supported. if (workspacePath.getPathString().contains(" ")) { - eventHandler.handle( - Event.error( - runtime.getProductName() - + " does not currently work properly from paths " - + "containing spaces (" - + workspace - + ").")); - return ExitCode.LOCAL_ENVIRONMENTAL_ERROR; + String message = + runtime.getProductName() + + " does not currently work properly from paths " + + "containing spaces (" + + workspacePath + + ")."; + eventHandler.handle(Event.error(message)); + return createDetailedExitCode(message, Code.SPACES_IN_WORKSPACE_PATH); } if (workspacePath.getParentDirectory() != null) { @@ -133,8 +136,9 @@ if (doNotBuild.exists()) { if (!commandAnnotation.canRunInOutputDirectory()) { - eventHandler.handle(Event.error(getNotInRealWorkspaceError(doNotBuild))); - return ExitCode.COMMAND_LINE_ERROR; + String message = getNotInRealWorkspaceError(doNotBuild); + eventHandler.handle(Event.error(message)); + return createDetailedExitCode(message, Code.IN_OUTPUT_DIRECTORY); } else { eventHandler.handle( Event.warn( @@ -142,7 +146,7 @@ } } } - return ExitCode.SUCCESS; + return DetailedExitCode.success(); } /** @@ -230,7 +234,7 @@ * TODO(bazel-team): When we move CoreOptions options to be defined in starlark, make sure they're * not passed in here during {@link #getOptionsResult}. */ - ExitCode parseStarlarkOptions(CommandEnvironment env, ExtendedEventHandler eventHandler) { + DetailedExitCode parseStarlarkOptions(CommandEnvironment env, ExtendedEventHandler eventHandler) { // For now, restrict starlark options to commands that already build to ensure that loading // will work. We may want to open this up to other commands in the future. The "info" // and "clean" commands have builds=true set in their annotation but don't actually do any @@ -238,31 +242,32 @@ if (!commandAnnotation.builds() || commandAnnotation.name().equals("info") || commandAnnotation.name().equals("clean")) { - return ExitCode.SUCCESS; + return DetailedExitCode.success(); } try { StarlarkOptionsParser.newStarlarkOptionsParser(env, optionsParser).parse(eventHandler); } catch (OptionsParsingException e) { env.getReporter().handle(Event.error(e.getMessage())); - logger.atInfo().withCause(e).log("Error parsing options"); - return ExitCode.PARSING_FAILURE; + logger.atInfo().withCause(e).log("Error parsing Starlark options"); + return createDetailedExitCode( + "Error parsing Starlark options: " + e.getMessage(), Code.STARLARK_OPTIONS_PARSE_FAILURE); } - return ExitCode.SUCCESS; + return DetailedExitCode.success(); } /** * Parses the options, taking care not to generate any output to outErr, return, or throw an * exception. * - * @return ExitCode.SUCCESS if everything went well, or some other value if not + * @return {@code DetailedExitCode.success()} if everything went well, or some other value if not */ - ExitCode parseOptions(List<String> args, ExtendedEventHandler eventHandler) { + DetailedExitCode parseOptions(List<String> args, ExtendedEventHandler eventHandler) { // The initialization code here was carefully written to parse the options early before we call // into the BlazeModule APIs, which means we must not generate any output to outErr, return, or // throw an exception. All the events happening here are instead stored in a temporary event // handler, and later replayed. - ExitCode earlyExitCode = checkCwdInWorkspace(eventHandler); - if (!earlyExitCode.equals(ExitCode.SUCCESS)) { + DetailedExitCode earlyExitCode = checkCwdInWorkspace(eventHandler); + if (!earlyExitCode.isSuccess()) { return earlyExitCode; } @@ -308,9 +313,10 @@ } catch (OptionsParsingException e) { eventHandler.handle(Event.error(e.getMessage())); logger.atInfo().withCause(e).log("Error parsing options"); - return ExitCode.COMMAND_LINE_ERROR; + return createDetailedExitCode( + "Error parsing options" + e.getMessage(), Code.OPTIONS_PARSE_FAILURE); } - return ExitCode.SUCCESS; + return DetailedExitCode.success(); } /** @@ -425,4 +431,12 @@ return commandToRcArgs; } + + private static DetailedExitCode createDetailedExitCode(String message, Code detailedCode) { + return DetailedExitCode.of( + FailureDetail.newBuilder() + .setMessage(message) + .setCommand(FailureDetails.Command.newBuilder().setCode(detailedCode)) + .build()); + } }
diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index 58d3332..1641448 100644 --- a/src/main/protobuf/failure_details.proto +++ b/src/main/protobuf/failure_details.proto
@@ -434,6 +434,9 @@ OPTIONS_PARSE_FAILURE = 9 [(metadata) = { exit_code: 2 }]; STARLARK_OPTIONS_PARSE_FAILURE = 10 [(metadata) = { exit_code: 2 }]; ARGUMENTS_NOT_RECOGNIZED = 11 [(metadata) = { exit_code: 2 }]; + NOT_IN_WORKSPACE = 12 [(metadata) = { exit_code: 2 }]; + SPACES_IN_WORKSPACE_PATH = 13 [(metadata) = { exit_code: 36 }]; + IN_OUTPUT_DIRECTORY = 14 [(metadata) = { exit_code: 2 }]; } Code code = 1;