Move most of BlazeRuntime.beforeCommand() to CommandEnvironment.

--
MOS_MIGRATED_REVID=105841866

2 files changed, 83 insertions, 89 deletions

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 0640fb696d..8339240329 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
@@ -40,17 +40,14 @@ import com.google.devtools.build.lib.actions.cache.CompactPersistentActionCache;
 import com.google.devtools.build.lib.actions.cache.NullActionCache;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.BlazeVersionInfo;
-import com.google.devtools.build.lib.analysis.BuildView;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.analysis.WorkspaceStatusAction;
 import com.google.devtools.build.lib.analysis.config.BinTools;
-import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFactory;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.OutputFilter;
 import com.google.devtools.build.lib.events.Reporter;
-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;
@@ -126,7 +123,6 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.TreeMap;
 import java.util.UUID;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
@@ -154,40 +150,36 @@ public final class BlazeRuntime {
 
   private static final Logger LOG = Logger.getLogger(BlazeRuntime.class.getName());
 
-  private final BlazeDirectories directories;
-
-  /** The execution time range of the previous build command in this server, if any. */
-  @Nullable
-  private Range<Long> lastExecutionRange = null;
-
-  private final SkyframeExecutor skyframeExecutor;
+  private final Iterable<BlazeModule> blazeModules;
+  private final Map<String, BlazeCommand> commandMap = new LinkedHashMap<>();
+  private final Clock clock;
 
   // Always null in production! Only non-null when tests inject a custom reporter.
   private final Reporter reporter;
   private final PackageFactory packageFactory;
   private final ConfigurationFactory configurationFactory;
   private final ConfiguredRuleClassProvider ruleClassProvider;
-  private ActionCache actionCache;
   private final TimestampGranularityMonitor timestampGranularityMonitor;
-  private final Clock clock;
-
-  private final Iterable<BlazeModule> blazeModules;
 
   private final AtomicInteger storedExitCode = new AtomicInteger();
 
   // We pass this through here to make it available to the MasterLogWriter.
   private final OptionsProvider startupOptionsProvider;
 
-  private final Map<String, BlazeCommand> commandMap = new LinkedHashMap<>();
-
   private final SubscriberExceptionHandler eventBusExceptionHandler;
-
   private final BinTools binTools;
-
   private final WorkspaceStatusAction.Factory workspaceStatusActionFactory;
-
   private final ProjectFile.Provider projectFileProvider;
 
+  // Workspace state (currently exactly one workspace per server)
+  private final BlazeDirectories directories;
+  private final SkyframeExecutor skyframeExecutor;
+  /** The action cache is loaded lazily on the first build command. */
+  private ActionCache actionCache;
+  /** The execution time range of the previous build command in this server, if any. */
+  @Nullable
+  private Range<Long> lastExecutionRange = null;
+
   private BlazeRuntime(BlazeDirectories directories, Reporter reporter,
       WorkspaceStatusAction.Factory workspaceStatusActionFactory,
       final SkyframeExecutor skyframeExecutor,
@@ -198,24 +190,26 @@ public final class BlazeRuntime {
       SubscriberExceptionHandler eventBusExceptionHandler,
       BinTools binTools, ProjectFile.Provider projectFileProvider,
       Iterable<BlazeCommand> commands) {
+    // Server state
+    this.blazeModules = blazeModules;
+    overrideCommands(commands);
+
     this.workspaceStatusActionFactory = workspaceStatusActionFactory;
-    this.directories = directories;
     this.reporter = reporter;
     this.packageFactory = pkgFactory;
     this.binTools = binTools;
     this.projectFileProvider = projectFileProvider;
 
-    this.skyframeExecutor = skyframeExecutor;
-
-    this.blazeModules = blazeModules;
     this.ruleClassProvider = ruleClassProvider;
     this.configurationFactory = configurationFactory;
     this.clock = clock;
     this.timestampGranularityMonitor = Preconditions.checkNotNull(timestampGranularityMonitor);
     this.startupOptionsProvider = startupOptionsProvider;
-
     this.eventBusExceptionHandler = eventBusExceptionHandler;
-    overrideCommands(commands);
+
+    // Workspace state
+    this.directories = directories;
+    this.skyframeExecutor = skyframeExecutor;
 
     if (inWorkspace()) {
       writeOutputBaseReadmeFile();
@@ -577,32 +571,8 @@ public final class BlazeRuntime {
    * @param options The CommonCommandOptions used by every command.
    * @throws AbruptExitException if this command is unsuitable to be run as specified
    */
-  void beforeCommand(Command command, CommandEnvironment env, OptionsParser optionsParser,
-      CommonCommandOptions options, long execStartTimeNanos)
+  void beforeCommand(CommandEnvironment env, CommonCommandOptions options, long execStartTimeNanos)
       throws AbruptExitException {
-    env.setOutputFileSystem(env.determineOutputFileSystem());
-
-    // Ensure that the working directory will be under the workspace directory.
-    Path workspace = getWorkspace();
-    Path workingDirectory;
-    if (inWorkspace()) {
-      workingDirectory = workspace.getRelative(options.clientCwd);
-    } else {
-      workspace = FileSystemUtils.getWorkingDirectory(directories.getFileSystem());
-      workingDirectory = workspace;
-    }
-    env.getLoadingPhaseRunner().updatePatternEvaluator(workingDirectory.relativeTo(workspace));
-    env.setWorkingDirectory(workingDirectory);
-
-    env.updateClientEnv(options.clientEnv, options.ignoreClientEnv);
-
-    // Fail fast in the case where a Blaze command forgets to install the package path correctly.
-    skyframeExecutor.setActive(false);
-    // Let skyframe figure out if it needs to store graph edges for this build.
-    skyframeExecutor.decideKeepIncrementalState(
-        startupOptionsProvider.getOptions(BlazeServerStartupOptions.class).batch,
-        optionsParser.getOptions(BuildView.Options.class));
-
     // Conditionally enable profiling
     // We need to compensate for launchTimeNanos (measurements taken outside of the jvm).
     long startupTimeNanos = options.startupTime * 1000000L;
@@ -628,37 +598,6 @@ public final class BlazeRuntime {
       }
     }
 
-    if (command.builds()) {
-      Map<String, String> testEnv = new TreeMap<>();
-      for (Map.Entry<String, String> entry :
-          optionsParser.getOptions(BuildConfiguration.Options.class).testEnvironment) {
-        testEnv.put(entry.getKey(), entry.getValue());
-      }
-
-      try {
-        for (Map.Entry<String, String> entry : testEnv.entrySet()) {
-          if (entry.getValue() == null) {
-            String clientValue = env.getClientEnv().get(entry.getKey());
-            if (clientValue != null) {
-              optionsParser.parse(OptionPriority.SOFTWARE_REQUIREMENT,
-                  "test environment variable from client environment",
-                  ImmutableList.of(
-                      "--test_env=" + entry.getKey() + "="
-                          + env.getClientEnv().get(entry.getKey())));
-            }
-          }
-        }
-      } catch (OptionsParsingException e) {
-        throw new IllegalStateException(e);
-      }
-    }
-    for (BlazeModule module : blazeModules) {
-      module.handleOptions(optionsParser);
-    }
-
-    env.getEventBus().post(
-        new CommandStartEvent(command.name(), env.getCommandId(), env.getClientEnv(),
-            workingDirectory));
     // Initialize exit code to dummy value for afterCommand.
     storedExitCode.set(ExitCode.RESERVED.getNumericExitCode());
   }
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 806fd01282..af2d05a611 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
@@ -18,6 +18,7 @@ import static com.google.devtools.build.lib.profiler.AutoProfiler.profiled;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Range;
 import com.google.common.eventbus.EventBus;
@@ -47,7 +48,9 @@ import com.google.devtools.build.lib.util.AbruptExitException;
 import com.google.devtools.build.lib.util.ExitCode;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.common.options.OptionPriority;
 import com.google.devtools.common.options.OptionsParser;
+import com.google.devtools.common.options.OptionsParsingException;
 import com.google.devtools.common.options.OptionsProvider;
 
 import java.io.IOException;
@@ -56,6 +59,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.TreeMap;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicReference;
 
@@ -296,10 +300,6 @@ public final class CommandEnvironment {
     return commandStartTime;
   }
 
-  void setOutputFileSystem(String outputFileSystem) {
-    this.outputFileSystem = outputFileSystem;
-  }
-
   void setWorkingDirectory(Path workingDirectory) {
     this.workingDirectory = workingDirectory;
   }
@@ -338,18 +338,73 @@ public final class CommandEnvironment {
       }
     }
 
-    getSkyframeExecutor().setBatchStatter(outputService == null
+    SkyframeExecutor skyframeExecutor = getSkyframeExecutor();
+    skyframeExecutor.setBatchStatter(outputService == null
         ? null
         : outputService.getBatchStatter());
 
-    runtime.beforeCommand(command, this, optionsParser, options, execStartTimeNanos);
+    this.outputFileSystem = determineOutputFileSystem();
+
+    // Ensure that the working directory will be under the workspace directory.
+    Path workspace = runtime.getWorkspace();
+    Path workingDirectory;
+    if (runtime.inWorkspace()) {
+      workingDirectory = workspace.getRelative(options.clientCwd);
+    } else {
+      workspace = FileSystemUtils.getWorkingDirectory(runtime.getDirectories().getFileSystem());
+      workingDirectory = workspace;
+    }
+    loadingPhaseRunner.updatePatternEvaluator(workingDirectory.relativeTo(workspace));
+    this.workingDirectory = workingDirectory;
+
+    updateClientEnv(options.clientEnv, options.ignoreClientEnv);
+
+    // Fail fast in the case where a Blaze command forgets to install the package path correctly.
+    skyframeExecutor.setActive(false);
+    // Let skyframe figure out if it needs to store graph edges for this build.
+    skyframeExecutor.decideKeepIncrementalState(
+        runtime.getStartupOptionsProvider().getOptions(BlazeServerStartupOptions.class).batch,
+        optionsParser.getOptions(BuildView.Options.class));
+
+    // Start the performance and memory profilers.
+    runtime.beforeCommand(this, options, execStartTimeNanos);
+
+    if (command.builds()) {
+      Map<String, String> testEnv = new TreeMap<>();
+      for (Map.Entry<String, String> entry :
+          optionsParser.getOptions(BuildConfiguration.Options.class).testEnvironment) {
+        testEnv.put(entry.getKey(), entry.getValue());
+      }
+
+      try {
+        for (Map.Entry<String, String> entry : testEnv.entrySet()) {
+          if (entry.getValue() == null) {
+            String clientValue = clientEnv.get(entry.getKey());
+            if (clientValue != null) {
+              optionsParser.parse(OptionPriority.SOFTWARE_REQUIREMENT,
+                  "test environment variable from client environment",
+                  ImmutableList.of(
+                      "--test_env=" + entry.getKey() + "=" + clientEnv.get(entry.getKey())));
+            }
+          }
+        }
+      } catch (OptionsParsingException e) {
+        throw new IllegalStateException(e);
+      }
+    }
+    for (BlazeModule module : runtime.getBlazeModules()) {
+      module.handleOptions(optionsParser);
+    }
+
+    eventBus.post(
+        new CommandStartEvent(command.name(), commandId, getClientEnv(), workingDirectory));
   }
 
   /**
    * Figures out what file system we are writing output to. Here we use
    * outputBase instead of outputPath because we need a file system to create the latter.
    */
-  String determineOutputFileSystem() {
+  private String determineOutputFileSystem() {
     if (getOutputService() != null) {
       return getOutputService().getFilesSystemName();
     }