Move LoadingPhaseRunner.loadForConfigurations to CommandEnvironment.

It's unclear if the method is still necessary with Skyframe, but I don't want
to investigate in detail right now.

--
MOS_MIGRATED_REVID=106838896

2 files changed, 18 insertions, 17 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunner.java
index e206b3b37b..04da44844f 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunner.java
@@ -128,22 +128,6 @@ public final class LoadingPhaseRunner {
   }
 
   /**
-   * This method only exists for the benefit of InfoCommand, which needs to construct
-   * a {@code BuildConfigurationCollection} without running a full loading phase. Don't
-   * add any more clients; instead, we should change info so that it doesn't need the configuration.
-   */
-  public boolean loadForConfigurations(EventHandler eventHandler,
-      Set<Label> labelsToLoad, boolean keepGoing) throws InterruptedException {
-    // Use a new Label Visitor here to avoid erasing the cache on the existing one.
-    TransitivePackageLoader transitivePackageLoader = packageManager.newTransitiveLoader();
-    boolean loadingSuccessful = transitivePackageLoader.sync(
-        eventHandler, ImmutableSet.<Target>of(),
-        labelsToLoad, keepGoing, /*parallelThreads=*/10,
-        /*maxDepth=*/Integer.MAX_VALUE);
-    return loadingSuccessful;
-  }
-
-  /**
    * Performs target pattern evaluation, test suite expansion (if requested), and loads the
    * transitive closure of the resulting targets as well as of the targets needed to use the
    * given build configuration provider.
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
index d4516a021c..bd46e11307 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
@@ -32,6 +32,7 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.DefaultsPackage;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.exec.OutputService;
 import com.google.devtools.build.lib.packages.NoSuchThingException;
@@ -40,6 +41,7 @@ import com.google.devtools.build.lib.pkgcache.LoadingPhaseRunner;
 import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
 import com.google.devtools.build.lib.pkgcache.PackageManager;
 import com.google.devtools.build.lib.pkgcache.TargetPatternEvaluator;
+import com.google.devtools.build.lib.pkgcache.TransitivePackageLoader;
 import com.google.devtools.build.lib.profiler.AutoProfiler;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
@@ -58,6 +60,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.TreeMap;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicReference;
@@ -231,7 +234,7 @@ public final class CommandEnvironment {
     BuildOptions buildOptions = runtime.createBuildOptions(optionsProvider);
     boolean keepGoing = optionsProvider.getOptions(BuildView.Options.class).keepGoing;
     boolean loadingSuccessful =
-        loadingPhaseRunner.loadForConfigurations(reporter,
+        loadForConfigurations(reporter,
             ImmutableSet.copyOf(buildOptions.getAllLabels().values()),
             keepGoing);
     if (!loadingSuccessful) {
@@ -241,6 +244,20 @@ public final class CommandEnvironment {
         buildOptions, runtime.getDirectories(), ImmutableSet.<String>of(), keepGoing);
   }
 
+  // TODO(ulfjack): Do we even need this method? With Skyframe, the config creation should
+  // implicitly trigger any necessary loading.
+  private boolean loadForConfigurations(EventHandler eventHandler,
+      Set<Label> labelsToLoad, boolean keepGoing) throws InterruptedException {
+    // Use a new Label Visitor here to avoid erasing the cache on the existing one.
+    TransitivePackageLoader transitivePackageLoader =
+        runtime.getSkyframeExecutor().getPackageManager().newTransitiveLoader();
+    boolean loadingSuccessful = transitivePackageLoader.sync(
+        eventHandler, ImmutableSet.<Target>of(),
+        labelsToLoad, keepGoing, /*parallelThreads=*/10,
+        /*maxDepth=*/Integer.MAX_VALUE);
+    return loadingSuccessful;
+  }
+
   /**
    * Hook method called by the BlazeCommandDispatcher right before the dispatch
    * of each command ends (while its outcome can still be modified).