From d44c3f98f717328d399899005e6e5ed1263e9725 Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 27 Nov 2023 08:21:56 -0800 Subject: [PATCH] Accept labels of aliases in config_setting. Previously, if a config_setting referenced a Starlark setting through an alias, it was always evaluated as if the setting had its default value. Now, it is evaluated correctly. This is done by looking up the value of the build setting in the configuration based on its own label as specified in BuildSettingProvider. Fixes #13463 . RELNOTES: None. PiperOrigin-RevId: 585658985 Change-Id: Id534cd95282355f1143302bf703145bb53708a41 --- .../build/lib/rules/config/ConfigSetting.java | 12 +++++--- .../lib/rules/LabelBuildSettingTest.java | 29 +++++++++++++++++++ .../lib/rules/config/ConfigSettingTest.java | 21 ++++++++++++++ 3 files changed, 58 insertions(+), 4 deletions(-) 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 cc06b86450274f..cb70244ed28eea 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 @@ -497,10 +497,14 @@ static UserDefinedFlagMatch fromAttributeValueAndPrerequisites( } else if (target.satisfies(BuildSettingProvider.REQUIRE_BUILD_SETTING_PROVIDER)) { // build setting BuildSettingProvider provider = target.getProvider(BuildSettingProvider.class); - Object configurationValue = - optionDetails.getOptionValue(specifiedLabel) != null - ? optionDetails.getOptionValue(specifiedLabel) - : provider.getDefaultValue(); + + Object configurationValue; + if (optionDetails.getOptionValue(provider.getLabel()) != null) { + configurationValue = optionDetails.getOptionValue(provider.getLabel()); + } else { + configurationValue = provider.getDefaultValue(); + } + Object convertedSpecifiedValue; try { // We don't need to supply a base package or repo mapping for the conversion here, diff --git a/src/test/java/com/google/devtools/build/lib/rules/LabelBuildSettingTest.java b/src/test/java/com/google/devtools/build/lib/rules/LabelBuildSettingTest.java index 5500ab253823fc..013cc9a02580d3 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/LabelBuildSettingTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/LabelBuildSettingTest.java @@ -116,6 +116,35 @@ public void testLabelFlag_set() throws Exception { assertThat(b.get("value")).isEqualTo("command_line_value"); } + @Test + public void withSelectThroughAlias() throws Exception { + writeRulesBzl("flag"); + scratch.file( + "test/BUILD", + "load('//test:rules.bzl', 'my_rule', 'simple_rule')", + "simple_rule(name = 'default', value = 'default_value')", + "simple_rule(name = 'command_line', value = 'command_line_value')", + "label_flag(name = 'my_label_flag', build_setting_default = ':default')", + "alias(name = 'my_label_flag_alias', actual = ':my_label_flag')", + "config_setting(", + " name = 'is_default_label',", + " flag_values = {':my_label_flag_alias': '//test:default'}", + ")", + "simple_rule(name = 'selector', value = select({':is_default_label': 'valid'}))"); + + useConfiguration(); + getConfiguredTarget("//test:selector"); + assertNoEvents(); + + reporter.removeHandler(failFastHandler); + useConfiguration( + ImmutableMap.of( + "//test:my_label_flag", Label.parseCanonicalUnchecked("//test:command_line"))); + getConfiguredTarget("//test:selector"); + assertContainsEvent( + "configurable attribute \"value\" in //test:selector doesn't match this configuration"); + } + @Test public void withSelect() throws Exception { writeRulesBzl("flag"); 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 b4bf5b70672996..5bb6968eb13617 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 @@ -2177,6 +2177,27 @@ public void ruleLicensesUsed() throws Exception { assertThat(getLicenses("//test:match")).containsExactly(LicenseType.NONE); } + @Test + public void aliasedStarlarkFlag() 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 = 'default')", + "alias(name = 'alias', actual = ':flag')", + "config_setting(name = 'alias_setting', flag_values = {':alias': 'specified'})"); + + useConfiguration(ImmutableMap.of("//test:flag", "specified")); + assertThat(getConfigMatchingProviderResultAsBoolean("//test:alias_setting")).isTrue(); + } + @Test public void simpleStarlarkFlag() throws Exception { scratch.file(