From ee32efffe4c407d7fe54b287a80a3b0a115040c6 Mon Sep 17 00:00:00 2001 From: keertk Date: Mon, 20 Mar 2023 10:14:11 -0700 Subject: [PATCH] [6.2.0] Allows --query_file to be used for cquery and aquery too. (#17823) Fixes https://github.com/bazelbuild/bazel/issues/12924. RELNOTES[NEW]: The aquery and cquery commands now respect the --query_file flag just like the query command. PiperOrigin-RevId: 487689456 Change-Id: Ia2c9d85e88fdf769a823eaf7b6585a77d654ae70 --- .../lib/query2/common/CommonQueryOptions.java | 10 +++ .../lib/query2/query/output/QueryOptions.java | 11 --- .../lib/runtime/commands/AqueryCommand.java | 13 ++-- .../devtools/build/lib/runtime/commands/BUILD | 1 + .../lib/runtime/commands/CqueryCommand.java | 16 +++-- .../QueryEnvironmentBasedCommand.java | 38 ++-------- .../runtime/commands/QueryOptionHelper.java | 72 +++++++++++++++++++ src/test/shell/integration/aquery_test.sh | 19 +++++ .../integration/configured_query_test.sh | 15 ++++ 9 files changed, 137 insertions(+), 58 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/runtime/commands/QueryOptionHelper.java diff --git a/src/main/java/com/google/devtools/build/lib/query2/common/CommonQueryOptions.java b/src/main/java/com/google/devtools/build/lib/query2/common/CommonQueryOptions.java index 09483e52cb7d2d..693c88fa62070e 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/common/CommonQueryOptions.java +++ b/src/main/java/com/google/devtools/build/lib/query2/common/CommonQueryOptions.java @@ -326,4 +326,14 @@ public AspectResolutionModeConverter() { + "will be merged together and their labels concatenated. This option is only " + "applicable to --output=graph.") public boolean graphFactored; + + @Option( + name = "query_file", + defaultValue = "", + documentationCategory = OptionDocumentationCategory.QUERY, + effectTags = {OptionEffectTag.CHANGES_INPUTS}, + help = + "If set, query will read the query from the file named here, rather than on the command " + + "line. It is an error to specify a file here as well as a command-line query.") + public String queryFile; } 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 75c54687eb7521..185f1f067e82d1 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 @@ -167,17 +167,6 @@ public enum OrderOutput { ) public boolean strictTestSuite; - @Option( - name = "query_file", - defaultValue = "", - documentationCategory = OptionDocumentationCategory.QUERY, - effectTags = {OptionEffectTag.CHANGES_INPUTS}, - help = - "If set, query will read the query from the file named here, rather than on the command " - + "line. It is an error to specify a file here as well as a command-line query." - ) - public String queryFile; - @Option( name = "experimental_graphless_query", defaultValue = "auto", diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/AqueryCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/AqueryCommand.java index 79f19a1d7c2b3d..d24833b247e28a 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/AqueryCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/AqueryCommand.java @@ -13,7 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.runtime.commands; -import com.google.common.base.Joiner; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -101,15 +100,13 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti InterruptedFailureDetails.detailedExitCode(errorMessage)); } - // When querying for the state of Skyframe, it's possible to omit the query expression. - if (options.getResidue().isEmpty() && !queryCurrentSkyframeState) { - String message = - "Missing query expression. Use the 'help aquery' command for syntax and help."; - env.getReporter().handle(Event.error(message)); - return createFailureResult(message, Code.COMMAND_LINE_EXPRESSION_MISSING); + String query = null; + try { + query = QueryOptionHelper.readQuery(aqueryOptions, options, env, queryCurrentSkyframeState); + } catch (QueryException e) { + return BlazeCommandResult.failureDetail(e.getFailureDetail()); } - String query = Joiner.on(' ').join(options.getResidue()); ImmutableMap functions = getFunctionsMap(env); // Query expression might be null in the case of --skyframe_state. diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD b/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD index ea846eca8f25e8..1a9e10d2333cb5 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD @@ -65,6 +65,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/profiler/memory:allocationtracker", "//src/main/java/com/google/devtools/build/lib/query2", "//src/main/java/com/google/devtools/build/lib/query2/common:abstract-blaze-query-env", + "//src/main/java/com/google/devtools/build/lib/query2/common:options", "//src/main/java/com/google/devtools/build/lib/query2/common:universe-scope", "//src/main/java/com/google/devtools/build/lib/query2/engine", "//src/main/java/com/google/devtools/build/lib/query2/query/output", diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/CqueryCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/CqueryCommand.java index 1d7a5a0170a762..fed132f90b89ca 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/CqueryCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/CqueryCommand.java @@ -13,7 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.runtime.commands; -import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.analysis.config.CoreOptions.IncludeConfigFragmentsEnum; import com.google.devtools.build.lib.buildtool.BuildRequest; @@ -27,6 +26,7 @@ import com.google.devtools.build.lib.query2.cquery.ConfiguredTargetQueryEnvironment; import com.google.devtools.build.lib.query2.cquery.CqueryOptions; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.QueryFunction; +import com.google.devtools.build.lib.query2.engine.QueryException; import com.google.devtools.build.lib.query2.engine.QueryExpression; import com.google.devtools.build.lib.query2.engine.QueryParser; import com.google.devtools.build.lib.query2.engine.QuerySyntaxException; @@ -133,13 +133,15 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti InterruptedFailureDetails.detailedExitCode(errorMessage)); } - if (options.getResidue().isEmpty()) { - String message = - "Missing query expression. Use the 'help cquery' command for syntax and help."; - env.getReporter().handle(Event.error(message)); - return createFailureResult(message, Code.COMMAND_LINE_EXPRESSION_MISSING); + String query = null; + try { + query = + QueryOptionHelper.readQuery( + options.getOptions(CqueryOptions.class), options, env, /* allowEmptyQuery= */ false); + } catch (QueryException e) { + return BlazeCommandResult.failureDetail(e.getFailureDetail()); } - String query = Joiner.on(' ').join(options.getResidue()); + HashMap functions = new HashMap<>(); for (QueryFunction queryFunction : ConfiguredTargetQueryEnvironment.FUNCTIONS) { functions.put(queryFunction.getName(), queryFunction); 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 083382b643aaaf..d5b28176fb649a 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 @@ -15,7 +15,6 @@ import static com.google.devtools.build.lib.packages.Rule.ALL_LABELS; -import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.analysis.NoBuildEvent; import com.google.devtools.build.lib.analysis.NoBuildRequestFinishedEvent; @@ -32,6 +31,7 @@ import com.google.devtools.build.lib.query2.engine.QueryEnvironment; 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; import com.google.devtools.build.lib.query2.query.output.OutputFormatter; import com.google.devtools.build.lib.query2.query.output.OutputFormatters; import com.google.devtools.build.lib.query2.query.output.QueryOptions; @@ -57,13 +57,9 @@ import com.google.devtools.build.lib.util.Either; import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.InterruptedFailureDetails; -import com.google.devtools.build.lib.vfs.FileSystemUtils; -import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.skyframe.WalkableGraph; import com.google.devtools.common.options.OptionsParsingResult; import com.google.devtools.common.options.TriState; -import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.util.Set; import java.util.function.Function; @@ -125,33 +121,11 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe return BlazeCommandResult.detailedExitCode(e.getDetailedExitCode()); } - String query; - if (!options.getResidue().isEmpty()) { - if (!queryOptions.queryFile.isEmpty()) { - return reportAndCreateFailureResult( - env, - "Command-line query and --query_file cannot both be specified", - Query.Code.QUERY_FILE_WITH_COMMAND_LINE_EXPRESSION); - } - query = Joiner.on(' ').join(options.getResidue()); - } else if (!queryOptions.queryFile.isEmpty()) { - // Works for absolute or relative query file. - Path residuePath = env.getWorkingDirectory().getRelative(queryOptions.queryFile); - try { - query = new String(FileSystemUtils.readContent(residuePath), StandardCharsets.UTF_8); - } catch (IOException e) { - return reportAndCreateFailureResult( - env, - "I/O error reading from " + residuePath.getPathString(), - Query.Code.QUERY_FILE_READ_FAILURE); - } - } else { - return reportAndCreateFailureResult( - env, - String.format( - "missing query expression. Type '%s help query' for syntax and help", - runtime.getProductName()), - Query.Code.COMMAND_LINE_EXPRESSION_MISSING); + String query = null; + try { + query = QueryOptionHelper.readQuery(queryOptions, options, env, /* allowEmptyQuery =*/ false); + } catch (QueryException e) { + return BlazeCommandResult.failureDetail(e.getFailureDetail()); } Iterable formatters = runtime.getQueryOutputFormatters(); diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryOptionHelper.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryOptionHelper.java new file mode 100644 index 00000000000000..11d676393c1423 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryOptionHelper.java @@ -0,0 +1,72 @@ +// Copyright 2022 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License.package com.google.devtools.build.lib.runtime.commands; +package com.google.devtools.build.lib.runtime.commands; + +import static java.nio.charset.StandardCharsets.UTF_8; + +import com.google.common.base.Joiner; +import com.google.devtools.build.lib.query2.common.CommonQueryOptions; +import com.google.devtools.build.lib.query2.engine.QueryException; +import com.google.devtools.build.lib.runtime.CommandEnvironment; +import com.google.devtools.build.lib.server.FailureDetails.Query; +import com.google.devtools.build.lib.vfs.FileSystemUtils; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.common.options.OptionsParsingResult; +import java.io.IOException; + +/** + * Reads the query for query, cquery and aquery using the --query_file option or from the residue of + * the command line. + */ +public final class QueryOptionHelper { + + public static String readQuery( + CommonQueryOptions queryOptions, + OptionsParsingResult options, + CommandEnvironment env, + boolean allowEmptyQuery) + throws QueryException { + String query = ""; + if (!options.getResidue().isEmpty()) { + if (!queryOptions.queryFile.isEmpty()) { + throw new QueryException( + "Command-line query and --query_file cannot both be specified", + Query.Code.QUERY_FILE_WITH_COMMAND_LINE_EXPRESSION); + } + query = Joiner.on(' ').join(options.getResidue()); + } else if (!queryOptions.queryFile.isEmpty()) { + // Works for absolute or relative query file. + Path residuePath = env.getWorkingDirectory().getRelative(queryOptions.queryFile); + try { + query = new String(FileSystemUtils.readContent(residuePath), UTF_8); + } catch (IOException unused) { + throw new QueryException( + "I/O error reading from " + residuePath.getPathString(), + Query.Code.QUERY_FILE_READ_FAILURE); + } + } else { + // When querying for the state of Skyframe, it's possible to omit the query expression. + if (!allowEmptyQuery) { + throw new QueryException( + String.format( + "missing query expression. Type '%s help query' for syntax and help", + env.getRuntime().getProductName()), + Query.Code.COMMAND_LINE_EXPRESSION_MISSING); + } + } + return query; + } + + private QueryOptionHelper() {} +} diff --git a/src/test/shell/integration/aquery_test.sh b/src/test/shell/integration/aquery_test.sh index 83adfcc4f70e28..85a538c4c65339 100755 --- a/src/test/shell/integration/aquery_test.sh +++ b/src/test/shell/integration/aquery_test.sh @@ -1665,6 +1665,25 @@ EOF fi } +function test_does_not_fail_horribly_with_file() { + rm -rf peach + mkdir -p peach + cat > "peach/BUILD" <<'EOF' +genrule( + name = "bar", + srcs = ["dummy.txt"], + outs = ["bar_out.txt"], + cmd = "echo unused > bar_out.txt", +) +EOF + + echo "//peach:bar" > query_file + bazel aquery --query_file=query_file > $TEST_log + + expect_log "Target: //peach:bar" "look in $TEST_log" + expect_log "ActionKey:" +} + function test_cpp_compile_action_env() { local pkg="${FUNCNAME[0]}" mkdir -p "$pkg" diff --git a/src/test/shell/integration/configured_query_test.sh b/src/test/shell/integration/configured_query_test.sh index 53106731086f02..615296bf77f902 100755 --- a/src/test/shell/integration/configured_query_test.sh +++ b/src/test/shell/integration/configured_query_test.sh @@ -1461,6 +1461,21 @@ EOF expect_not_log "QueryException" } +function test_does_not_fail_horribly_with_file() { + rm -rf peach + mkdir -p peach + cat > peach/BUILD < query_file + bazel cquery --query_file=query_file > $TEST_log + + expect_log "//peach:brighton" + expect_log "//peach:harken" +} + function test_files_include_source_files() { local -r pkg=$FUNCNAME mkdir -p $pkg