Accept build IDs by flag.

We accepted these by environment variable largely because setting it via invocation policy would require changing invocation policy for each command, which had caused the Bazel server to restart, loosing incremental state. This is fixed: changing invocation policy no longer causes Bazel to restart its servers, so accept these as normal options.

We will soon no longer accept these flags by environment variable, but will accept both for a transition period, so that nobody relying on these values is broken by a single release. To inform users of this environment variable, anyone setting the environment variable without the flag will receive a warning but the value will be kept. The following release will no longer accept an environment variable.

Note on format: invocation_id we accept only clean UUIDs, but for build_request_id, to help differentiate otherwise undifferentiable id types, we accept arbitrary prefixes before the UUID. The user is responsible for picking prefixes that are sane.

RELNOTES: None.
PiperOrigin-RevId: 173432904

4 files changed, 149 insertions, 40 deletions

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 06e895b9e7..9363daaef5 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
@@ -279,7 +279,8 @@ public class BlazeCommandDispatcher {
         new CommandLineEvent.CanonicalCommandLineEvent(runtime, commandName, options);
 
     // The initCommand call also records the start time for the timestamp granularity monitor.
-    CommandEnvironment env = workspace.initCommand(commandAnnotation, options);
+    List<String> commandEnvWarnings = new ArrayList<>();
+    CommandEnvironment env = workspace.initCommand(commandAnnotation, options, commandEnvWarnings);
     // Record the command's starting time for use by the commands themselves.
     env.recordCommandStartTime(firstContactTime);
 
@@ -421,6 +422,9 @@ public class BlazeCommandDispatcher {
         module.checkEnvironment(env);
       }
 
+      for (String warning : commandEnvWarnings) {
+        reporter.handle(Event.warn(warning));
+      }
       if (commonOptions.announceRcOptions) {
         if (startupOptionsTaggedWithBazelRc.isPresent()) {
           String lastBlazerc = "";
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java
index 2a2d23edda..e682d4ef58 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java
@@ -36,6 +36,7 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.common.options.OptionsProvider;
 import java.io.IOException;
+import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.Nullable;
@@ -186,13 +187,24 @@ public final class BlazeWorkspace {
   /**
    * Initializes a CommandEnvironment to execute a command in this workspace.
    *
-   * <p>This method should be called from the "main" thread on which the command will execute;
-   * that thread will receive interruptions if a module requests an early exit.
+   * <p>This method should be called from the "main" thread on which the command will execute; that
+   * thread will receive interruptions if a module requests an early exit.
+   *
+   * @param warnings a list of warnings to which the CommandEnvironment can add any warning
+   * generated during initialization. This is needed because Blaze's output handling is not yet
+   * fully configured at this point.
    */
-  public CommandEnvironment initCommand(Command command, OptionsProvider options) {
-    CommandEnvironment env = new CommandEnvironment(
-        runtime, this, new EventBus(eventBusExceptionHandler), Thread.currentThread(), command,
-        options);
+  public CommandEnvironment initCommand(
+      Command command, OptionsProvider options, List<String> warnings) {
+    CommandEnvironment env =
+        new CommandEnvironment(
+            runtime,
+            this,
+            new EventBus(eventBusExceptionHandler),
+            Thread.currentThread(),
+            command,
+            options,
+            warnings);
     skyframeExecutor.setClientEnv(env.getClientEnv());
     return env;
   }
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 1976a55823..6e28665024 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
@@ -117,15 +117,20 @@ public final class CommandEnvironment {
    * Creates a new command environment which can be used for executing commands for the given
    * runtime in the given workspace, which will publish events on the given eventBus. The
    * commandThread passed is interrupted when a module requests an early exit.
+   *
+   * @param warnings will be filled with any warnings from command environment initialization.
    */
   CommandEnvironment(
-      BlazeRuntime runtime, BlazeWorkspace workspace, EventBus eventBus, Thread commandThread,
-      Command command, OptionsProvider options) {
+      BlazeRuntime runtime,
+      BlazeWorkspace workspace,
+      EventBus eventBus,
+      Thread commandThread,
+      Command command,
+      OptionsProvider options,
+      List<String> warnings) {
     this.runtime = runtime;
     this.workspace = workspace;
     this.directories = workspace.getDirectories();
-    this.commandId = null; // Will be set once we get the client environment
-    this.buildRequestId = null;  // Will be set once we get the client environment
     this.reporter = new Reporter(eventBus);
     this.eventBus = eventBus;
     this.commandThread = commandThread;
@@ -146,7 +151,10 @@ public final class CommandEnvironment {
 
     workspace.getSkyframeExecutor().setEventBus(eventBus);
 
-    updateClientEnv(options.getOptions(CommonCommandOptions.class).clientEnv);
+    CommonCommandOptions commandOptions = options.getOptions(CommonCommandOptions.class);
+    this.commandId = commandOptions.invocationId;
+    this.buildRequestId = commandOptions.buildRequestId;
+    updateClientEnv(commandOptions.clientEnv, warnings);
 
     // actionClientEnv contains the environment where values from actionEnvironment are overridden.
     actionClientEnv.putAll(clientEnv);
@@ -246,41 +254,43 @@ public final class CommandEnvironment {
     return Collections.unmodifiableMap(result);
   }
 
-  private UUID getUuidFromEnvOrGenerate(String varName) {
-    // Try to set the clientId from the client environment.
-    String uuidString = clientEnv.getOrDefault(varName, "");
-    if (!uuidString.isEmpty()) {
-      try {
-        return UUID.fromString(uuidString);
-      } catch (IllegalArgumentException e) {
-        // String was malformed, so we will resort to generating a random UUID
-      }
-    }
-    // We have been provided with the client environment, but it didn't contain
-    // the variable; hence generate our own id.
-    return UUID.randomUUID();
-  }
-
-  private String getFromEnvOrGenerate(String varName) {
-    String id = clientEnv.getOrDefault(varName, "");
-    if (id.isEmpty()) {
-      id = UUID.randomUUID().toString();
-    }
-    return id;
-  }
-
-  private void updateClientEnv(List<Map.Entry<String, String>> clientEnvList) {
+  private void updateClientEnv(
+      List<Map.Entry<String, String>> clientEnvList, List<String> warnings) {
     Preconditions.checkState(clientEnv.isEmpty());
 
     Collection<Map.Entry<String, String>> env = clientEnvList;
     for (Map.Entry<String, String> entry : env) {
       clientEnv.put(entry.getKey(), entry.getValue());
     }
-    if (commandId == null) {
-      commandId = getUuidFromEnvOrGenerate("BAZEL_INTERNAL_INVOCATION_ID");
+
+    // TODO(b/67895628): Stop reading ids from the environment after the compatibility window has
+    // passed.
+    if (commandId == null) { // Try to set the clientId from the client environment.
+      String uuidString = clientEnv.getOrDefault("BAZEL_INTERNAL_INVOCATION_ID", "");
+      if (!uuidString.isEmpty()) {
+        try {
+          commandId = UUID.fromString(uuidString);
+          warnings.add(
+              "BAZEL_INTERNAL_INVOCATION_ID is set. This will soon be deprecated in favor of "
+                  + "--invocation_id. Please switch to using the flag.");
+        } catch (IllegalArgumentException e) {
+          // String was malformed, so we will resort to generating a random UUID
+          commandId = UUID.randomUUID();
+        }
+      } else {
+        commandId = UUID.randomUUID();
+      }
     }
     if (buildRequestId == null) {
-      buildRequestId = getFromEnvOrGenerate("BAZEL_INTERNAL_BUILD_REQUEST_ID");
+      String uuidString = clientEnv.getOrDefault("BAZEL_INTERNAL_BUILD_REQUEST_ID", "");
+      if (!uuidString.isEmpty()) {
+        buildRequestId = uuidString;
+        warnings.add(
+            "BAZEL_INTERNAL_BUILD_REQUEST_ID is set. This will soon be deprecated in favor of "
+                + "--build_request_id. Please switch to using the flag.");
+      } else {
+        buildRequestId = UUID.randomUUID().toString();
+      }
     }
     setCommandIdInCrashData();
   }
@@ -321,7 +331,8 @@ public final class CommandEnvironment {
 
   /**
    * Returns the ID that Blaze uses to identify everything logged from the current build request.
-   * TODO(olaola): this should be a UUID, but some existing clients still use arbitrary strings.
+   * TODO(olaola): this should be a prefixed UUID, but some existing clients still use arbitrary
+   * strings, so we accept these when passed by environment variable for compatibility.
    */
   public String getBuildRequestId() {
     return Preconditions.checkNotNull(buildRequestId);
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java
index 440271ef86..c602dcd779 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java
@@ -13,6 +13,8 @@
 // limitations under the License.
 package com.google.devtools.build.lib.runtime;
 
+import static com.google.common.base.Strings.isNullOrEmpty;
+
 import com.google.devtools.build.lib.runtime.CommandLineEvent.ToolCommandLineEvent;
 import com.google.devtools.build.lib.util.OptionsUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -26,6 +28,7 @@ import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsParsingException;
 import java.util.List;
 import java.util.Map;
+import java.util.UUID;
 import java.util.logging.Level;
 
 /**
@@ -97,6 +100,58 @@ public class CommonCommandOptions extends OptionsBase {
     }
   }
 
+  /** Converter for UUID. Accepts values as specified by {@link UUID#fromString(String)}. */
+  public static class UUIDConverter implements Converter<UUID> {
+
+    @Override
+    public UUID convert(String input) throws OptionsParsingException {
+      if (isNullOrEmpty(input)) {
+        return null;
+      }
+      try {
+        return UUID.fromString(input);
+      } catch (IllegalArgumentException e) {
+        throw new OptionsParsingException(
+            String.format("Value '%s' is not a value UUID.", input), e);
+      }
+    }
+
+    @Override
+    public String getTypeDescription() {
+      return "a UUID";
+    }
+  }
+
+  /**
+   * Converter for options (--build_request_id) that accept prefixed UUIDs. Since we do not care
+   * about the structure of this value after validation, we store it as a string.
+   */
+  public static class PrefixedUUIDConverter implements Converter<String> {
+
+    @Override
+    public String convert(String input) throws OptionsParsingException {
+      if (isNullOrEmpty(input)) {
+        return null;
+      }
+      // UUIDs that are accepted by UUID#fromString have 36 characters. Interpret the last 36
+      // characters as an UUID and the rest as a prefix. We do not check anything about the contents
+      // of the prefix.
+      try {
+        int uuidStartIndex = input.length() - 36;
+        UUID.fromString(input.substring(uuidStartIndex));
+      } catch (IllegalArgumentException | IndexOutOfBoundsException e) {
+        throw new OptionsParsingException(
+            String.format("Value '%s' does end in a valid UUID.", input), e);
+      }
+      return input;
+    }
+
+    @Override
+    public String getTypeDescription() {
+      return "An optionally prefixed UUID. The last 36 characters will be verified as a UUID.";
+    }
+  }
+
   // To create a new incompatible change, see the javadoc for AllIncompatibleChangesExpansion.
   @Option(
     name = "all_incompatible_changes",
@@ -321,6 +376,33 @@ public class CommonCommandOptions extends OptionsBase {
   )
   public String toolTag;
 
+  // Command ID and build request ID can be set either by flag or environment variable. In most
+  // cases, the internally generated ids should be sufficient, but we allow these to be set
+  // externally if required. Option wins over environment variable, if both are set.
+  // TODO(b/67895628) Stop reading ids from the environment after the compatibility window has
+  // passed.
+  @Option(
+    name = "invocation_id",
+    defaultValue = "",
+    converter = UUIDConverter.class,
+    documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+    effectTags = {OptionEffectTag.BAZEL_MONITORING, OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION},
+    metadataTags = {OptionMetadataTag.HIDDEN},
+    help = "Unique identifier for the command being run."
+  )
+  public UUID invocationId;
+
+  @Option(
+    name = "build_request_id",
+    defaultValue = "",
+    converter = PrefixedUUIDConverter.class,
+    documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+    effectTags = {OptionEffectTag.BAZEL_MONITORING, OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION},
+    metadataTags = {OptionMetadataTag.HIDDEN},
+    help = "Unique identifier for the build being run."
+  )
+  public String buildRequestId;
+
   @Option(
     name = "restart_reason",
     defaultValue = "no_restart",