Move getOutputFilesystem to CommandEnvironment; refactor BlazeRuntime commands.

This change makes it so commands are no longer both stored in the BlazeRuntime
and in the BlazeCommandDispatcher. Instead, they are only stored in
BlazeRuntime and usually passed there during construction. We have some tests
where this is tricky, so I'm keeping the old code path for now.

--
MOS_MIGRATED_REVID=103364581

4 files changed, 69 insertions, 63 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 597892af3b..256373cdac 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
@@ -152,7 +152,7 @@ public final class BuildTool {
     LoadingResult loadingResult = null;
     BuildConfigurationCollection configurations = null;
     try {
-      env.getEventBus().post(new BuildStartingEvent(runtime.getOutputFileSystem(), request));
+      env.getEventBus().post(new BuildStartingEvent(env.getOutputFileSystem(), request));
       LOG.info("Build identifier: " + request.getId());
       executionTool = new ExecutionTool(env, request);
       if (needsExecutionPhase(request.getBuildOptions())) {
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 5629c405cc..9f701b1999 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
@@ -44,11 +44,9 @@ import java.io.IOException;
 import java.io.OutputStream;
 import java.io.PrintStream;
 import java.util.ArrayList;
-import java.util.Collections;
-import java.util.LinkedHashMap;
+import java.util.Arrays;
 import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 import java.util.logging.Level;
 
@@ -92,50 +90,25 @@ public class BlazeCommandDispatcher {
   }
 
   private final BlazeRuntime runtime;
-  private final Map<String, BlazeCommand> commandsByName = new LinkedHashMap<>();
 
   private OutputStream logOutputStream = null;
 
   /**
-   * Create a Blaze dispatcher that uses the specified {@code BlazeRuntime}
-   * instance, and no default options, and delegates to {@code commands} as
-   * appropriate.
+   * Create a Blaze dispatcher that uses the specified {@code BlazeRuntime} instance, but overrides
+   * the command map with the given commands (plus any commands from modules).
    */
   @VisibleForTesting
   public BlazeCommandDispatcher(BlazeRuntime runtime, BlazeCommand... commands) {
-    this(runtime, ImmutableList.copyOf(commands));
+    this(runtime);
+    runtime.overrideCommands(Arrays.asList(commands));
   }
 
   /**
-   * Create a Blaze dispatcher that uses the specified {@code BlazeRuntime}
-   * instance, and delegates to {@code commands} as appropriate.
+   * Create a Blaze dispatcher that uses the specified {@code BlazeRuntime} instance.
    */
-  public BlazeCommandDispatcher(BlazeRuntime runtime, Iterable<BlazeCommand> commands) {
+  @VisibleForTesting
+  public BlazeCommandDispatcher(BlazeRuntime runtime) {
     this.runtime = runtime;
-    for (BlazeCommand command : commands) {
-      addCommandByName(command);
-    }
-
-    for (BlazeModule module : runtime.getBlazeModules()) {
-      for (BlazeCommand command : module.getCommands()) {
-        addCommandByName(command);
-      }
-    }
-
-    runtime.setCommandMap(commandsByName);
-  }
-
-  /**
-   * Adds the given command under the given name to the map of commands.
-   *
-   * @throws AssertionError if the name is already used by another command.
-   */
-  private void addCommandByName(BlazeCommand command) {
-    String name = command.getClass().getAnnotation(Command.class).name();
-    if (commandsByName.containsKey(name)) {
-      throw new IllegalStateException("Command name or alias " + name + " is already used.");
-    }
-    commandsByName.put(name, command);
   }
 
   /**
@@ -199,7 +172,7 @@ public class BlazeCommandDispatcher {
     CommonCommandOptions rcFileOptions = optionsParser.getOptions(CommonCommandOptions.class);
     List<Pair<String, ListMultimap<String, String>>> optionsMap =
         getOptionsMap(outErr, rcFileOptions.rcSource, rcFileOptions.optionsOverrides,
-            commandsByName.keySet());
+            runtime.getCommandMap().keySet());
 
     parseOptionsForCommand(rcfileNotes, commandAnnotation, optionsParser, optionsMap, null);
 
@@ -250,7 +223,7 @@ public class BlazeCommandDispatcher {
       commandName = "help";
     }
 
-    BlazeCommand command = commandsByName.get(commandName);
+    BlazeCommand command = runtime.getCommandMap().get(commandName);
     if (command == null) {
       outErr.printErrLn(String.format(
           "Command '%s' not found. Try '%s help'.", commandName, Constants.PRODUCT_NAME));
@@ -657,13 +630,6 @@ public class BlazeCommandDispatcher {
   }
 
   /**
-   * The map from command names to commands that this dispatcher dispatches to.
-   */
-  Map<String, BlazeCommand> getCommandsByName() {
-    return Collections.unmodifiableMap(commandsByName);
-  }
-
-  /**
    * Shuts down all the registered commands to give them a chance to cleanup or
    * close resources. Should be called by the owner of this command dispatcher
    * in all termination cases.
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 f4886aace7..7d2cfae097 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
@@ -26,6 +26,7 @@ import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Range;
 import com.google.common.collect.Sets;
@@ -127,6 +128,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Date;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -188,8 +190,7 @@ public final class BlazeRuntime {
   // We pass this through here to make it available to the MasterLogWriter.
   private final OptionsProvider startupOptionsProvider;
 
-  private String outputFileSystem;
-  private Map<String, BlazeCommand> commandMap;
+  private final Map<String, BlazeCommand> commandMap = new LinkedHashMap<>();
 
   private final SubscriberExceptionHandler eventBusExceptionHandler;
 
@@ -207,7 +208,8 @@ public final class BlazeRuntime {
       OptionsProvider startupOptionsProvider, Iterable<BlazeModule> blazeModules,
       TimestampGranularityMonitor timestampGranularityMonitor,
       SubscriberExceptionHandler eventBusExceptionHandler,
-      BinTools binTools, ProjectFile.Provider projectFileProvider) {
+      BinTools binTools, ProjectFile.Provider projectFileProvider,
+      Iterable<BlazeCommand> commands) {
     this.workspaceStatusActionFactory = workspaceStatusActionFactory;
     this.directories = directories;
     this.workingDirectory = directories.getWorkspace();
@@ -232,6 +234,7 @@ public final class BlazeRuntime {
     this.startupOptionsProvider = startupOptionsProvider;
 
     this.eventBusExceptionHandler = eventBusExceptionHandler;
+    overrideCommands(commands);
 
     if (inWorkspace()) {
       writeOutputBaseReadmeFile();
@@ -255,6 +258,31 @@ public final class BlazeRuntime {
   }
 
   /**
+   * Adds the given command under the given name to the map of commands.
+   *
+   * @throws AssertionError if the name is already used by another command.
+   */
+  private void addCommand(BlazeCommand command) {
+    String name = command.getClass().getAnnotation(Command.class).name();
+    if (commandMap.containsKey(name)) {
+      throw new IllegalStateException("Command name or alias " + name + " is already used.");
+    }
+    commandMap.put(name, command);
+  }
+
+  final void overrideCommands(Iterable<BlazeCommand> commands) {
+    commandMap.clear();
+    for (BlazeCommand command : commands) {
+      addCommand(command);
+    }
+    for (BlazeModule module : blazeModules) {
+      for (BlazeCommand command : module.getCommands()) {
+        addCommand(command);
+      }
+    }
+  }
+
+  /**
    * 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.
    */
@@ -267,10 +295,6 @@ public final class BlazeRuntime {
     }
   }
 
-  public String getOutputFileSystem() {
-    return outputFileSystem;
-  }
-
   public CommandEnvironment initCommand() {
     EventBus eventBus = new EventBus(eventBusExceptionHandler);
     skyframeExecutor.setEventBus(eventBus);
@@ -663,7 +687,7 @@ public final class BlazeRuntime {
         ? null
         : outputService.getBatchStatter());
 
-    outputFileSystem = determineOutputFileSystem();
+    env.setOutputFileSystem(determineOutputFileSystem());
 
     // Ensure that the working directory will be under the workspace directory.
     Path workspace = getWorkspace();
@@ -843,10 +867,6 @@ public final class BlazeRuntime {
     }
   }
 
-  void setCommandMap(Map<String, BlazeCommand> commandMap) {
-    this.commandMap = ImmutableMap.copyOf(commandMap);
-  }
-
   public Map<String, BlazeCommand> getCommandMap() {
     return commandMap;
   }
@@ -1117,8 +1137,7 @@ public final class BlazeRuntime {
       return e.getExitCode().getNumericExitCode();
     }
 
-    BlazeCommandDispatcher dispatcher =
-        new BlazeCommandDispatcher(runtime, getBuiltinCommandList());
+    BlazeCommandDispatcher dispatcher = new BlazeCommandDispatcher(runtime);
 
     try {
       LOG.info(getRequestLogString(commandLineOptions.getOtherArgs()));
@@ -1166,8 +1185,7 @@ public final class BlazeRuntime {
     BlazeServerStartupOptions startupOptions = options.getOptions(BlazeServerStartupOptions.class);
 
     final BlazeRuntime runtime = newRuntime(modules, options);
-    final BlazeCommandDispatcher dispatcher =
-        new BlazeCommandDispatcher(runtime, getBuiltinCommandList());
+    final BlazeCommandDispatcher dispatcher = new BlazeCommandDispatcher(runtime);
 
     final ServerCommand blazeCommand;
 
@@ -1344,6 +1362,7 @@ public final class BlazeRuntime {
     for (BlazeModule blazeModule : blazeModules) {
       runtimeBuilder.addBlazeModule(blazeModule);
     }
+    runtimeBuilder.addCommands(getBuiltinCommandList());
 
     BlazeRuntime runtime = runtimeBuilder.build();
     AutoProfiler.setClock(runtime.getClock());
@@ -1440,11 +1459,12 @@ public final class BlazeRuntime {
     private ConfigurationFactory configurationFactory;
     private Clock clock;
     private OptionsProvider startupOptionsProvider;
-    private final List<BlazeModule> blazeModules = Lists.newArrayList();
+    private final List<BlazeModule> blazeModules = new ArrayList<>();
     private SubscriberExceptionHandler eventBusExceptionHandler =
         new RemoteExceptionHandler();
     private BinTools binTools;
     private UUID instanceId;
+    private final List<BlazeCommand> commands = new ArrayList<>();
 
     public BlazeRuntime build() throws AbruptExitException {
       Preconditions.checkNotNull(directories);
@@ -1607,7 +1627,7 @@ public final class BlazeRuntime {
       return new BlazeRuntime(directories, reporter, workspaceStatusActionFactory, skyframeExecutor,
           pkgFactory, ruleClassProvider, configurationFactory,
           clock, startupOptionsProvider, ImmutableList.copyOf(blazeModules),
-          timestampMonitor, eventBusExceptionHandler, binTools, projectFileProvider);
+          timestampMonitor, eventBusExceptionHandler, binTools, projectFileProvider, commands);
     }
 
     public Builder setBinTools(BinTools binTools) {
@@ -1666,5 +1686,15 @@ public final class BlazeRuntime {
       this.eventBusExceptionHandler = eventBusExceptionHandler;
       return this;
     }
+
+    public Builder addCommands(BlazeCommand... commands) {
+      this.commands.addAll(Arrays.asList(commands));
+      return this;
+    }
+
+    public Builder addCommands(Iterable<BlazeCommand> commands) {
+      Iterables.addAll(this.commands, commands);
+      return this;
+    }
   }
 }
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 7641ec9d33..5590b061c8 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
@@ -61,6 +61,8 @@ public final class CommandEnvironment {
   private final BlazeModule.ModuleEnvironment blazeModuleEnvironment;
   private final Map<String, String> clientEnv = new HashMap<>();
 
+  private String outputFileSystem;
+
   private AtomicReference<AbruptExitException> pendingException = new AtomicReference<>();
 
   private class BlazeModuleEnvironment implements BlazeModule.ModuleEnvironment {
@@ -222,4 +224,12 @@ public final class CommandEnvironment {
   public long getCommandStartTime() {
     return runtime.getCommandStartTime();
   }
+
+  void setOutputFileSystem(String outputFileSystem) {
+    this.outputFileSystem = outputFileSystem;
+  }
+
+  public String getOutputFileSystem() {
+    return outputFileSystem;
+  }
 }