Move loadingPhaseRunner from BlazeRuntime to CommandEnvironment.

The loadingPhaseRunner is stateless, and moving it prevents accidentally
adding state to it. The TargetPatternEvaluator is not stateless, but is
currently retained by the SkyframePackageManager, so we still keep that state
across invocations (see follow-up CL).

--
MOS_MIGRATED_REVID=104187659

4 files changed, 37 insertions, 22 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
index b7a74c695e..1d78a79834 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -398,7 +398,7 @@ public final class BuildTool {
       }
     };
 
-    LoadingResult result = runtime.getLoadingPhaseRunner().execute(getReporter(),
+    LoadingResult result = env.getLoadingPhaseRunner().execute(getReporter(),
         env.getEventBus(), request.getTargets(), request.getLoadingOptions(),
         runtime.createBuildOptions(request).getAllLabels(), keepGoing,
         isLoadingEnabled(request), request.shouldRunTests(), callback);
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
index dbca029f67..3519317e2b 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
@@ -344,8 +344,7 @@ public class BlazeCommandDispatcher {
 
       try {
         // Notify the BlazeRuntime, so it can do some initial setup.
-        runtime.beforeCommand(commandAnnotation, env, optionsParser, commonOptions,
-            execStartTimeNanos);
+        env.beforeCommand(commandAnnotation, optionsParser, commonOptions, execStartTimeNanos);
         // Allow the command to edit options after parsing:
         command.editOptions(env, optionsParser);
       } catch (AbruptExitException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index 026b2cf3e7..d33434bd46 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -57,10 +57,8 @@ import com.google.devtools.build.lib.exec.OutputService;
 import com.google.devtools.build.lib.packages.PackageFactory;
 import com.google.devtools.build.lib.packages.Preprocessor;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
-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.profiler.AutoProfiler;
 import com.google.devtools.build.lib.profiler.MemoryProfiler;
 import com.google.devtools.build.lib.profiler.ProfilePhase;
@@ -171,7 +169,6 @@ public final class BlazeRuntime {
 
   // Always null in production! Only non-null when tests inject a custom reporter.
   private final Reporter reporter;
-  private final LoadingPhaseRunner loadingPhaseRunner;
   private final PackageFactory packageFactory;
   private final ConfigurationFactory configurationFactory;
   private final ConfiguredRuleClassProvider ruleClassProvider;
@@ -217,9 +214,6 @@ public final class BlazeRuntime {
     this.projectFileProvider = projectFileProvider;
 
     this.skyframeExecutor = skyframeExecutor;
-    this.loadingPhaseRunner = new LoadingPhaseRunner(
-        skyframeExecutor.getPackageManager(),
-        pkgFactory.getRuleClassNames());
 
     this.blazeModules = blazeModules;
     this.ruleClassProvider = ruleClassProvider;
@@ -532,10 +526,6 @@ public final class BlazeRuntime {
     return ruleClassProvider;
   }
 
-  public LoadingPhaseRunner getLoadingPhaseRunner() {
-    return loadingPhaseRunner;
-  }
-
   public Iterable<BlazeModule> getBlazeModules() {
     return blazeModules;
   }
@@ -556,13 +546,6 @@ public final class BlazeRuntime {
   }
 
   /**
-   * Returns the target pattern parser.
-   */
-  public TargetPatternEvaluator getTargetPatternEvaluator() {
-    return loadingPhaseRunner.getTargetPatternEvaluator();
-  }
-
-  /**
    * Returns reference to the lazily instantiated persistent action cache
    * instance. Note, that method may recreate instance between different build
    * requests, so return value should not be cached.
@@ -679,7 +662,7 @@ public final class BlazeRuntime {
       workspace = FileSystemUtils.getWorkingDirectory(directories.getFileSystem());
       workingDirectory = workspace;
     }
-    loadingPhaseRunner.updatePatternEvaluator(workingDirectory.relativeTo(workspace));
+    env.getLoadingPhaseRunner().updatePatternEvaluator(workingDirectory.relativeTo(workspace));
 
     env.updateClientEnv(options.clientEnv, options.ignoreClientEnv);
 
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 a229e21c09..43e1d60e3c 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,12 +32,15 @@ import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.packages.NoSuchThingException;
 import com.google.devtools.build.lib.packages.Target;
+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.skyframe.SkyframeExecutor;
 import com.google.devtools.build.lib.util.AbruptExitException;
 import com.google.devtools.build.lib.util.ExitCode;
 import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsProvider;
 
 import java.io.IOException;
@@ -61,6 +64,8 @@ public final class CommandEnvironment {
   private final EventBus eventBus;
   private final BlazeModule.ModuleEnvironment blazeModuleEnvironment;
   private final Map<String, String> clientEnv = new HashMap<>();
+
+  private final LoadingPhaseRunner loadingPhaseRunner;
   private final BuildView view;
 
   private String outputFileSystem;
@@ -90,6 +95,10 @@ public final class CommandEnvironment {
     this.reporter = reporter == null ? new Reporter() : reporter;
     this.eventBus = eventBus;
     this.blazeModuleEnvironment = new BlazeModuleEnvironment();
+
+    this.loadingPhaseRunner = new LoadingPhaseRunner(
+        runtime.getSkyframeExecutor().getPackageManager(),
+        runtime.getPackageFactory().getRuleClassNames());
     this.view = new BuildView(runtime.getDirectories(), runtime.getRuleClassProvider(),
         runtime.getSkyframeExecutor(), runtime.getCoverageReportActionFactory());
   }
@@ -140,6 +149,17 @@ public final class CommandEnvironment {
     return runtime.getPackageManager();
   }
 
+  public LoadingPhaseRunner getLoadingPhaseRunner() {
+    return loadingPhaseRunner;
+  }
+
+  /**
+   * Returns the target pattern parser.
+   */
+  public TargetPatternEvaluator getTargetPatternEvaluator() {
+    return loadingPhaseRunner.getTargetPatternEvaluator();
+  }
+
   public BuildView getView() {
     return view;
   }
@@ -179,7 +199,7 @@ public final class CommandEnvironment {
     BuildOptions buildOptions = runtime.createBuildOptions(optionsProvider);
     boolean keepGoing = optionsProvider.getOptions(BuildView.Options.class).keepGoing;
     boolean loadingSuccessful =
-        runtime.getLoadingPhaseRunner().loadForConfigurations(reporter,
+        loadingPhaseRunner.loadForConfigurations(reporter,
             ImmutableSet.copyOf(buildOptions.getAllLabels().values()),
             keepGoing);
     if (!loadingSuccessful) {
@@ -240,4 +260,17 @@ public final class CommandEnvironment {
   public String getOutputFileSystem() {
     return outputFileSystem;
   }
+
+  /**
+   * Hook method called by the BlazeCommandDispatcher prior to the dispatch of
+   * each command.
+   *
+   * @param options The CommonCommandOptions used by every command.
+   * @throws AbruptExitException if this command is unsuitable to be run as specified
+   */
+  void beforeCommand(Command command, OptionsParser optionsParser,
+      CommonCommandOptions options, long execStartTimeNanos)
+      throws AbruptExitException {
+    runtime.beforeCommand(command, this, optionsParser, options, execStartTimeNanos);
+  }
 }