Automated rollback of commit c6b6dbadd0a93936c51154b25abc5fbba8f2d1af.

Not a totally clean rollback because of the intervening https://github.com/bazelbuild/bazel/commit/b5158a9a677b149e04e844c40a01e9a9dde40783.

*** Reason for rollback ***

PiperOrigin-RevId: 173957187

4 files changed, 40 insertions, 154 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 9363daaef5..06e895b9e7 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,8 +279,7 @@ public class BlazeCommandDispatcher {
         new CommandLineEvent.CanonicalCommandLineEvent(runtime, commandName, options);
 
     // The initCommand call also records the start time for the timestamp granularity monitor.
-    List<String> commandEnvWarnings = new ArrayList<>();
-    CommandEnvironment env = workspace.initCommand(commandAnnotation, options, commandEnvWarnings);
+    CommandEnvironment env = workspace.initCommand(commandAnnotation, options);
     // Record the command's starting time for use by the commands themselves.
     env.recordCommandStartTime(firstContactTime);
 
@@ -422,9 +421,6 @@ 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 e682d4ef58..2a2d23edda 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,7 +36,6 @@ 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;
@@ -187,24 +186,13 @@ 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.
-   *
-   * @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.
+   * <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.
    */
-  public CommandEnvironment initCommand(
-      Command command, OptionsProvider options, List<String> warnings) {
-    CommandEnvironment env =
-        new CommandEnvironment(
-            runtime,
-            this,
-            new EventBus(eventBusExceptionHandler),
-            Thread.currentThread(),
-            command,
-            options,
-            warnings);
+  public CommandEnvironment initCommand(Command command, OptionsProvider options) {
+    CommandEnvironment env = new CommandEnvironment(
+        runtime, this, new EventBus(eventBusExceptionHandler), Thread.currentThread(), command,
+        options);
     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 a36f3afaba..ab49705d6d 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,20 +117,15 @@ 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,
-      List<String> warnings) {
+      BlazeRuntime runtime, BlazeWorkspace workspace, EventBus eventBus, Thread commandThread,
+      Command command, OptionsProvider options) {
     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;
@@ -151,17 +146,11 @@ public final class CommandEnvironment {
 
     workspace.getSkyframeExecutor().setEventBus(eventBus);
 
-    CommonCommandOptions commandOptions =
-        Preconditions.checkNotNull(
-            options.getOptions(CommonCommandOptions.class),
-            "CommandEnvironment needs its options provider to have CommonCommandOptions loaded.");
     ClientOptions clientOptions =
         Preconditions.checkNotNull(
             options.getOptions(ClientOptions.class),
             "CommandEnvironment needs its options provider to have ClientOptions loaded.");
-    this.commandId = commandOptions.invocationId;
-    this.buildRequestId = commandOptions.buildRequestId;
-    updateClientEnv(clientOptions.clientEnv, warnings);
+    updateClientEnv(clientOptions.clientEnv);
 
     // actionClientEnv contains the environment where values from actionEnvironment are overridden.
     actionClientEnv.putAll(clientEnv);
@@ -261,43 +250,41 @@ public final class CommandEnvironment {
     return Collections.unmodifiableMap(result);
   }
 
-  private void updateClientEnv(
-      List<Map.Entry<String, String>> clientEnvList, List<String> warnings) {
+  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) {
     Preconditions.checkState(clientEnv.isEmpty());
 
     Collection<Map.Entry<String, String>> env = clientEnvList;
     for (Map.Entry<String, String> entry : env) {
       clientEnv.put(entry.getKey(), entry.getValue());
     }
-
-    // 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 (commandId == null) {
+      commandId = getUuidFromEnvOrGenerate("BAZEL_INTERNAL_INVOCATION_ID");
     }
     if (buildRequestId == null) {
-      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();
-      }
+      buildRequestId = getFromEnvOrGenerate("BAZEL_INTERNAL_BUILD_REQUEST_ID");
     }
     setCommandIdInCrashData();
   }
@@ -338,8 +325,7 @@ 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 prefixed UUID, but some existing clients still use arbitrary
-   * strings, so we accept these when passed by environment variable for compatibility.
+   * TODO(olaola): this should be a UUID, but some existing clients still use arbitrary strings.
    */
   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 29fa7d09c9..1f4e0d738f 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,21 +13,16 @@
 // 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;
-import com.google.devtools.common.options.Converter;
 import com.google.devtools.common.options.Converters;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionDocumentationCategory;
 import com.google.devtools.common.options.OptionEffectTag;
 import com.google.devtools.common.options.OptionMetadataTag;
 import com.google.devtools.common.options.OptionsBase;
-import com.google.devtools.common.options.OptionsParsingException;
 import java.util.List;
-import java.util.UUID;
 import java.util.logging.Level;
 
 /**
@@ -35,58 +30,6 @@ import java.util.logging.Level;
  */
 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",
@@ -269,33 +212,6 @@ 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",