Allow starlark string flags to mark themselves as allowMultiple.

Duplicate values are not proactively removed. Order is maintained. Values are stored in the BuildOptions starlark options map as a list of strings.

Along the way implement a local cache of build setting targets so we don't repeatedly load them if they're set more than once.

PiperOrigin-RevId: 346798536

site/docs/skylark/config.md

@@ -74,7 +74,7 @@ build setting. The type is limited to a set of basic Starlark types like
  [documentation](lib/config.html)  for details. More complicated typing can be done in the rule's
 implementation function. More on this below.
 
-The `config` function also takes an optional boolean parameter, `flag`, which is
+The `config` module's functions takes an optional boolean parameter, `flag`, which is
 set to false by default. if `flag` is set to true, the build setting can be set
 on the command line by users as well as internally by rule writers via default
 values and
@@ -120,6 +120,38 @@ But all other references to the value of the build setting (e.g. in transitions)
 will see its basic Starlark-typed value, not this post implementation function
 value.
 
+#### Defining multi-set string flags
+String settings have an additional `allow_multiple` parameter which allows the
+flag to be set multiple times on the command line or in bazelrcs. Their default
+value is still set with a string-typed attribute:
+
+```python
+# example/buildsettings/build_settings.bzl
+allow_multiple_flag = rule(
+    implementation = _impl,
+    build_setting = config.string(flag = True, allow_multiple = True)
+)
+```
+
+```python
+# example/buildsettings/BUILD
+load("//example/buildsettings:build_settings.bzl", "allow_multiple_flag")
+allow_multiple_flag(
+    name = "roasts",
+    build_setting_default = "medium"
+)
+```
+
+Each setting of the flag is treated as a single value:
+
+```shell
+$ bazel build //my/target --//example:roasts=blonde \
+    --//example:roasts=medium,dark
+```
+
+The above will be parsed to {//example:roasts:["blonde", "medium,dark"]} and
+`ctx.build_setting_value` will return a list ["blonde", "medium,dark"].
+
 #### Instantiating Build Settings
 
 Rules defined with the `build_setting` parameter have an implicit mandatory
@@ -558,6 +590,40 @@ setting the individual flags to their appropriate values. Unfortunately this
 requires maintaining the expansion in two places. Note that this workaround
 does not allow for command-specific behavior like `--config` does.
 
+### Transitions on allow multiple build settings
+
+When setting build settings that
+[allow multiple values](#defining-multi-set-string-flags), the value of the
+setting must be set with a list.
+
+```python
+# example/buildsettings/build_settings.bzl
+string_flag = rule(
+    implementation = _impl,
+    build_setting = config.string(flag = True, allow_multiple = True)
+)
+```
+
+```python
+# example/BUILD
+load("//example/buildsettings:build_settings.bzl", "string_flag")
+string_flag(name = "roasts", build_setting_default = "medium")
+```
+
+```python
+# example/transitions/rules.bzl
+def _transition_impl(settings, attr):
+    # Using a value of just "dark" here will throw an error
+    return {"//example:roasts" : ["dark"]},
+
+coffee_transition = transition(
+    implementation = _transition_impl,
+    inputs = [],
+    outputs = ["//example:roasts"]
+)
+```
+
+
 ### Accessing attributes with transitions
 
 [End to end example](https://github.com/bazelbuild/examples/tree/master/rules/starlark_configurations/read_attr_in_transition)

src/main/java/com/google/devtools/build/lib/analysis/BuildSettingProvider.java

@@ -45,6 +45,10 @@ public Type<?> getType() {
     return buildSetting.getType();
   }
 
+  public boolean allowsMultiple() {
+    return buildSetting.allowsMultiple();
+  }
+
   public Object getDefaultValue() {
     return defaultValue;
   }

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkConfig.java

@@ -31,22 +31,22 @@ public class StarlarkConfig implements StarlarkConfigApi {
 
   @Override
   public BuildSetting intSetting(Boolean flag) {
-    return new BuildSetting(flag, INTEGER);
+    return BuildSetting.create(flag, INTEGER);
   }
 
   @Override
   public BuildSetting boolSetting(Boolean flag) {
-    return new BuildSetting(flag, BOOLEAN);
+    return BuildSetting.create(flag, BOOLEAN);
   }
 
   @Override
-  public BuildSetting stringSetting(Boolean flag) {
-    return new BuildSetting(flag, STRING);
+  public BuildSetting stringSetting(Boolean flag, Boolean allowMultiple) {
+    return BuildSetting.create(flag, STRING, allowMultiple);
   }
 
   @Override
   public BuildSetting stringListSetting(Boolean flag) {
-    return new BuildSetting(flag, STRING_LIST);
+    return BuildSetting.create(flag, STRING_LIST);
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkTransition.java

@@ -17,6 +17,7 @@
 import static com.google.devtools.build.lib.packages.RuleClass.Builder.STARLARK_BUILD_SETTING_DEFAULT_ATTR_NAME;
 
 import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
@@ -32,12 +33,14 @@
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.packages.Type;
 import com.google.devtools.build.lib.packages.Type.ConversionException;
 import com.google.devtools.build.lib.skyframe.PackageValue;
 import com.google.devtools.build.lib.util.ClassName;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -323,21 +326,51 @@ public static Map<String, BuildOptions> validate(
             changedSettingToRule.keySet(),
             actualSetting,
             options.getStarlarkOptions().keySet());
-        Object convertedValue;
-        try {
-          convertedValue =
-              rule.getRuleClassObject()
-                  .getBuildSetting()
-                  .getType()
-                  .convert(newValue, maybeAliasSetting);
-        } catch (ConversionException e) {
-          throw new TransitionException(e);
-        }
-        if (convertedValue.equals(rule.getAttr(STARLARK_BUILD_SETTING_DEFAULT_ATTR_NAME))) {
-          if (cleanedOptions == null) {
-            cleanedOptions = options.toBuilder();
+        boolean allowsMultiple = rule.getRuleClassObject().getBuildSetting().allowsMultiple();
+        if (allowsMultiple) {
+          // if this setting allows multiple settings
+          if (!(newValue instanceof List)) {
+            throw new TransitionException(
+                String.format(
+                    "'%s' allows multiple values and must be set"
+                        + " in transition using a starlark list instead of single value '%s'",
+                    actualSetting, newValue));
+          }
+          List<?> rawNewValueAsList = (List<?>) newValue;
+          List<Object> convertedValue = new ArrayList<>();
+          Type<?> type = rule.getRuleClassObject().getBuildSetting().getType();
+          for (Object value : rawNewValueAsList) {
+            try {
+              convertedValue.add(type.convert(value, maybeAliasSetting));
+            } catch (ConversionException e) {
+              throw new TransitionException(e);
+            }
+          }
+          if (convertedValue.equals(
+              ImmutableList.of(rule.getAttr(STARLARK_BUILD_SETTING_DEFAULT_ATTR_NAME)))) {
+            if (cleanedOptions == null) {
+              cleanedOptions = options.toBuilder();
+            }
+            cleanedOptions.removeStarlarkOption(rule.getLabel());
+          }
+        } else {
+          // if this setting does not allow multiple settings
+          Object convertedValue;
+          try {
+            convertedValue =
+                rule.getRuleClassObject()
+                    .getBuildSetting()
+                    .getType()
+                    .convert(newValue, maybeAliasSetting);
+          } catch (ConversionException e) {
+            throw new TransitionException(e);
+          }
+          if (convertedValue.equals(rule.getAttr(STARLARK_BUILD_SETTING_DEFAULT_ATTR_NAME))) {
+            if (cleanedOptions == null) {
+              cleanedOptions = options.toBuilder();
+            }
+            cleanedOptions.removeStarlarkOption(rule.getLabel());
           }
-          cleanedOptions.removeStarlarkOption(rule.getLabel());
         }
       }
       // Keep the same instance if we didn't do anything to maintain reference equality later on.

src/main/java/com/google/devtools/build/lib/packages/BuildSetting.java

@@ -24,10 +24,20 @@
 public class BuildSetting implements BuildSettingApi {
   private final boolean isFlag;
   private final Type<?> type;
+  private final boolean allowMultiple;
 
-  public BuildSetting(boolean isFlag, Type<?> type) {
+  private BuildSetting(boolean isFlag, Type<?> type, boolean allowMultiple) {
     this.isFlag = isFlag;
     this.type = type;
+    this.allowMultiple = allowMultiple;
+  }
+
+  public static BuildSetting create(boolean isFlag, Type<?> type, boolean allowMultiple) {
+    return new BuildSetting(isFlag, type, allowMultiple);
+  }
+
+  public static BuildSetting create(boolean isFlag, Type<?> type) {
+    return new BuildSetting(isFlag, type, /* allowMultiple= */ false);
   }
 
   public Type<?> getType() {
@@ -39,6 +49,10 @@ public boolean isFlag() {
     return isFlag;
   }
 
+  public boolean allowsMultiple() {
+    return allowMultiple;
+  }
+
   @Override
   public void repr(Printer printer) {
     printer.append("<build_setting." + type + ">");

src/main/java/com/google/devtools/build/lib/rules/LabelBuildSettings.java

@@ -80,7 +80,7 @@ private static RuleClass buildRuleClass(RuleClass.Builder builder, boolean flag)
         .removeAttribute("licenses")
         .removeAttribute("distribs")
         .add(attr(":alias", LABEL).value(ACTUAL))
-        .setBuildSetting(new BuildSetting(flag, LABEL))
+        .setBuildSetting(BuildSetting.create(flag, LABEL))
         .canHaveAnyProvider()
         .useToolchainResolution(false)
         .build();

src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java

@@ -494,20 +494,25 @@ static UserDefinedFlagMatch fromAttributeValueAndPrerequisites(
           }
 
           if (configurationValue instanceof List) {
-            // If the build_setting is a list, we use the same semantics as for multi-value native
-            // flags: if *any* entry in the list matches the config_setting's expected entry, it's
-            // a match. In other words, config_setting(flag_values {"//foo": "bar"} matches
-            // //foo=["bar", "baz"].
-
-            // If the config_setting expects "foo", convertedSpecifiedValue converts it to the
-            // flag's native type, which produces ["foo"]. So unpack that again.
-            Iterable<?> specifiedValueAsIterable = (Iterable<?>) convertedSpecifiedValue;
+            // If the build_setting is a list it's either an allow-multiple string-typed build
+            // setting or a string_list-typed build setting. We use the same semantics as for
+            // multi-value native flags: if *any* entry in the list matches the config_setting's
+            // expected entry, it's a match. In other words,
+            // config_setting(flag_values {"//foo": "bar"} matches //foo=["bar", "baz"].
+
+            // If this is an allow-multiple build setting, the converter will have converted the
+            // config settings value to a singular object, if it's a string_list build setting the
+            // converter will have converted it to a list.
+            Iterable<?> specifiedValueAsIterable =
+                provider.allowsMultiple()
+                    ? ImmutableList.of(convertedSpecifiedValue)
+                    : (Iterable<?>) convertedSpecifiedValue;
             if (Iterables.size(specifiedValueAsIterable) != 1) {
               ruleContext.attributeError(
                   ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE,
                   String.format(
-                      "\"%s\" not a valid value for flag %s. Only single, exact values are allowed."
-                          + " If you want to match multiple values, consider Skylib's "
+                      "\"%s\" not a valid value for flag %s. Only single, exact values are"
+                          + " allowed. If you want to match multiple values, consider Skylib's "
                           + "selects.config_setting_group",
                       specifiedValue, specifiedLabel));
               matches = false;