Allows --query_file to be used for cquery and aquery too.

Fixes #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
bazelbuild · Nov 11, 2022 · fb23246 · fb23246
1 parent 6e945e8
commit fb23246
Show file tree

Hide file tree

Showing 9 changed files with 138 additions and 58 deletions.
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
@@ -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
@@ -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
@@ -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<String, QueryFunction> 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
@@ -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
@@ -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<String, QueryFunction> functions = new HashMap<>();
     for (QueryFunction queryFunction : ConfiguredTargetQueryEnvironment.FUNCTIONS) {
       functions.put(queryFunction.getName(), queryFunction);

diff --git a/...ain/java/com/google/devtools/build/lib/runtime/commands/QueryEnvironmentBasedCommand.java b/...ain/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<OutputFormatter> 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
@@ -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
@@ -1659,6 +1659,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:"
+}
+
 # TODO(bazel-team): The non-text aquery output formats don't correctly handle
 # non-ASCII fields (input/output paths, environment variables, etc).
 function DISABLED_test_unicode_textproto() {

diff --git a/src/test/shell/integration/configured_query_test.sh b/src/test/shell/integration/configured_query_test.sh
@@ -1430,4 +1430,20 @@ EOF
   expect_not_log "QueryException"
 }
 
+function test_does_not_fail_horribly_with_file() {
+  rm -rf peach
+  mkdir -p peach
+  cat > peach/BUILD <<EOF
+sh_library(name='brighton', deps=[':harken'])
+sh_library(name='harken')
+EOF
+
+  echo "deps(//peach:brighton)" > query_file
+  bazel cquery --query_file=query_file > $TEST_log
+
+  expect_log "//peach:brighton"
+  expect_log "//peach:harken"
+}
+
+
 run_suite "${PRODUCT_NAME} configured query tests"