Skip to content

Commit

Permalink
Use correct exit code on invalid aquery --output
Browse files Browse the repository at this point in the history
Currently this exits 0, which is dangerous - scripts may assume a query
succeeded with no results, where actually it never ran.

Instead, properly exit with exit code 2.

Closes bazelbuild#13660.

PiperOrigin-RevId: 385959603
  • Loading branch information
illicitonion authored and Copybara-Service committed Jul 21, 2021
1 parent 6211b86 commit 4158b61
Showing 1 changed file with 20 additions and 12 deletions.
Expand Up @@ -28,13 +28,15 @@
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.runtime.QueryRuntimeHelper;
import com.google.devtools.build.lib.runtime.QueryRuntimeHelper.QueryRuntimeHelperException;
import com.google.devtools.build.lib.server.FailureDetails.ActionQuery;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.server.FailureDetails.Query;
import com.google.devtools.build.lib.server.FailureDetails.Query.Code;
import com.google.devtools.build.lib.skyframe.SkyframeExecutorWrappingWalkableGraph;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.WalkableGraph;
import com.google.devtools.common.options.OptionsParsingException;
import java.io.IOException;
import java.util.Collection;
import java.util.Set;
Expand Down Expand Up @@ -68,7 +70,7 @@ protected void postProcessAnalysisResult(BuildRequest request, AnalysisResult an
.setMessage(
"Queries based on analysis results are not allowed if incrementality state"
+ " is not being kept")
.setQuery(Query.newBuilder().setCode(Code.ANALYSIS_QUERY_PREREQ_UNMET))
.setQuery(Query.newBuilder().setCode(Query.Code.ANALYSIS_QUERY_PREREQ_UNMET))
.build()));
}
try (QueryRuntimeHelper queryRuntimeHelper =
Expand All @@ -92,13 +94,22 @@ protected void postProcessAnalysisResult(BuildRequest request, AnalysisResult an
FailureDetail failureDetail =
FailureDetail.newBuilder()
.setMessage(errorMessage + ": " + e.getMessage())
.setQuery(Query.newBuilder().setCode(Code.OUTPUT_FORMATTER_IO_EXCEPTION))
.setQuery(Query.newBuilder().setCode(Query.Code.OUTPUT_FORMATTER_IO_EXCEPTION))
.build();
throw new ViewCreationFailedException(errorMessage, failureDetail, e);
}
env.getReporter().error(null, errorMessage, e);
} catch (QueryRuntimeHelperException e) {
throw new ExitException(DetailedExitCode.of(e.getFailureDetail()));
} catch (OptionsParsingException e) {
throw new ExitException(
DetailedExitCode.of(
ExitCode.COMMAND_LINE_ERROR,
FailureDetail.newBuilder()
.setMessage(e.getMessage())
.setActionQuery(
ActionQuery.newBuilder().setCode(ActionQuery.Code.INCORRECT_ARGUMENTS))
.build()));
}
}
}
Expand All @@ -118,7 +129,8 @@ private void doPostAnalysisQuery(
Collection<SkyKey> transitiveConfigurationKeys,
QueryRuntimeHelper queryRuntimeHelper,
QueryExpression queryExpression)
throws InterruptedException, QueryException, IOException, QueryRuntimeHelperException {
throws InterruptedException, QueryException, IOException, QueryRuntimeHelperException,
OptionsParsingException {
WalkableGraph walkableGraph =
SkyframeExecutorWrappingWalkableGraph.of(env.getSkyframeExecutor());

Expand All @@ -143,14 +155,10 @@ private void doPostAnalysisQuery(
NamedThreadSafeOutputFormatterCallback<T> callback =
NamedThreadSafeOutputFormatterCallback.selectCallback(outputFormat, callbacks);
if (callback == null) {
env.getReporter()
.handle(
Event.error(
String.format(
"Invalid output format '%s'. Valid values are: %s",
outputFormat,
NamedThreadSafeOutputFormatterCallback.callbackNames(callbacks))));
return;
throw new OptionsParsingException(
String.format(
"Invalid output format '%s'. Valid values are: %s",
outputFormat, NamedThreadSafeOutputFormatterCallback.callbackNames(callbacks)));
}

// A certain subset of output formatters support "streaming" results - the formatter is called
Expand Down

0 comments on commit 4158b61

Please sign in to comment.