Vendor command small refactors (related #19563)

PiperOrigin-RevId: 607312047
Change-Id: I363badb12d54005ab8fda48806a70549eaa788f3

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

@@ -187,7 +187,7 @@ private BlazeCommandResult fetchRepos(
       throws InterruptedException {
     try {
       ImmutableMap<RepositoryName, RepositoryDirectoryValue> repositoryNamesAndValues =
-          RepositoryFetcher.fetchRepos(repos, env, env.getSkyframeExecutor(), threadsOption);
+          RepositoryFetcher.fetchRepos(repos, env, threadsOption);
       String notFoundRepos =
           repositoryNamesAndValues.values().stream()
               .filter(value -> !value.repositoryExists())

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

@@ -28,7 +28,6 @@
 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.build.lib.skyframe.SkyframeExecutor;
 import com.google.devtools.build.skyframe.EvaluationContext;
 import com.google.devtools.build.skyframe.EvaluationResult;
 import com.google.devtools.build.skyframe.SkyKey;
@@ -40,27 +39,23 @@
 final class RepositoryFetcher {
 
   private final CommandEnvironment env;
-  private final SkyframeExecutor skyframeExecutor;
   private final LoadingPhaseThreadsOption threadsOption;
 
   private RepositoryFetcher(
       CommandEnvironment env,
-      SkyframeExecutor skyframeExecutor,
       LoadingPhaseThreadsOption threadsOption) {
     this.env = env;
-    this.skyframeExecutor = skyframeExecutor;
     this.threadsOption = threadsOption;
   }
 
   static ImmutableMap<RepositoryName, RepositoryDirectoryValue> fetchRepos(
       List<String> repos,
       CommandEnvironment env,
-      SkyframeExecutor skyframeExecutor,
       LoadingPhaseThreadsOption threadsOption)
       throws RepositoryMappingResolutionException,
           InterruptedException,
           RepositoryFetcherException {
-    return new RepositoryFetcher(env, skyframeExecutor, threadsOption).fetchRepos(repos);
+    return new RepositoryFetcher(env, threadsOption).fetchRepos(repos);
   }
 
   private ImmutableMap<RepositoryName, RepositoryDirectoryValue> fetchRepos(List<String> repos)
@@ -88,7 +83,7 @@ private EvaluationResult<SkyValue> evaluateFetch(ImmutableSet<RepositoryName> re
     ImmutableSet<SkyKey> repoDelegatorKeys =
         reposnames.stream().map(RepositoryDirectoryValue::key).collect(toImmutableSet());
     EvaluationResult<SkyValue> evaluationResult =
-        skyframeExecutor.prepareAndGet(repoDelegatorKeys, evaluationContext);
+        env.getSkyframeExecutor().prepareAndGet(repoDelegatorKeys, evaluationContext);
     if (evaluationResult.hasError()) {
       Exception e = evaluationResult.getError().getException();
       throw new RepositoryFetcherException(

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

@@ -43,7 +43,6 @@
 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.skyframe.SkyframeExecutor;
 import com.google.devtools.build.lib.util.AbruptExitException;
 import com.google.devtools.build.lib.util.DetailedExitCode;
 import com.google.devtools.build.lib.util.InterruptedFailureDetails;
@@ -61,6 +60,7 @@
 import java.util.List;
 import java.util.Map.Entry;
 import java.util.Objects;
+import javax.annotation.Nullable;
 
 /** Fetches external repositories into a specified directory. */
 @Command(
@@ -83,22 +83,9 @@ public final class VendorCommand implements BlazeCommand {
 
   @Override
   public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult options) {
-    RepositoryOptions repoOptions = options.getOptions(RepositoryOptions.class);
-    if (!options.getOptions(BuildLanguageOptions.class).enableBzlmod) {
-      return createFailedBlazeCommandResult(
-          env.getReporter(),
-          "Bzlmod has to be enabled for vendoring to work, run with --enable_bzlmod");
-    }
-    if (repoOptions.vendorDirectory == null) {
-      return createFailedBlazeCommandResult(
-          env.getReporter(),
-          Code.OPTIONS_INVALID,
-          "You cannot run vendor without specifying --vendor_dir");
-    }
-    PackageOptions pkgOptions = options.getOptions(PackageOptions.class);
-    if (!pkgOptions.fetch) {
-      return createFailedBlazeCommandResult(
-          env.getReporter(), Code.OPTIONS_INVALID, "You cannot run vendor with --nofetch");
+    BlazeCommandResult invalidResult = validateOptions(env, options);
+    if (invalidResult != null) {
+      return invalidResult;
     }
 
     env.getEventBus()
@@ -108,25 +95,25 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti
                 env.getCommandStartTime(),
                 /* separateFinishedEvent= */ true,
                 /* showProgress= */ true,
-                /* id= */ null));
+                env.getCommandId().toString()));
 
-    SkyframeExecutor skyframeExecutor = env.getSkyframeExecutor();
     // IS_VENDOR_COMMAND & VENDOR_DIR is already injected in "BazelRepositoryModule", we just need
     // to update this value for the delegator function to recognize this call is from VendorCommand
-    skyframeExecutor.injectExtraPrecomputedValues(
-        ImmutableList.of(
-            PrecomputedValue.injected(RepositoryDelegatorFunction.IS_VENDOR_COMMAND, true)));
+    env.getSkyframeExecutor()
+        .injectExtraPrecomputedValues(
+            ImmutableList.of(
+                PrecomputedValue.injected(RepositoryDelegatorFunction.IS_VENDOR_COMMAND, true)));
 
     BlazeCommandResult result;
     VendorOptions vendorOptions = options.getOptions(VendorOptions.class);
+    PathFragment vendorDirectory = options.getOptions(RepositoryOptions.class).vendorDirectory;
+    LoadingPhaseThreadsOption threadsOption = options.getOptions(LoadingPhaseThreadsOption.class);
     try {
       env.syncPackageLoading(options);
       if (!vendorOptions.repos.isEmpty()) {
-        result =
-            vendorRepos(
-                skyframeExecutor, env, options, vendorOptions.repos, repoOptions.vendorDirectory);
+        result = vendorRepos(env, threadsOption, vendorOptions.repos, vendorDirectory);
       } else {
-        result = vendorAll(skyframeExecutor, env, options, repoOptions.vendorDirectory);
+        result = vendorAll(env, threadsOption, vendorDirectory);
       }
     } catch (AbruptExitException e) {
       return createFailedBlazeCommandResult(
@@ -146,22 +133,38 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti
     return result;
   }
 
+  @Nullable
+  private BlazeCommandResult validateOptions(CommandEnvironment env, OptionsParsingResult options) {
+    if (!options.getOptions(BuildLanguageOptions.class).enableBzlmod) {
+      return createFailedBlazeCommandResult(
+          env.getReporter(),
+          "Bzlmod has to be enabled for vendoring to work, run with --enable_bzlmod");
+    }
+    if (options.getOptions(RepositoryOptions.class).vendorDirectory == null) {
+      return createFailedBlazeCommandResult(
+          env.getReporter(),
+          Code.OPTIONS_INVALID,
+          "You cannot run vendor without specifying --vendor_dir");
+    }
+    if (!options.getOptions(PackageOptions.class).fetch) {
+      return createFailedBlazeCommandResult(
+          env.getReporter(), Code.OPTIONS_INVALID, "You cannot run vendor with --nofetch");
+    }
+    return null;
+  }
+
   private BlazeCommandResult vendorAll(
-      SkyframeExecutor skyframeExecutor,
-      CommandEnvironment env,
-      OptionsParsingResult options,
-      PathFragment vendorDirectory)
+      CommandEnvironment env, LoadingPhaseThreadsOption threadsOption, PathFragment vendorDirectory)
       throws InterruptedException, IOException {
-    LoadingPhaseThreadsOption threadsOption = options.getOptions(LoadingPhaseThreadsOption.class);
     EvaluationContext evaluationContext =
         EvaluationContext.newBuilder()
             .setParallelism(threadsOption.threads)
             .setEventHandler(env.getReporter())
             .build();
 
     SkyKey fetchKey = BazelFetchAllValue.key(/* configureEnabled= */ false);
-      EvaluationResult<SkyValue> evaluationResult =
-          skyframeExecutor.prepareAndGet(ImmutableSet.of(fetchKey), evaluationContext);
+    EvaluationResult<SkyValue> evaluationResult =
+        env.getSkyframeExecutor().prepareAndGet(ImmutableSet.of(fetchKey), evaluationContext);
       if (evaluationResult.hasError()) {
         Exception e = evaluationResult.getError().getException();
         return createFailedBlazeCommandResult(
@@ -175,17 +178,14 @@ private BlazeCommandResult vendorAll(
   }
 
   private BlazeCommandResult vendorRepos(
-      SkyframeExecutor skyframeExecutor,
       CommandEnvironment env,
-      OptionsParsingResult options,
+      LoadingPhaseThreadsOption threadsOption,
       List<String> repos,
       PathFragment vendorDirectory)
       throws InterruptedException, IOException {
     ImmutableMap<RepositoryName, RepositoryDirectoryValue> repositoryNamesAndValues;
     try {
-      repositoryNamesAndValues =
-          RepositoryFetcher.fetchRepos(
-              repos, env, skyframeExecutor, options.getOptions(LoadingPhaseThreadsOption.class));
+      repositoryNamesAndValues = RepositoryFetcher.fetchRepos(repos, env, threadsOption);
     } catch (RepositoryMappingResolutionException e) {
       return createFailedBlazeCommandResult(
           env.getReporter(), "Invalid repo name: " + e.getMessage(), e.getDetailedExitCode());