Split apart options that should only be set by the client from CommonCommandOptions.
client_env and default_override stand apart from the other common options. They can be very long, and should only be used by the environment setup. They should ideally not be passed by option at all, but in the RunRequest proto message, but for now, isolate them so that they don't clutter the common command options, which are passed all over the place.
RELNOTES: None.
PiperOrigin-RevId: 173592980
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandUtils.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandUtils.java
index cd618d2..9e8bb80 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandUtils.java
@@ -40,18 +40,16 @@
BlazeServerStartupOptions.class,
HostJvmStartupOptions.class);
- /**
- * The set of option-classes that are common to all Blaze commands.
- */
+ /** The set of option-classes that are common to all Blaze commands. */
private static final ImmutableList<Class<? extends OptionsBase>> COMMON_COMMAND_OPTIONS =
ImmutableList.of(
BlazeCommandEventHandler.Options.class,
CommonCommandOptions.class,
+ ClientOptions.class,
// Skylark options aren't applicable to all commands, but making them a common option
// allows users to put them in the common section of the bazelrc. See issue #3538.
SkylarkSemanticsOptions.class);
-
private BlazeCommandUtils() {}
public static ImmutableList<Class<? extends OptionsBase>> getStartupOptions(
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java
index b1ceb77..25afbf2 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java
@@ -167,7 +167,7 @@
// Command-specific options from .blazerc passed in via --default_override
// and --rc_source. A no-op if none are provided.
- CommonCommandOptions rcFileOptions = optionsParser.getOptions(CommonCommandOptions.class);
+ ClientOptions rcFileOptions = optionsParser.getOptions(ClientOptions.class);
List<Pair<String, ListMultimap<String, String>>> optionsMap =
getOptionsMap(
eventHandler,
@@ -404,13 +404,13 @@
static List<Pair<String, ListMultimap<String, String>>> getOptionsMap(
EventHandler eventHandler,
List<String> rcFiles,
- List<CommonCommandOptions.OptionOverride> overrides,
+ List<ClientOptions.OptionOverride> overrides,
Set<String> validCommands) {
List<Pair<String, ListMultimap<String, String>>> result = new ArrayList<>();
String lastRcFile = null;
ListMultimap<String, String> lastMap = null;
- for (CommonCommandOptions.OptionOverride override : overrides) {
+ for (ClientOptions.OptionOverride override : overrides) {
if (override.blazeRc < 0 || override.blazeRc >= rcFiles.size()) {
eventHandler.handle(
Event.warn("inconsistency in generated command line args. Ignoring bogus argument\n"));
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/ClientOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/ClientOptions.java
new file mode 100644
index 0000000..db6f988
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/runtime/ClientOptions.java
@@ -0,0 +1,141 @@
+// Copyright 2014 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.runtime;
+
+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.Map;
+
+/**
+ * Options that the Bazel client passes to server, which are then incorporated into the environment.
+ *
+ * <p>The rc file options are parsed in their own right and appear, if applicable, in the final
+ * value of the parsed options. The environment variables update the stored values in the
+ * CommandEnvironment. These options should never be accessed directly from this class after command
+ * environment initialization.
+ */
+public class ClientOptions extends OptionsBase {
+ /**
+ * A class representing a blazerc option. blazeRc is serial number of the rc file this option came
+ * from, option is the name of the option and value is its value (or null if not specified).
+ */
+ public static class OptionOverride {
+ final int blazeRc;
+ final String command;
+ final String option;
+
+ public OptionOverride(int blazeRc, String command, String option) {
+ this.blazeRc = blazeRc;
+ this.command = command;
+ this.option = option;
+ }
+
+ @Override
+ public String toString() {
+ return String.format("%d:%s=%s", blazeRc, command, option);
+ }
+ }
+
+ /** Converter for --default_override. The format is: --default_override=blazerc:command=option. */
+ public static class OptionOverrideConverter implements Converter<OptionOverride> {
+ static final String ERROR_MESSAGE =
+ "option overrides must be in form rcfile:command=option, where rcfile is a nonzero integer";
+
+ public OptionOverrideConverter() {}
+
+ @Override
+ public OptionOverride convert(String input) throws OptionsParsingException {
+ int colonPos = input.indexOf(':');
+ int assignmentPos = input.indexOf('=');
+
+ if (colonPos < 0) {
+ throw new OptionsParsingException(ERROR_MESSAGE);
+ }
+
+ if (assignmentPos <= colonPos + 1) {
+ throw new OptionsParsingException(ERROR_MESSAGE);
+ }
+
+ int blazeRc;
+ try {
+ blazeRc = Integer.valueOf(input.substring(0, colonPos));
+ } catch (NumberFormatException e) {
+ throw new OptionsParsingException(ERROR_MESSAGE);
+ }
+
+ if (blazeRc < 0) {
+ throw new OptionsParsingException(ERROR_MESSAGE);
+ }
+
+ String command = input.substring(colonPos + 1, assignmentPos);
+ String option = input.substring(assignmentPos + 1);
+
+ return new OptionOverride(blazeRc, command, option);
+ }
+
+ @Override
+ public String getTypeDescription() {
+ return "blazerc option override";
+ }
+ }
+
+ @Option(
+ name = "client_env",
+ defaultValue = "",
+ documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+ metadataTags = {OptionMetadataTag.HIDDEN},
+ effectTags = {OptionEffectTag.CHANGES_INPUTS},
+ converter = Converters.AssignmentConverter.class,
+ allowMultiple = true,
+ help = "A system-generated parameter which specifies the client's environment"
+ )
+ public List<Map.Entry<String, String>> clientEnv;
+
+ /**
+ * These are the actual default overrides. Each value is a tuple of (bazelrc index, command name,
+ * value). The blazerc index is a number used to find the blazerc in --rc_source's values.
+ *
+ * <p>For example: "--default_override=rc:build=--cpu=piii"
+ */
+ @Option(
+ name = "default_override",
+ defaultValue = "",
+ allowMultiple = true,
+ documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+ effectTags = {OptionEffectTag.CHANGES_INPUTS},
+ metadataTags = {OptionMetadataTag.HIDDEN},
+ converter = OptionOverrideConverter.class,
+ help = ""
+ )
+ public List<OptionOverride> optionsOverrides;
+
+ /** This is the filename that the Blaze client parsed. */
+ @Option(
+ name = "rc_source",
+ defaultValue = "",
+ allowMultiple = true,
+ documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+ effectTags = {OptionEffectTag.CHANGES_INPUTS},
+ metadataTags = {OptionMetadataTag.HIDDEN},
+ help = ""
+ )
+ public List<String> rcSource;
+}
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 6e28665..6329ee1 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
@@ -151,10 +151,17 @@
workspace.getSkyframeExecutor().setEventBus(eventBus);
- CommonCommandOptions commandOptions = options.getOptions(CommonCommandOptions.class);
+ 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(commandOptions.clientEnv, warnings);
+ updateClientEnv(clientOptions.clientEnv, warnings);
// actionClientEnv contains the environment where values from actionEnvironment are overridden.
actionClientEnv.putAll(clientEnv);
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 c602dcd..29fa7d0 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
@@ -27,7 +27,6 @@
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;
@@ -35,70 +34,6 @@
* Options common to all commands.
*/
public class CommonCommandOptions extends OptionsBase {
- /**
- * A class representing a blazerc option. blazeRc is serial number of the rc
- * file this option came from, option is the name of the option and value is
- * its value (or null if not specified).
- */
- public static class OptionOverride {
- final int blazeRc;
- final String command;
- final String option;
-
- public OptionOverride(int blazeRc, String command, String option) {
- this.blazeRc = blazeRc;
- this.command = command;
- this.option = option;
- }
-
- @Override
- public String toString() {
- return String.format("%d:%s=%s", blazeRc, command, option);
- }
- }
-
- /** Converter for --default_override. The format is: --default_override=blazerc:command=option. */
- public static class OptionOverrideConverter implements Converter<OptionOverride> {
- static final String ERROR_MESSAGE = "option overrides must be in form "
- + " rcfile:command=option, where rcfile is a nonzero integer";
-
- public OptionOverrideConverter() {}
-
- @Override
- public OptionOverride convert(String input) throws OptionsParsingException {
- int colonPos = input.indexOf(':');
- int assignmentPos = input.indexOf('=');
-
- if (colonPos < 0) {
- throw new OptionsParsingException(ERROR_MESSAGE);
- }
-
- if (assignmentPos <= colonPos + 1) {
- throw new OptionsParsingException(ERROR_MESSAGE);
- }
-
- int blazeRc;
- try {
- blazeRc = Integer.valueOf(input.substring(0, colonPos));
- } catch (NumberFormatException e) {
- throw new OptionsParsingException(ERROR_MESSAGE);
- }
-
- if (blazeRc < 0) {
- throw new OptionsParsingException(ERROR_MESSAGE);
- }
-
- String command = input.substring(colonPos + 1, assignmentPos);
- String option = input.substring(assignmentPos + 1);
-
- return new OptionOverride(blazeRc, command, option);
- }
-
- @Override
- public String getTypeDescription() {
- return "blazerc option override";
- }
- }
/** Converter for UUID. Accepts values as specified by {@link UUID#fromString(String)}. */
public static class UUIDConverter implements Converter<UUID> {
@@ -196,18 +131,6 @@
public Level verbosity;
@Option(
- name = "client_env",
- defaultValue = "",
- documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
- metadataTags = {OptionMetadataTag.HIDDEN},
- effectTags = {OptionEffectTag.CHANGES_INPUTS},
- converter = Converters.AssignmentConverter.class,
- allowMultiple = true,
- help = "A system-generated parameter which specifies the client's environment"
- )
- public List<Map.Entry<String, String>> clientEnv;
-
- @Option(
name = "client_cwd",
defaultValue = "",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
@@ -228,36 +151,6 @@
)
public boolean announceRcOptions;
- /**
- * These are the actual default overrides. Each value is a tuple of (bazelrc index, command name,
- * value). The blazerc index is a number used to find the blazerc in --rc_source's values.
- *
- * <p>For example: "--default_override=rc:build=--cpu=piii"
- */
- @Option(
- name = "default_override",
- defaultValue = "",
- allowMultiple = true,
- documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
- effectTags = {OptionEffectTag.CHANGES_INPUTS},
- metadataTags = {OptionMetadataTag.HIDDEN},
- converter = OptionOverrideConverter.class,
- help = ""
- )
- public List<OptionOverride> optionsOverrides;
-
- /** This is the filename that the Blaze client parsed. */
- @Option(
- name = "rc_source",
- defaultValue = "",
- allowMultiple = true,
- documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
- effectTags = {OptionEffectTag.CHANGES_INPUTS},
- metadataTags = {OptionMetadataTag.HIDDEN},
- help = ""
- )
- public List<String> rcSource;
-
@Option(
name = "always_profile_slow_operations",
defaultValue = "true",
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/AbstractCommandTest.java b/src/test/java/com/google/devtools/build/lib/runtime/AbstractCommandTest.java
index 7ca6590..6b698d7 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/AbstractCommandTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/AbstractCommandTest.java
@@ -138,6 +138,7 @@
Collections.addAll(result, optionClasses);
result.add(BlazeCommandEventHandler.Options.class);
result.add(CommonCommandOptions.class);
+ result.add(ClientOptions.class);
result.add(SkylarkSemanticsOptions.class);
return result;
}
