Allow *allowMultiple* options to be *set* by Starlark transitions.
Specifically helpful for --define.
Work towards #5574
RELNOTES: None.
PiperOrigin-RevId: 242736849
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/FunctionTransitionUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/FunctionTransitionUtil.java
index 07f2649..da86316 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/FunctionTransitionUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/FunctionTransitionUtil.java
@@ -33,12 +33,16 @@
import com.google.devtools.build.lib.syntax.Runtime;
import com.google.devtools.build.lib.syntax.Runtime.NoneType;
import com.google.devtools.build.lib.syntax.SkylarkDict;
+import com.google.devtools.build.lib.syntax.SkylarkList;
+import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
import com.google.devtools.common.options.OptionDefinition;
import com.google.devtools.common.options.OptionsParser;
import com.google.devtools.common.options.OptionsParsingException;
import java.lang.reflect.Field;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
+import java.util.ArrayList;
+import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
@@ -175,7 +179,33 @@
Field field = optionInfo.getDefinition().getField();
FragmentOptions options = buildOptions.get(optionInfo.getOptionClass());
Object optionValue = field.get(options);
-
+ if (optionInfo.getDefinition().allowsMultiple()) {
+ List<?> optionValueList = (List<?>) optionValue;
+ if (optionValueList.isEmpty()) {
+ optionValue = MutableList.empty();
+ } else {
+ if (optionValueList.get(0) instanceof Map.Entry) {
+ SkylarkDict<String, String> valueDict = SkylarkDict.withMutability(mutability);
+ for (Map.Entry singleValue : (List<Map.Entry>) optionValueList) {
+ valueDict.put(
+ singleValue.getKey().toString(),
+ singleValue.getValue().toString(),
+ starlarkTransition.getLocationForErrorReporting(),
+ mutability);
+ }
+ optionValue = valueDict;
+ }
+ }
+ } else {
+ if (optionValue instanceof Map.Entry) {
+ SkylarkDict<String, String> valueDict = SkylarkDict.withMutability(mutability);
+ valueDict.put(
+ ((Map.Entry) optionValue).getKey().toString(),
+ ((Map.Entry) optionValue).getValue().toString(),
+ starlarkTransition.getLocationForErrorReporting(),
+ mutability);
+ }
+ }
dict.put(optionKey, optionValue == null ? Runtime.NONE : optionValue, null, mutability);
} catch (IllegalAccessException e) {
// These exceptions should not happen, but if they do, throw a RuntimeException.
@@ -254,17 +284,35 @@
OptionDefinition def = optionInfo.getDefinition();
Field field = def.getField();
FragmentOptions options = buildOptions.get(optionInfo.getOptionClass());
- if (optionValue == null || def.getType().isInstance(optionValue)) {
- field.set(options, optionValue);
- } else if (optionValue instanceof String) {
- field.set(options, def.getConverter().convert((String) optionValue));
+
+ if (!def.allowsMultiple()) {
+ if (optionValue == null || def.getType().isInstance(optionValue)) {
+ field.set(options, optionValue);
+ } else if (optionValue instanceof String) {
+ field.set(options, def.getConverter().convert((String) optionValue));
+ } else {
+ throw new EvalException(
+ starlarkTransition.getLocationForErrorReporting(),
+ "Invalid value type for option '" + optionName + "'");
+ }
} else {
- throw new EvalException(
- starlarkTransition.getLocationForErrorReporting(),
- "Invalid value type for option '" + optionName + "'");
+ SkylarkList rawValues =
+ optionValue instanceof SkylarkList
+ ? (SkylarkList) optionValue
+ : SkylarkList.createImmutable(Collections.singletonList(optionValue));
+ List<Object> allValues = new ArrayList<>(rawValues.size());
+ for (Object singleValue : rawValues) {
+ if (singleValue instanceof String) {
+ allValues.add(def.getConverter().convert((String) singleValue));
+ } else {
+ allValues.add(singleValue);
+ }
+ }
+ field.set(options, ImmutableList.copyOf(allValues));
}
} catch (IllegalAccessException e) {
- throw new RuntimeException(
+ throw new EvalException(
+ starlarkTransition.getLocationForErrorReporting(),
"IllegalAccess for option " + optionName + ": " + e.getMessage());
} catch (OptionsParsingException e) {
throw new EvalException(
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java
index c5bf9d0..ad5ab6c 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java
@@ -17,6 +17,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Maps;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration.EmptyToNullLabelConverter;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
@@ -28,9 +29,12 @@
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
+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 java.util.List;
+import java.util.Map;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -54,6 +58,16 @@
effectTags = {OptionEffectTag.NO_OP},
help = "An option that is sometimes set to null.")
public Label nullable;
+
+ @Option(
+ name = "allow_multiple",
+ converter = Converters.AssignmentConverter.class,
+ defaultValue = "",
+ allowMultiple = true,
+ documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
+ effectTags = {OptionEffectTag.CHANGES_INPUTS, OptionEffectTag.AFFECTS_OUTPUTS},
+ help = "Each --define option specifies an assignment for a build variable.")
+ public List<Map.Entry<String, String>> allowMultiple;
}
/** Loads a new {link @DummyTestFragment} instance. */
@@ -854,4 +868,38 @@
assertThat(configuration.getOptions().get(TestOptions.class).testArguments)
.containsExactly("post-transition");
}
+
+ @Test
+ public void testWriteNativeOption_allowMultipleOption() throws Exception {
+ writeWhitelistFile();
+ scratch.file(
+ "test/transitions.bzl",
+ "def _impl(settings, attr):",
+ " return {",
+ " '//command_line_option:allow_multiple': ['APRIL=SHOWERS', 'MAY=FLOWERS'],",
+ " }",
+ "my_transition = transition(implementation = _impl, inputs = [],",
+ " outputs = ['//command_line_option:allow_multiple'])");
+ scratch.file(
+ "test/rules.bzl",
+ "load('//test:transitions.bzl', 'my_transition')",
+ "def _impl(ctx):",
+ " return []",
+ "my_rule = rule(",
+ " implementation = _impl,",
+ " cfg = my_transition,",
+ " attrs = {",
+ " '_whitelist_function_transition': attr.label(",
+ " default = '//tools/whitelists/function_transition_whitelist',",
+ " ),",
+ " })");
+ scratch.file("test/BUILD", "load('//test:rules.bzl', 'my_rule')", "my_rule(name = 'test')");
+
+ useConfiguration("--allow_multiple=APRIL=FOOLS");
+
+ BuildConfiguration configuration = getConfiguration(getConfiguredTarget("//test"));
+ assertThat(configuration.getOptions().get(DummyTestOptions.class).allowMultiple)
+ .containsExactly(
+ Maps.immutableEntry("APRIL", "SHOWERS"), Maps.immutableEntry("MAY", "FLOWERS"));
+ }
}
diff --git a/src/test/shell/bazel/whitelist_test.sh b/src/test/shell/bazel/whitelist_test.sh
index e3c2b12..8c4ea1b 100755
--- a/src/test/shell/bazel/whitelist_test.sh
+++ b/src/test/shell/bazel/whitelist_test.sh
@@ -99,7 +99,7 @@
--noimplicit_deps --experimental_starlark_config_transitions \
>& $TEST_log || fail "failed to query //vinegar"
expect_log "@secret_ingredient//hotsauce"
- expect_log "test_arg:\[hotlanta\] -> \[\[\"tapatio\"\]\]"
+ expect_log "test_arg:\[hotlanta\] -> \[\[tapatio\]\]"
}
