diff --git a/src/main/java/com/google/devtools/build/lib/query2/engine/QueryExpression.java b/src/main/java/com/google/devtools/build/lib/query2/engine/QueryExpression.java index eb032516eeac83..0f88b0126226b5 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/engine/QueryExpression.java +++ b/src/main/java/com/google/devtools/build/lib/query2/engine/QueryExpression.java @@ -106,4 +106,10 @@ public final String toTrunctatedString() { public static String truncate(String expr) { return Ascii.truncate(expr, MAX_QUERY_EXPRESSION_LOG_CHARS, "[truncated]"); } + + /** Checks if this QueryExpression has a SomePathFunction at its top level. */ + public boolean isTopLevelSomePathFunction() { + return this instanceof FunctionExpression + && "somepath".equals(((FunctionExpression) this).getFunction().getName()); + } } diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/output/QueryOptions.java b/src/main/java/com/google/devtools/build/lib/query2/query/output/QueryOptions.java index 3b983f389acf22..563e10b53cdacc 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/query/output/QueryOptions.java +++ b/src/main/java/com/google/devtools/build/lib/query2/query/output/QueryOptions.java @@ -116,7 +116,7 @@ public enum OrderOutput { public OrderOutput orderOutput; @Option( - name = "incompatible_use_lexicographical_unordered_output", + name = "incompatible_lexicographical_output", defaultValue = "false", documentationCategory = OptionDocumentationCategory.QUERY, effectTags = {OptionEffectTag.TERMINAL_OUTPUT}, @@ -125,7 +125,7 @@ public enum OrderOutput { OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES }, help = "If this option is set, sorts --order_output=auto output in lexicographical order.") - public boolean useLexicographicalUnorderedOutput; + public boolean lexicographicalOutput; @Option( name = "graph:conditional_edges_limit", diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/output/QueryOutputUtils.java b/src/main/java/com/google/devtools/build/lib/query2/query/output/QueryOutputUtils.java index c0d21e603ae84d..671f199f1b32a6 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/query/output/QueryOutputUtils.java +++ b/src/main/java/com/google/devtools/build/lib/query2/query/output/QueryOutputUtils.java @@ -34,13 +34,23 @@ public class QueryOutputUtils { // Utility class cannot be instantiated. private QueryOutputUtils() {} - public static boolean shouldStreamResults(QueryOptions queryOptions, OutputFormatter formatter) { - return (queryOptions.orderOutput == OrderOutput.NO - || (queryOptions.orderOutput == OrderOutput.AUTO - && queryOptions.useLexicographicalUnorderedOutput)) + public static boolean lexicographicallySortOutput( + QueryOptions queryOptions, OutputFormatter formatter) { + return queryOptions.orderOutput == OrderOutput.AUTO + && queryOptions.lexicographicalOutput && formatter instanceof StreamedFormatter; } + public static boolean shouldStreamUnorderedOutput( + QueryOptions queryOptions, OutputFormatter formatter) { + return queryOptions.orderOutput == OrderOutput.NO && formatter instanceof StreamedFormatter; + } + + public static boolean shouldStreamResults(QueryOptions queryOptions, OutputFormatter formatter) { + return shouldStreamUnorderedOutput(queryOptions, formatter) + || lexicographicallySortOutput(queryOptions, formatter); + } + public static void output( QueryOptions queryOptions, QueryEvalResult result, diff --git a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java index 1fdef3bb7e6ae4..c6a27c890fa8a9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java +++ b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java @@ -66,7 +66,6 @@ import com.google.devtools.build.lib.query2.QueryEnvironmentFactory; import com.google.devtools.build.lib.query2.common.AbstractBlazeQueryEnvironment; import com.google.devtools.build.lib.query2.common.UniverseScope; -import com.google.devtools.build.lib.query2.engine.FunctionExpression; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.Setting; import com.google.devtools.build.lib.query2.engine.QueryEvalResult; import com.google.devtools.build.lib.query2.engine.QueryException; @@ -384,7 +383,7 @@ private GenQueryResult doQuery( QueryExpression expr = QueryExpression.parse(query, queryEnvironment); formatter.verifyCompatible(queryEnvironment, expr); targets = - graphlessQuery && !hasTopLevelSomePathFunction(expr) + graphlessQuery && !expr.isTopLevelSomePathFunction() ? QueryUtil.newLexicographicallySortedTargetAggregator() : QueryUtil.newOrderedAggregateAllOutputFormatterCallback(queryEnvironment); queryResult = queryEnvironment.evaluateQuery(expr, targets); @@ -427,12 +426,6 @@ private GenQueryResult doQuery( return outputStream.getResult(); } - /** Checks if the {@code expr} has a SomePathFunction at its top level */ - private static boolean hasTopLevelSomePathFunction(QueryExpression expr) { - return expr instanceof FunctionExpression - && "somepath".equals(((FunctionExpression) expr).getFunction().getName()); - } - @Immutable // assuming no other reference to result private static final class QueryResultAction extends AbstractFileWriteAction { private final GenQueryResult result; diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java index 2cf22e86c50462..11fb637211e46e 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java @@ -106,6 +106,14 @@ protected Either doQuery( expr = queryEnv.transformParsedQuery(expr); + // This only applies to --order_output=auto. Instead of being written directly to the stream + // by the callback, this option aggregates the results in the lexicographically sorted + // aggregator first before using the StreamedFormatter to write it to stream later. + // An exception to this is when somepath is used at the top level of the query expression. + boolean lexicographicallySortOutput = + QueryOutputUtils.lexicographicallySortOutput(queryOptions, formatter) + && !expr.isTopLevelSomePathFunction(); + OutputStream out; if (formatter.canBeBuffered()) { // There is no particular reason for the 16384 constant here, except its a multiple of the @@ -127,7 +135,7 @@ protected Either doQuery( queryOptions.aspectDeps.createResolver(env.getPackageManager(), env.getReporter()), hashFunction); streamedFormatter.setEventHandler(env.getReporter()); - if (queryOptions.useLexicographicalUnorderedOutput) { + if (lexicographicallySortOutput) { callback = QueryUtil.newLexicographicallySortedTargetAggregator(); } else { callback = streamedFormatter.createStreamCallback(out, queryOptions, queryEnv); @@ -170,7 +178,7 @@ protected Either doQuery( out.flush(); } } - if (!streamResults || queryOptions.useLexicographicalUnorderedOutput) { + if (!streamResults || lexicographicallySortOutput) { disableAnsiCharactersFiltering(env); try (SilentCloseable closeable = Profiler.instance().profile("QueryOutputUtils.output")) { Set targets = diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryEnvironmentBasedCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryEnvironmentBasedCommand.java index 030d958de8b5c3..a8fcfa62590f82 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryEnvironmentBasedCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryEnvironmentBasedCommand.java @@ -153,8 +153,8 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe return reportAndCreateFailureResult( env, String.format( - "--experimental_graphless_query requires --order_output=no and an --output" - + " option that supports streaming; valid values are: %s", + "--experimental_graphless_query requires --order_output=no or --order_output=auto and" + + " an --output option that supports streaming; valid values are: %s", OutputFormatters.streamingFormatterNames(formatters)), Query.Code.GRAPHLESS_PREREQ_UNMET); } diff --git a/src/test/shell/integration/bazel_query_test.sh b/src/test/shell/integration/bazel_query_test.sh index 1d174d97f192bd..1b4d20f53dd981 100755 --- a/src/test/shell/integration/bazel_query_test.sh +++ b/src/test/shell/integration/bazel_query_test.sh @@ -712,6 +712,7 @@ EOF } function test_graphless_query_matches_graphless_genquery_output() { + rm -rf foo mkdir -p foo cat > foo/BUILD <& foo/query_output || fail "Expected success" - # The outputs of graphless query and graphless genquery should be the same. - assert_equals "$(cat bazel-bin/foo/q)" "$(cat foo/query_output)" - - # The outputs of both graphless query and graphless genquery should be in - # lexicographical order (comparing one should be sufficient). + # The outputs of graphless query and graphless genquery should be the same and + # should both be in lexicographical order. + assert_equals \ + "$(cat foo/expected_lexicographical_result)" "$(cat foo/query_output)" assert_equals \ "$(cat foo/expected_lexicographical_result)" "$(cat bazel-bin/foo/q)" } +function test_lexicographical_output_does_not_affect_order_output_no() { + rm -rf foo + mkdir -p foo + cat > foo/BUILD <& foo/query_output \ + || fail "Expected success" + bazel query --order_output=no \ + --incompatible_lexicographical_output \ + "deps(//foo:b)" | grep foo >& foo/lex_query_output \ + || fail "Expected success" + + # The --incompatible_lexicographical_output flag should not affect query + # order_output=no. Note that there is a chance it may output in + # lexicographical order since it is unordered. + assert_equals \ + "$(cat foo/query_output)" "$(cat foo/lex_query_output)" +} + +function test_lexicographical_output_does_not_affect_somepath() { + rm -rf foo + mkdir -p foo + cat > foo/BUILD < foo/expected_deps_output <& foo/query_output + + assert_equals \ + "$(cat foo/expected_deps_output)" "$(cat foo/query_output)" +} + # Regression test for https://github.com/bazelbuild/bazel/issues/8582. function test_rbuildfiles_can_handle_non_loading_phase_edges() { mkdir -p foo