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
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 597892a..256373c 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 @@
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 5629c40..9f701b1 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.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 @@
}
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 @@
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 @@
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 @@
}
/**
- * 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 f4886aa..7d2cfae 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.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.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 @@
// 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 @@
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 @@
this.startupOptionsProvider = startupOptionsProvider;
this.eventBusExceptionHandler = eventBusExceptionHandler;
+ overrideCommands(commands);
if (inWorkspace()) {
writeOutputBaseReadmeFile();
@@ -255,6 +258,31 @@
}
/**
+ * 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 String getOutputFileSystem() {
- return outputFileSystem;
- }
-
public CommandEnvironment initCommand() {
EventBus eventBus = new EventBus(eventBusExceptionHandler);
skyframeExecutor.setEventBus(eventBus);
@@ -663,7 +687,7 @@
? 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 @@
}
}
- void setCommandMap(Map<String, BlazeCommand> commandMap) {
- this.commandMap = ImmutableMap.copyOf(commandMap);
- }
-
public Map<String, BlazeCommand> getCommandMap() {
return commandMap;
}
@@ -1117,8 +1137,7 @@
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 @@
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 @@
for (BlazeModule blazeModule : blazeModules) {
runtimeBuilder.addBlazeModule(blazeModule);
}
+ runtimeBuilder.addCommands(getBuiltinCommandList());
BlazeRuntime runtime = runtimeBuilder.build();
AutoProfiler.setClock(runtime.getClock());
@@ -1440,11 +1459,12 @@
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 @@
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 @@
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 7641ec9..5590b06 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 @@
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 long getCommandStartTime() {
return runtime.getCommandStartTime();
}
+
+ void setOutputFileSystem(String outputFileSystem) {
+ this.outputFileSystem = outputFileSystem;
+ }
+
+ public String getOutputFileSystem() {
+ return outputFileSystem;
+ }
}
