Move the commandId and getCommandStartTime() to CommandEnvironment.

They are usually on the same line, so doing this in separate changes can cause
merge conflicts.

--
MOS_MIGRATED_REVID=103362797

12 files changed, 62 insertions, 60 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/FetchCommand.java b/src/main/java/com/google/devtools/build/lib/bazel/commands/FetchCommand.java
index 3bc0cadbf2..83abc549d4 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/commands/FetchCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/commands/FetchCommand.java
@@ -70,7 +70,7 @@ public final class FetchCommand implements BlazeCommand {
     }
 
     try {
-      runtime.setupPackageCache(
+      env.setupPackageCache(
           options.getOptions(PackageCacheOptions.class),
           runtime.getDefaultsPackageContent());
     } catch (InterruptedException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/dash/DashModule.java b/src/main/java/com/google/devtools/build/lib/bazel/dash/DashModule.java
index d8408f560c..bd27672f87 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/dash/DashModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/dash/DashModule.java
@@ -28,12 +28,10 @@ import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.pkgcache.TargetParsingCompleteEvent;
 import com.google.devtools.build.lib.rules.test.TestResult;
 import com.google.devtools.build.lib.runtime.BlazeModule;
-import com.google.devtools.build.lib.runtime.BlazeRuntime;
 import com.google.devtools.build.lib.runtime.Command;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.runtime.CommandStartEvent;
 import com.google.devtools.build.lib.runtime.GotOptionsEvent;
-import com.google.devtools.build.lib.util.io.OutErr;
 import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsParser.UnparsedOptionValueDescription;
 import com.google.devtools.common.options.OptionsProvider;
@@ -96,7 +94,7 @@ public class DashModule extends BlazeModule {
     DashOptions options = optionsProvider.getOptions(DashOptions.class);
     sender = (options == null || !options.useDash)
         ? new NoOpSender()
-        : new Sender(options.url, env.getRuntime(), env.getReporter(), executorService);
+        : new Sender(options.url, env, executorService);
     if (optionsBuildData != null) {
       sender.send("options", optionsBuildData);
     }
@@ -229,16 +227,15 @@ public class DashModule extends BlazeModule {
   private static class Sender implements Sendable {
     private final String url;
     private final String buildId;
-    private final OutErr outErr;
+    private final Reporter reporter;
     private final ExecutorService executorService;
 
-    public Sender(String url, BlazeRuntime runtime, Reporter reporter,
-        ExecutorService executorService) {
+    public Sender(String url, CommandEnvironment env, ExecutorService executorService) {
       this.url = url;
-      this.buildId = runtime.getCommandId().toString();
-      this.outErr = reporter.getOutErr();
+      this.buildId = env.getCommandId().toString();
+      this.reporter = env.getReporter();
       this.executorService = executorService;
-      reporter
+      env.getReporter()
           .handle(Event.info("Results are being streamed to " + url + "/result/" + buildId));
     }
 
@@ -252,14 +249,18 @@ public class DashModule extends BlazeModule {
           httppost.setHeader(HttpHeaders.CONTENT_TYPE, "application/x-protobuf");
           httppost.setEntity(new ByteArrayEntity(message.toByteArray()));
 
+          // TODO(ulfjack): It's not safe to print directly to out err. The output can be dropped
+          // if the command already returned.
           try {
             httpClient.execute(httppost);
           } catch (IOException | IllegalStateException e) {
             // IllegalStateException is thrown if the URL was invalid (e.g., someone passed
             // --dash_url=localhost:8080 instead of --dash_url=http://localhost:8080).
-            outErr.printErrLn("Error sending results to " + url + ": " + e.getMessage());
+            reporter.getOutErr().printErrLn(
+                "Error sending results to " + url + ": " + e.getMessage());
           } catch (Exception e) {
-            outErr.printErrLn("Unknown error sending results to " + url + ": " + e.getMessage());
+            reporter.getOutErr().printErrLn(
+                "Unknown error sending results to " + url + ": " + e.getMessage());
           }
         }
       });
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 8881cb579a..597892af3b 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
@@ -145,7 +145,7 @@ public final class BuildTool {
     validateOptions(request);
     BuildOptions buildOptions = runtime.createBuildOptions(request);
     // Sync the package manager before sending the BuildStartingEvent in runLoadingPhase()
-    runtime.setupPackageCache(request.getPackageCacheOptions(),
+    env.setupPackageCache(request.getPackageCacheOptions(),
         DefaultsPackage.getDefaultsPackageContent(buildOptions));
 
     ExecutionTool executionTool = null;
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 d4cf1611fb..5629c405cc 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
@@ -50,7 +50,6 @@ import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.UUID;
 import java.util.logging.Level;
 
 /**
@@ -292,9 +291,6 @@ public class BlazeCommandDispatcher {
           Level.WARNING, "Unable to delete or open command.log", ioException);
     }
 
-    // Create the UUID for this command.
-    runtime.setCommandId(UUID.randomUUID());
-
     ExitCode result = checkCwdInWorkspace(commandAnnotation, commandName, outErr);
     if (result != ExitCode.SUCCESS) {
       return result.getNumericExitCode();
@@ -334,7 +330,7 @@ public class BlazeCommandDispatcher {
 
     // Do this before an actual crash so we don't have to worry about
     // allocating memory post-crash.
-    String[] crashData = runtime.getCrashData();
+    String[] crashData = runtime.getCrashData(env);
     int numericExitCode = ExitCode.BLAZE_INTERNAL_ERROR.getNumericExitCode();
     PrintStream savedOut = System.out;
     PrintStream savedErr = System.err;
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 a727decee6..f4886aace7 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
@@ -183,8 +183,6 @@ public final class BlazeRuntime {
 
   private final Iterable<BlazeModule> blazeModules;
 
-  private UUID commandId;  // Unique identifier for the command being run
-
   private final AtomicInteger storedExitCode = new AtomicInteger();
 
   // We pass this through here to make it available to the MasterLogWriter.
@@ -276,7 +274,8 @@ public final class BlazeRuntime {
   public CommandEnvironment initCommand() {
     EventBus eventBus = new EventBus(eventBusExceptionHandler);
     skyframeExecutor.setEventBus(eventBus);
-    return new CommandEnvironment(this, reporter, eventBus);
+    UUID commandId = UUID.randomUUID();
+    return new CommandEnvironment(this, commandId, reporter, eventBus);
   }
 
   private void clearEventBus() {
@@ -382,6 +381,7 @@ public final class BlazeRuntime {
   public Range<Long> getLastExecutionTimeRange() {
     return lastExecutionStartFinish;
   }
+
   public void recordCommandStartTime(long commandStartTime) {
     this.commandStartTime = commandStartTime;
   }
@@ -687,7 +687,7 @@ public final class BlazeRuntime {
     // Conditionally enable profiling
     // We need to compensate for launchTimeNanos (measurements taken outside of the jvm).
     long startupTimeNanos = options.startupTime * 1000000L;
-    if (initProfiler(env, options, this.getCommandId(), execStartTimeNanos - startupTimeNanos)) {
+    if (initProfiler(env, options, env.getCommandId(), execStartTimeNanos - startupTimeNanos)) {
       Profiler profiler = Profiler.instance();
 
       // Instead of logEvent() we're calling the low level function to pass the timings we took in
@@ -738,7 +738,8 @@ public final class BlazeRuntime {
     }
 
     env.getEventBus().post(
-        new CommandStartEvent(command.name(), commandId, env.getClientEnv(), workingDirectory));
+        new CommandStartEvent(command.name(), env.getCommandId(), env.getClientEnv(),
+            workingDirectory));
     // Initialize exit code to dummy value for afterCommand.
     storedExitCode.set(ExitCode.RESERVED.getNumericExitCode());
   }
@@ -819,21 +820,14 @@ public final class BlazeRuntime {
    * An array of String values useful if Blaze crashes.
    * For now, just returns the size of the action cache and the build id.
    */
-  public String[] getCrashData() {
+  public String[] getCrashData(CommandEnvironment env) {
     return new String[]{
         getFileSizeString(CompactPersistentActionCache.cacheFile(getCacheDirectory()),
                           "action cache"),
-        commandIdString(),
+        env.getCommandId() + " (build id)",
     };
   }
 
-  private String commandIdString() {
-    UUID uuid = getCommandId();
-    return (uuid == null)
-        ? "no build id"
-        : uuid + " (build id)";
-  }
-
   /**
    * @return the OutputService in use, or null if none.
    */
@@ -849,14 +843,6 @@ public final class BlazeRuntime {
     }
   }
 
-  /**
-   * Returns the UUID that Blaze uses to identify everything
-   * logged from the current build command.
-   */
-  public UUID getCommandId() {
-    return commandId;
-  }
-
   void setCommandMap(Map<String, BlazeCommand> commandMap) {
     this.commandMap = ImmutableMap.copyOf(commandMap);
   }
@@ -866,27 +852,19 @@ public final class BlazeRuntime {
   }
 
   /**
-   * Sets the UUID that Blaze uses to identify everything
-   * logged from the current build command.
-   */
-  @VisibleForTesting
-  public void setCommandId(UUID runId) {
-    commandId = runId;
-  }
-
-  /**
    * Initializes the package cache using the given options, and syncs the package cache. Also
    * injects a defaults package using the options for the {@link BuildConfiguration}.
    *
    * @see DefaultsPackage
    */
   public void setupPackageCache(PackageCacheOptions packageCacheOptions,
-      String defaultsPackageContents) throws InterruptedException, AbruptExitException {
+      String defaultsPackageContents, UUID commandId)
+          throws InterruptedException, AbruptExitException {
     if (!skyframeExecutor.hasIncrementalState()) {
       clearSkyframeRelevantCaches();
     }
     skyframeExecutor.sync(packageCacheOptions, getOutputBase(), getWorkingDirectory(),
-        defaultsPackageContents, getCommandId());
+        defaultsPackageContents, commandId);
   }
 
   public void shutdown() {
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 381225c0a0..7641ec9d33 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
@@ -21,13 +21,16 @@ import com.google.common.eventbus.EventBus;
 import com.google.devtools.build.lib.actions.cache.ActionCache;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.BuildView;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
 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.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.LoadedPackageProvider;
+import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
 import com.google.devtools.build.lib.pkgcache.PackageManager;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
 import com.google.devtools.build.lib.syntax.Label;
@@ -51,6 +54,8 @@ import java.util.concurrent.atomic.AtomicReference;
  */
 public final class CommandEnvironment {
   private final BlazeRuntime runtime;
+
+  private final UUID commandId;  // Unique identifier for the command being run
   private final Reporter reporter;
   private final EventBus eventBus;
   private final BlazeModule.ModuleEnvironment blazeModuleEnvironment;
@@ -74,8 +79,10 @@ public final class CommandEnvironment {
     }
   }
 
-  public CommandEnvironment(BlazeRuntime runtime, Reporter reporter, EventBus eventBus) {
+  public CommandEnvironment(BlazeRuntime runtime, UUID commandId, Reporter reporter,
+      EventBus eventBus) {
     this.runtime = runtime;
+    this.commandId = commandId;
     this.reporter = reporter;
     this.eventBus = eventBus;
     this.blazeModuleEnvironment = new BlazeModuleEnvironment();
@@ -131,8 +138,13 @@ public final class CommandEnvironment {
     return runtime.getView();
   }
 
+  /**
+   * Returns the UUID that Blaze uses to identify everything logged from the current build command.
+   * It's also used to invalidate Skyframe nodes that are specific to a certain invocation, such as
+   * the build info.
+   */
   public UUID getCommandId() {
-    return runtime.getCommandId();
+    return commandId;
   }
 
   public SkyframeExecutor getSkyframeExecutor() {
@@ -195,4 +207,19 @@ public final class CommandEnvironment {
       throw exception;
     }
   }
+
+  /**
+   * Initializes the package cache using the given options, and syncs the package cache. Also
+   * injects a defaults package using the options for the {@link BuildConfiguration}.
+   *
+   * @see DefaultsPackage
+   */
+  public void setupPackageCache(PackageCacheOptions packageCacheOptions,
+      String defaultsPackageContents) throws InterruptedException, AbruptExitException {
+    runtime.setupPackageCache(packageCacheOptions, defaultsPackageContents, commandId);
+  }
+
+  public long getCommandStartTime() {
+    return runtime.getCommandStartTime();
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/BuildCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/BuildCommand.java
index 0082d7149d..2a89754a9a 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/BuildCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/BuildCommand.java
@@ -67,7 +67,7 @@ public final class BuildCommand implements BlazeCommand {
         getClass().getAnnotation(Command.class).name(), options,
         runtime.getStartupOptionsProvider(),
         targets,
-        env.getReporter().getOutErr(), runtime.getCommandId(), runtime.getCommandStartTime());
+        env.getReporter().getOutErr(), env.getCommandId(), env.getCommandStartTime());
     return new BuildTool(env).processRequest(request, null).getExitCondition();
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java
index 8347328e81..d90d27ed9f 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java
@@ -199,7 +199,7 @@ public class InfoCommand implements BlazeCommand {
           // In order to be able to answer configuration-specific queries, we need to setup the
           // package path. Since info inherits all the build options, all the necessary information
           // is available here.
-          runtime.setupPackageCache(
+          env.setupPackageCache(
               optionsProvider.getOptions(PackageCacheOptions.class),
               runtime.getDefaultsPackageContent(optionsProvider));
           // TODO(bazel-team): What if there are multiple configurations? [multi-config]
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/MobileInstallCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/MobileInstallCommand.java
index 9701289c57..0c8519379f 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/MobileInstallCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/MobileInstallCommand.java
@@ -80,7 +80,7 @@ public class MobileInstallCommand implements BlazeCommand {
     BuildRequest request = BuildRequest.create(
         this.getClass().getAnnotation(Command.class).name(), options,
         runtime.getStartupOptionsProvider(), targets,
-        env.getReporter().getOutErr(), runtime.getCommandId(), runtime.getCommandStartTime());
+        env.getReporter().getOutErr(), env.getCommandId(), env.getCommandStartTime());
     return new BuildTool(env).processRequest(request, null).getExitCondition();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java
index dd6ac2549e..84ba83c512 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java
@@ -76,7 +76,7 @@ public final class QueryCommand implements BlazeCommand {
     QueryOptions queryOptions = options.getOptions(QueryOptions.class);
 
     try {
-      runtime.setupPackageCache(
+      env.setupPackageCache(
           options.getOptions(PackageCacheOptions.class),
           runtime.getDefaultsPackageContent());
     } catch (InterruptedException e) {
@@ -143,7 +143,7 @@ public final class QueryCommand implements BlazeCommand {
     try {
       QueryOutputUtils.output(queryOptions, result, formatter, output,
           queryOptions.aspectDeps.createResolver(
-              runtime.getPackageManager(), env.getReporter()));
+              env.getPackageManager(), env.getReporter()));
     } catch (ClosedByInterruptException | InterruptedException e) {
       env.getReporter().handle(Event.error("query interrupted"));
       return ExitCode.INTERRUPTED;
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
index 65a63ec1d8..2a7276d60b 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
@@ -147,7 +147,7 @@ public class RunCommand implements BlazeCommand  {
     BuildRequest request = BuildRequest.create(
         this.getClass().getAnnotation(Command.class).name(), options,
         runtime.getStartupOptionsProvider(), targets, outErr,
-        runtime.getCommandId(), runtime.getCommandStartTime());
+        env.getCommandId(), env.getCommandStartTime());
 
     currentRunUnder = runUnder;
     BuildResult result;
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
index 9d9ca9de6b..33a636e763 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
@@ -113,7 +113,7 @@ public class TestCommand implements BlazeCommand {
     BuildRequest request = BuildRequest.create(
         getClass().getAnnotation(Command.class).name(), options,
         runtime.getStartupOptionsProvider(), targets,
-        env.getReporter().getOutErr(), runtime.getCommandId(), runtime.getCommandStartTime());
+        env.getReporter().getOutErr(), env.getCommandId(), env.getCommandStartTime());
     request.setRunTests();
 
     BuildResult buildResult = new BuildTool(env).processRequest(request, null);