Make config_setting work with list-typed Starlark flags.
Before this change, no list-typed Starlark flag match
any config_setting, period.
After this change, a config_setting expecting value "foo"
matches any list that contains "foo" in any of its entries.
This is consistent with config_setting's existing semantics
for multi-value native flags (like "--define foo --define bar").
Fixes https://github.com/bazelbuild/bazel/issues/8939.
PiperOrigin-RevId: 259370619
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java
index 25aedb6..ebdf33d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java
@@ -154,9 +154,6 @@
/* <!-- #BLAZE_RULE(config_setting).ATTRIBUTE(values) -->
The set of configuration values that match this rule (expressed as Bazel flags)
- <i>(Dictionary mapping flags to expected values, both expressed as strings;
- mandatory)</i>
-
<p>This rule inherits the configuration of the configured target that
references it in a <code>select</code> statement. It is considered to
"match" a Bazel invocation if, for every entry in the dictionary, its
@@ -193,14 +190,9 @@
The same as <a href="${link config_setting.values}"><code>values</code></a> but
specifically for the <code>--define</code> flag.
- <p><code>--define</code> is special for two reasons:
-
- <ol>
- <li>It's the primary interface Bazel has today for declaring user-definable settings.
- </li>
- <li>Its syntax (<code>--define KEY=VAL</code>) means <code>KEY=VAL</code> is
- a <i>value</i> from a Bazel flag perspective.</li>
- </ol>
+ <p><code>--define</code> is special because its syntax (<code>--define KEY=VAL</code>)
+ means <code>KEY=VAL</code> is a <i>value</i> from a Bazel flag perspective.
+ </p>
<p>That means:
@@ -225,7 +217,7 @@
})
</pre>
- <p>corrrectly matches <code>bazel build //foo --define a=1 --define b=2</code>.
+ <p>correctly matches <code>bazel build //foo --define a=1 --define b=2</code>.
<p><code>--define</code> can still appear in
<a href="${link config_setting.values}"><code>values</code></a> with normal flag syntax,
@@ -234,6 +226,13 @@
.add(
attr(DEFINE_SETTINGS_ATTRIBUTE, STRING_DICT)
.nonconfigurable(NONCONFIGURABLE_ATTRIBUTE_REASON))
+
+ /* <!-- #BLAZE_RULE(config_setting).ATTRIBUTE(flag_values) -->
+ The same as <a href="${link config_setting.values}"><code>values</code></a> but
+ for <a href="https://docs.bazel.build/versions/master/skylark/config.html#user-defined-build-settings">
+ Starlark-defined flags</a>.
+ <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
+
// Originally this attribute was a map of feature flags targets -> feature flag values,
// the latter of which are always strings. Now it also includes starlark build setting
// targets -> starlark build setting values. In other places in the starlark configuration
@@ -243,7 +242,6 @@
// label->object dict is not an option for attribute types right now.
.add(
attr(FLAG_SETTINGS_ATTRIBUTE, LABEL_KEYED_STRING_DICT)
- .undocumented("the feature flag feature has not yet been launched")
.allowedFileTypes()
.nonconfigurable(NONCONFIGURABLE_ATTRIBUTE_REASON))
/* <!-- #BLAZE_RULE(config_setting).ATTRIBUTE(constraint_values) -->
@@ -349,11 +347,18 @@
match invocations using either form.
</p>
- <p>The currently endorsed method for creating custom conditions that can't be expressed through
- dedicated build flags is through the <code>--define</code> flag. Use this flag with caution:
- it's not ideal and only endorsed for lack of a currently better workaround. See the
- <a href="${link common-definitions#configurable-attributes}">
- Configurable attributes</a> section for more discussion.
+ <p>
+ If a flag takes multiple values (like <code>--define a=b --define c=d</code> or a list-typed
+ <a href="https://docs.bazel.build/versions/master/skylark/config.html#user-defined-build-settings">
+ Starlark flag</a>), a <code>config_setting</code> that expects <code>"foo"</code> matches if
+ <code>"foo"</code> is present <i>anywhere</i> in the actual list.
+ </p>
+
+ <p>If you need to define conditions that aren't modeled by built-in Bazel flags, use
+ <a href="https://docs.bazel.build/versions/master/skylark/config.html#user-defined-build-settings">
+ Starlark-defined flags</a>. You can also use <code>--define</code>, but this offers weaker
+ support and is not recommended. See <a href="${link common-definitions#configurable-attributes}">
+ for more discussion.
</p>
<p>Avoid repeating identical <code>config_setting</code> definitions in different packages.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java
index 38d2bd9..83d27d5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java
@@ -469,7 +469,21 @@
matches = false;
continue;
}
- if (!configurationValue.equals(convertedSpecifiedValue)) {
+
+ 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.
+ Object specifiedUnpacked =
+ Iterables.getOnlyElement((Iterable<?>) convertedSpecifiedValue);
+ if (!((List<?>) configurationValue).contains(specifiedUnpacked)) {
+ matches = false;
+ }
+ } else if (!configurationValue.equals(convertedSpecifiedValue)) {
matches = false;
}
} else {
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
index c250a6a..4262a6e 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
@@ -1739,4 +1739,101 @@
useConfiguration("--copt", "-Dfoo");
assertThat(getLicenses("//test:match")).containsExactly(LicenseType.NONE);
}
+
+ @Test
+ public void simpleStarlarkFlag() throws Exception {
+ scratch.file(
+ "test/flagdef.bzl",
+ "def _impl(ctx):",
+ " return []",
+ "my_flag = rule(",
+ " implementation = _impl,",
+ " build_setting = config.string(flag = True))");
+ scratch.file(
+ "test/BUILD",
+ "load('//test:flagdef.bzl', 'my_flag')",
+ "my_flag(",
+ " name = 'flag',",
+ " build_setting_default = 'actual_flag_value')",
+ "config_setting(",
+ " name = 'matches',",
+ " flag_values = {",
+ " ':flag': 'actual_flag_value',",
+ " })",
+ "config_setting(",
+ " name = 'doesntmatch',",
+ " flag_values = {",
+ " ':flag': 'other_flag_value',",
+ " })");
+ assertThat(getConfigMatchingProvider("//test:matches").matches()).isTrue();
+ assertThat(getConfigMatchingProvider("//test:doesntmatch").matches()).isFalse();
+ }
+
+ @Test
+ public void starlarkListFlagSingleValue() throws Exception {
+ // When a list-typed Starlark flag has value ["foo"], the config_setting's expected value "foo"
+ // must match exactly.
+ scratch.file(
+ "test/flagdef.bzl",
+ "def _impl(ctx):",
+ " return []",
+ "my_flag = rule(",
+ " implementation = _impl,",
+ " build_setting = config.string_list(flag = True))");
+ scratch.file(
+ "test/BUILD",
+ "load('//test:flagdef.bzl', 'my_flag')",
+ "my_flag(",
+ " name = 'one_value_flag',",
+ " build_setting_default = ['one'])",
+ "config_setting(",
+ " name = 'matches',",
+ " flag_values = {",
+ " ':one_value_flag': 'one',",
+ " })",
+ "config_setting(",
+ " name = 'doesntmatch',",
+ " flag_values = {",
+ " ':one_value_flag': 'other',",
+ " })");
+ assertThat(getConfigMatchingProvider("//test:matches").matches()).isTrue();
+ assertThat(getConfigMatchingProvider("//test:doesntmatch").matches()).isFalse();
+ }
+
+ @Test
+ public void starlarkListFlagMultiValue() throws Exception {
+ // When a list-typed Starlark flag has value ["foo", "bar"], the config_setting's expected
+ // value "foo" must match *any* entry in the list.
+ scratch.file(
+ "test/flagdef.bzl",
+ "def _impl(ctx):",
+ " return []",
+ "my_flag = rule(",
+ " implementation = _impl,",
+ " build_setting = config.string_list(flag = True))");
+ scratch.file(
+ "test/BUILD",
+ "load('//test:flagdef.bzl', 'my_flag')",
+ "my_flag(",
+ " name = 'two_value_flag',",
+ " build_setting_default = ['one', 'two'])",
+ "config_setting(",
+ " name = 'matches_one',",
+ " flag_values = {",
+ " ':two_value_flag': 'one',",
+ " })",
+ "config_setting(",
+ " name = 'matches_two',",
+ " flag_values = {",
+ " ':two_value_flag': 'two',",
+ " })",
+ "config_setting(",
+ " name = 'doesntmatch',",
+ " flag_values = {",
+ " ':two_value_flag': 'other',",
+ " })");
+ assertThat(getConfigMatchingProvider("//test:matches_one").matches()).isTrue();
+ assertThat(getConfigMatchingProvider("//test:matches_two").matches()).isTrue();
+ assertThat(getConfigMatchingProvider("//test:doesntmatch").matches()).isFalse();
+ }
}
