Fix fetch target (fixes bazelbuild#13847)

Context:
- Traditional query relies on the initial loading phase of the build, this lacks the context of build configurations (flags, select() logic), leading to potentially inaccurate or over-inclusive dependency listings.

- cquery executes after the analysis phase, where Bazel has resolved configurations and determined how options influence target definitions. This allows cquery to provide the dependencies truly needed for a build under the current settings.

Considering these differences, I'm updating fetch target logic to rely on cquery instead. This ensures that all necessary repositories are fetched for an offline build while avoiding potential over-fetching

PiperOrigin-RevId: 611455579
Change-Id: I2a954476c06182fd9eb78ad86def7bd72f04074a

src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD

@@ -45,11 +45,10 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/packages",
-        "//src/main/java/com/google/devtools/build/lib/packages:label_printer",
         "//src/main/java/com/google/devtools/build/lib/packages/semantics",
         "//src/main/java/com/google/devtools/build/lib/pkgcache",
-        "//src/main/java/com/google/devtools/build/lib/query2/common:abstract-blaze-query-env",
-        "//src/main/java/com/google/devtools/build/lib/query2/common:universe-scope",
+        "//src/main/java/com/google/devtools/build/lib/query2",
+        "//src/main/java/com/google/devtools/build/lib/query2/common:cquery-node",
         "//src/main/java/com/google/devtools/build/lib/query2/engine",
         "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
         "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",

src/main/java/com/google/devtools/build/lib/bazel/commands/FetchCommand.java

@@ -30,6 +30,7 @@
 import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
 import com.google.devtools.build.lib.pkgcache.PackageOptions;
+import com.google.devtools.build.lib.query2.cquery.CqueryOptions;
 import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
 import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
 import com.google.devtools.build.lib.runtime.BlazeCommand;
@@ -38,30 +39,35 @@
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.runtime.KeepGoingOption;
 import com.google.devtools.build.lib.runtime.LoadingPhaseThreadsOption;
+import com.google.devtools.build.lib.runtime.commands.TestCommand;
 import com.google.devtools.build.lib.server.FailureDetails;
 import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
 import com.google.devtools.build.lib.server.FailureDetails.FetchCommand.Code;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue;
 import com.google.devtools.build.lib.skyframe.RepositoryMappingValue.RepositoryMappingResolutionException;
-import com.google.devtools.build.lib.util.AbruptExitException;
 import com.google.devtools.build.lib.util.DetailedExitCode;
 import com.google.devtools.build.lib.util.InterruptedFailureDetails;
 import com.google.devtools.build.skyframe.EvaluationContext;
 import com.google.devtools.build.skyframe.EvaluationResult;
 import com.google.devtools.build.skyframe.SkyValue;
+import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsParsingResult;
 import java.util.List;
 import javax.annotation.Nullable;
 
 /** Fetches external repositories. Which is so fetch. */
 @Command(
     name = FetchCommand.NAME,
+    builds = true,
+    inherits = {TestCommand.class},
     options = {
       FetchOptions.class,
+      CqueryOptions.class,
       PackageOptions.class,
       KeepGoingOption.class,
       LoadingPhaseThreadsOption.class
     },
+    usesConfigurationOptions = true,
     help = "resource:fetch.txt",
     shortDescription = "Fetches external repositories that are prerequisites to the targets.",
     allowResidue = true,
@@ -70,6 +76,14 @@ public final class FetchCommand implements BlazeCommand {
 
   public static final String NAME = "fetch";
 
+  @Override
+  public void editOptions(OptionsParser optionsParser) {
+    // We only need to inject these options with fetch target (when there is a residue)
+    if (!optionsParser.getResidue().isEmpty()) {
+      TargetFetcher.injectOptionsToFetchTarget(optionsParser);
+    }
+  }
+
   @Override
   public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult options) {
     BlazeCommandResult invalidResult = validateOptions(env, options);
@@ -100,17 +114,13 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti
     BlazeCommandResult result;
     LoadingPhaseThreadsOption threadsOption = options.getOptions(LoadingPhaseThreadsOption.class);
     try {
-      env.syncPackageLoading(options);
       if (!options.getResidue().isEmpty()) {
         result = fetchTarget(env, options, options.getResidue());
       } else if (!fetchOptions.repos.isEmpty()) {
         result = fetchRepos(env, threadsOption, fetchOptions.repos);
       } else { // --all, --configure, or just 'fetch'
         result = fetchAll(env, threadsOption, fetchOptions.configure);
       }
-    } catch (AbruptExitException e) {
-      return createFailedBlazeCommandResult(
-          env.getReporter(), e.getMessage(), e.getDetailedExitCode());
     } catch (InterruptedException e) {
       return createFailedBlazeCommandResult(
           env.getReporter(), "Fetch interrupted: " + e.getMessage());

src/main/java/com/google/devtools/build/lib/bazel/commands/TargetFetcher.java

@@ -14,30 +14,39 @@
 
 package com.google.devtools.build.lib.bazel.commands;
 
+import static com.google.common.collect.ImmutableMap.toImmutableMap;
+
 import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.buildtool.BuildRequest;
+import com.google.devtools.build.lib.buildtool.BuildResult;
+import com.google.devtools.build.lib.buildtool.BuildTool;
+import com.google.devtools.build.lib.buildtool.CqueryProcessor;
 import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.cmdline.TargetPattern;
 import com.google.devtools.build.lib.cmdline.TargetPattern.Parser;
-import com.google.devtools.build.lib.packages.LabelPrinter;
-import com.google.devtools.build.lib.packages.Target;
-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.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.NamedThreadSafeOutputFormatterCallback;
+import com.google.devtools.build.lib.query2.common.CqueryNode;
+import com.google.devtools.build.lib.query2.cquery.ConfiguredTargetQueryEnvironment;
+import com.google.devtools.build.lib.query2.engine.QueryEnvironment.QueryFunction;
 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;
-import com.google.devtools.build.lib.query2.engine.ThreadSafeOutputFormatterCallback;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.runtime.KeepGoingOption;
 import com.google.devtools.build.lib.runtime.LoadingPhaseThreadsOption;
-import com.google.devtools.build.lib.runtime.commands.QueryCommand;
 import com.google.devtools.build.lib.skyframe.RepositoryMappingValue.RepositoryMappingResolutionException;
+import com.google.devtools.common.options.OptionPriority.PriorityCategory;
+import com.google.devtools.common.options.OptionsParser;
+import com.google.devtools.common.options.OptionsParsingException;
 import com.google.devtools.common.options.OptionsParsingResult;
-import java.io.IOException;
-import java.util.EnumSet;
+import java.util.Collection;
 import java.util.List;
+import java.util.Optional;
+import java.util.function.Function;
+import java.util.stream.Stream;
 
 /** Fetches all repos needed for building a given set of targets. */
 public class TargetFetcher {
@@ -47,7 +56,7 @@ private TargetFetcher(CommandEnvironment env) {
     this.env = env;
   }
 
-  /** Uses `deps` query to find and fetch all repos needed for these targets */
+  /** Uses cquery to find and fetch all repos needed to build these targets */
   public static void fetchTargets(
       CommandEnvironment env, OptionsParsingResult options, List<String> targets)
       throws RepositoryMappingResolutionException, InterruptedException, TargetFetcherException {
@@ -56,28 +65,17 @@ public static void fetchTargets(
 
   private void fetchTargets(OptionsParsingResult options, List<String> targets)
       throws InterruptedException, TargetFetcherException, RepositoryMappingResolutionException {
-    AbstractBlazeQueryEnvironment<Target> queryEnv = getQueryEnv(options);
-    QueryExpression expr = createQueryExpression(targets, queryEnv);
-    QueryEvalResult queryEvalResult;
-    try {
-      queryEvalResult =
-          queryEnv.evaluateQuery(
-              expr,
-              new ThreadSafeOutputFormatterCallback<>() {
-                @Override
-                public void processOutput(Iterable<Target> partialResult) {}
-              });
-    } catch (IOException e) {
-      // Should be impossible since our OutputFormatterCallback doesn't throw IOException.
-      throw new IllegalStateException(e);
-    } catch (QueryException e) {
-      throw new TargetFetcherException(
-          String.format(
-              "Fetching target dependencies for %s encountered an error: %s",
-              expr, e.getMessage()));
-    }
+    QueryExpression expr = createQueryExpression(targets);
+    BuildRequest request = createBuildRequest(env, options, targets);
+    TargetPattern.Parser mainRepoTargetParser = getMainRepoMappingParser(env);
 
-    if (!queryEvalResult.getSuccess()) {
+    BuildResult result =
+        new BuildTool(
+                env,
+                new CqueryProcessor(
+                    expr, mainRepoTargetParser, Optional.of(createNoOutputFormatter())))
+            .processRequest(request, /* validator= */ null);
+    if (!result.getSuccess()) {
       throw new TargetFetcherException(
           String.format(
               "Fetching some target dependencies for %s failed, but --keep_going specified. "
@@ -86,33 +84,59 @@ public void processOutput(Iterable<Target> partialResult) {}
     }
   }
 
-  AbstractBlazeQueryEnvironment<Target> getQueryEnv(OptionsParsingResult options)
+  /** Creates special output formatter for fetch that doesn't print anything */
+  private NamedThreadSafeOutputFormatterCallback<CqueryNode> createNoOutputFormatter() {
+    return new NamedThreadSafeOutputFormatterCallback<CqueryNode>() {
+      @Override
+      public String getName() {
+        return "no_output";
+      }
+
+      @Override
+      public void processOutput(Iterable<CqueryNode> partialResult) {
+        // Just do nothing!
+        // This will be later used to collect repos for vendoring
+      }
+    };
+  }
+
+  private BuildRequest createBuildRequest(
+      CommandEnvironment env, OptionsParsingResult options, List<String> targets) {
+    return BuildRequest.builder()
+        .setCommandName(env.getCommandName())
+        .setId(env.getCommandId())
+        .setOptions(options)
+        .setStartupOptions(env.getRuntime().getStartupOptionsProvider())
+        .setOutErr(env.getReporter().getOutErr())
+        .setTargets(targets)
+        .setStartTimeMillis(env.getCommandStartTime())
+        .setCheckforActionConflicts(false)
+        .setReportIncompatibleTargets(false)
+        .build();
+  }
+
+  private Parser getMainRepoMappingParser(CommandEnvironment env)
       throws RepositoryMappingResolutionException, InterruptedException {
-    boolean keepGoing = options.getOptions(KeepGoingOption.class).keepGoing;
-    LoadingPhaseThreadsOption threadsOption = options.getOptions(LoadingPhaseThreadsOption.class);
     RepositoryMapping repoMapping =
         env.getSkyframeExecutor()
-            .getMainRepoMapping(keepGoing, threadsOption.threads, env.getReporter());
-    TargetPattern.Parser targetParser =
-        new Parser(env.getRelativeWorkingDirectory(), RepositoryName.MAIN, repoMapping);
-    return QueryCommand.newQueryEnvironment(
-        env,
-        keepGoing,
-        false,
-        UniverseScope.EMPTY,
-        threadsOption.threads,
-        EnumSet.noneOf(Setting.class),
-        /* useGraphlessQuery= */ true,
-        targetParser,
-        LabelPrinter.legacy());
+            .getMainRepoMapping(
+                env.getOptions().getOptions(KeepGoingOption.class).keepGoing,
+                env.getOptions().getOptions(LoadingPhaseThreadsOption.class).threads,
+                env.getReporter());
+    return new Parser(env.getRelativeWorkingDirectory(), RepositoryName.MAIN, repoMapping);
   }
 
-  private QueryExpression createQueryExpression(
-      List<String> targets, AbstractBlazeQueryEnvironment<Target> queryEnv)
+  private QueryExpression createQueryExpression(List<String> targets)
       throws TargetFetcherException {
     String query = "deps(" + Joiner.on(" union ").join(targets) + ")";
+
+    ImmutableMap<String, QueryFunction> functions =
+        Stream.of(ConfiguredTargetQueryEnvironment.FUNCTIONS, env.getRuntime().getQueryFunctions())
+            .flatMap(Collection::stream)
+            .collect(toImmutableMap(QueryFunction::getName, Function.identity()));
+
     try {
-      return QueryExpression.parse(query, queryEnv);
+      return QueryParser.parse(query, functions);
     } catch (QuerySyntaxException e) {
       throw new TargetFetcherException(
           String.format(
@@ -121,6 +145,29 @@ private QueryExpression createQueryExpression(
     }
   }
 
+  static void injectOptionsToFetchTarget(OptionsParser optionsParser) {
+    try {
+      optionsParser.parse(
+          PriorityCategory.COMPUTED_DEFAULT,
+          "Options required to fetch target",
+          ImmutableList.of("--nobuild"));
+      optionsParser.parse(
+          PriorityCategory.COMPUTED_DEFAULT,
+          "Fetch target should include 'tags = [\"manual\"]' targets by default",
+          ImmutableList.of("--build_manual_tests"));
+      optionsParser.parse(
+          PriorityCategory.SOFTWARE_REQUIREMENT,
+          "Fetch target should not exclude test_suite rules",
+          ImmutableList.of("--noexpand_test_suites"));
+      optionsParser.parse(
+          PriorityCategory.SOFTWARE_REQUIREMENT,
+          "Fetch target should not exclude tests",
+          ImmutableList.of("--nobuild_tests_only"));
+    } catch (OptionsParsingException e) {
+      throw new IllegalStateException("Fetch target needed options failed to parse", e);
+    }
+  }
+
   static class TargetFetcherException extends Exception {
     public TargetFetcherException(String message) {
       super(message);

src/main/java/com/google/devtools/build/lib/buildtool/CqueryProcessor.java

@@ -17,6 +17,7 @@
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.cmdline.TargetPattern;
 import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
+import com.google.devtools.build.lib.query2.NamedThreadSafeOutputFormatterCallback;
 import com.google.devtools.build.lib.query2.PostAnalysisQueryEnvironment.TopLevelConfigurations;
 import com.google.devtools.build.lib.query2.cquery.ConfiguredTargetQueryEnvironment;
 import com.google.devtools.build.lib.query2.cquery.CqueryOptions;
@@ -26,14 +27,30 @@
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.WalkableGraph;
 import java.util.Collection;
+import java.util.Optional;
 import net.starlark.java.eval.StarlarkSemantics;
 
 /** Performs {@code cquery} processing. */
 public final class CqueryProcessor extends PostAnalysisQueryProcessor<ConfiguredTarget> {
 
+  /**
+   * Only passed when this is a call from a non query command like Fetch or Vendor, where we don't
+   * need the output printed
+   */
+  private Optional<NamedThreadSafeOutputFormatterCallback<CqueryNode>> noOutputFormatter;
+
   public CqueryProcessor(
       QueryExpression queryExpression, TargetPattern.Parser mainRepoTargetParser) {
     super(queryExpression, mainRepoTargetParser);
+    this.noOutputFormatter = Optional.empty();
+  }
+
+  public CqueryProcessor(
+      QueryExpression queryExpression,
+      TargetPattern.Parser mainRepoTargetParser,
+      Optional<NamedThreadSafeOutputFormatterCallback<CqueryNode>> noOutputFormatter) {
+    this(queryExpression, mainRepoTargetParser);
+    this.noOutputFormatter = noOutputFormatter;
   }
 
   @Override
@@ -66,6 +83,7 @@ protected ConfiguredTargetQueryEnvironment getQueryEnvironment(
         request.getTopLevelArtifactContext(),
         request
             .getOptions(CqueryOptions.class)
-            .getLabelPrinter(starlarkSemantics, mainRepoTargetParser.getRepoMapping()));
+            .getLabelPrinter(starlarkSemantics, mainRepoTargetParser.getRepoMapping()),
+        noOutputFormatter);
   }
 }