Use build with no-build for fetching targets (Fixes #21728)

Using cquery for fetching targets caused some problems #21728 as cquery is not prepared to deal with targets including `...` Updated the logic to use build command with no-build option, which should operate the same and actually do less work than cquery PiperOrigin-RevId: 619540572 Change-Id: Id26a61229c13bd12373478fe17e67200f2760be2
bazelbuild · Mar 28, 2024 · 77f9f08 · 77f9f08
1 parent 9d17bef
commit 77f9f08
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 257 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/FetchCommand.java b/src/main/java/com/google/devtools/build/lib/bazel/commands/FetchCommand.java
@@ -30,7 +30,6 @@
 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;
@@ -62,15 +61,14 @@
     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,
+    shortDescription = "Fetches external repositories that are prerequisites to the targets.",
+    help = "resource:fetch.txt",
     completion = "label")
 public final class FetchCommand implements BlazeCommand {
 
@@ -80,7 +78,7 @@ public final class FetchCommand implements BlazeCommand {
   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);
+      TargetFetcher.injectNoBuildOption(optionsParser);
     }
   }
 
@@ -217,18 +215,13 @@ private BlazeCommandResult fetchRepos(
   }
 
   private BlazeCommandResult fetchTarget(
-      CommandEnvironment env, OptionsParsingResult options, List<String> targets)
-      throws InterruptedException {
+      CommandEnvironment env, OptionsParsingResult options, List<String> targets) {
     try {
       TargetFetcher.fetchTargets(env, options, targets);
     } catch (TargetFetcherException e) {
       return createFailedBlazeCommandResult(
           env.getReporter(), Code.QUERY_EVALUATION_ERROR, e.getMessage());
-    } catch (RepositoryMappingResolutionException e) {
-      return createFailedBlazeCommandResult(
-          env.getReporter(), e.getMessage(), e.getDetailedExitCode());
     }
-
     env.getReporter()
         .handle(Event.info("All external dependencies for these targets fetched successfully."));
     return BlazeCommandResult.success();

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/TargetFetcher.java b/src/main/java/com/google/devtools/build/lib/bazel/commands/TargetFetcher.java
@@ -14,39 +14,16 @@
 
 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.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.runtime.CommandEnvironment;
-import com.google.devtools.build.lib.runtime.KeepGoingOption;
-import com.google.devtools.build.lib.runtime.LoadingPhaseThreadsOption;
-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.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 {
@@ -56,115 +33,42 @@ private TargetFetcher(CommandEnvironment env) {
     this.env = env;
   }
 
-  /** Uses cquery to find and fetch all repos needed to build these targets */
+  /** Creates a no-build build request to fetch all repos needed to build these targets */
   public static void fetchTargets(
       CommandEnvironment env, OptionsParsingResult options, List<String> targets)
-      throws RepositoryMappingResolutionException, InterruptedException, TargetFetcherException {
+      throws TargetFetcherException {
     new TargetFetcher(env).fetchTargets(options, targets);
   }
 
   private void fetchTargets(OptionsParsingResult options, List<String> targets)
-      throws InterruptedException, TargetFetcherException, RepositoryMappingResolutionException {
-    QueryExpression expr = createQueryExpression(targets);
-    BuildRequest request = createBuildRequest(env, options, targets);
-    TargetPattern.Parser mainRepoTargetParser = getMainRepoMappingParser(env);
-
-    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. "
-                  + " Ignoring errors",
-              expr));
-    }
-  }
-
-  /** 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 {
-    RepositoryMapping repoMapping =
-        env.getSkyframeExecutor()
-            .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)
       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 QueryParser.parse(query, functions);
-    } catch (QuerySyntaxException e) {
+    BuildRequest request =
+        BuildRequest.builder()
+            .setCommandName(env.getCommandName())
+            .setId(env.getCommandId())
+            .setOptions(options)
+            .setStartupOptions(env.getRuntime().getStartupOptionsProvider())
+            .setOutErr(env.getReporter().getOutErr())
+            .setTargets(targets)
+            .setStartTimeMillis(env.getCommandStartTime())
+            .build();
+
+    BuildResult result = new BuildTool(env).processRequest(request, null);
+    if (!result.getSuccess()) {
       throw new TargetFetcherException(
-          String.format(
-              "Fetching target dependencies for %s encountered an error: %s",
-              QueryExpression.truncate(query), e.getMessage()));
+          "Fetching some target dependencies failed with errors: "
+              + result.getDetailedExitCode().getFailureDetail().getMessage());
     }
   }
 
-  static void injectOptionsToFetchTarget(OptionsParser optionsParser) {
+  static void injectNoBuildOption(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);
+      throw new IllegalStateException("Fetch target needed option failed to parse", e);
     }
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/CqueryProcessor.java b/src/main/java/com/google/devtools/build/lib/buildtool/CqueryProcessor.java
@@ -16,7 +16,6 @@
 import com.google.common.collect.ImmutableList;
 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.common.CqueryNode;
 import com.google.devtools.build.lib.query2.cquery.ConfiguredTargetQueryEnvironment;
@@ -27,30 +26,14 @@
 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<CqueryNode> {
 
-  /**
-   * 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
@@ -83,7 +66,6 @@ protected ConfiguredTargetQueryEnvironment getQueryEnvironment(
         request.getTopLevelArtifactContext(),
         request
             .getOptions(CqueryOptions.class)
-            .getLabelPrinter(starlarkSemantics, mainRepoTargetParser.getRepoMapping()),
-        noOutputFormatter);
+            .getLabelPrinter(starlarkSemantics, mainRepoTargetParser.getRepoMapping()));
   }
 }