diff --git a/src/main/java/com/google/devtools/build/lib/packages/Type.java b/src/main/java/com/google/devtools/build/lib/packages/Type.java index 1c21e68c158819..9d22f5b05841eb 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Type.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Type.java @@ -620,11 +620,11 @@ public Object copyAndLiftStarlarkValue( @Override public Map concat(Iterable> iterable) { - Dict.Builder output = new Dict.Builder<>(); + ImmutableMap.Builder builder = ImmutableMap.builder(); for (Map map : iterable) { - output.putAll(map); + builder.putAll(map); } - return output.buildImmutable(); + return builder.buildKeepingLast(); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java b/src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java index e4ae5b68fc09db..609b8cb11ce432 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.util.BuildViewTestBase.AnalysisFailureRecorder; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.analysis.util.DummyTestFragment; @@ -31,10 +32,12 @@ import com.google.devtools.build.lib.packages.NoSuchTargetException; import com.google.devtools.build.lib.packages.RuleClass.ToolchainResolutionMode; import com.google.devtools.build.lib.packages.Type; +import com.google.devtools.build.lib.packages.Types; import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; import com.google.devtools.build.lib.util.FileTypeSet; import java.io.IOException; +import java.util.Arrays; import java.util.Collection; import java.util.Set; import org.junit.Before; @@ -166,6 +169,13 @@ public Object getDefault(AttributeMap rule) { .add(attr("deps", LABEL_LIST).allowedFileTypes()) .toolchainResolutionMode(ToolchainResolutionMode.DISABLED)); + + private static final MockRule RULE_WITH_STRING_LIST_DICT_ATTR = + () -> MockRule.define( + "rule_with_string_list_dict_attr", + attr("string_list_dict_attr", Types.STRING_LIST_DICT + )); + @Override protected ConfiguredRuleClassProvider createRuleClassProvider() { ConfiguredRuleClassProvider.Builder builder = @@ -175,7 +185,8 @@ protected ConfiguredRuleClassProvider createRuleClassProvider() { .addRuleDefinition(RULE_WITH_BOOLEAN_ATTR) .addRuleDefinition(RULE_WITH_ALLOWED_VALUES) .addRuleDefinition(RULE_WITH_LABEL_DEFAULT) - .addRuleDefinition(RULE_WITH_NO_PLATFORM); + .addRuleDefinition(RULE_WITH_NO_PLATFORM) + .addRuleDefinition(RULE_WITH_STRING_LIST_DICT_ATTR); TestRuleClassProvider.addStandardRules(builder); // Allow use of --foo as a dummy flag builder.addConfigurationFragment(DummyTestFragment.class); @@ -2115,4 +2126,24 @@ def my_java_binary(name, deps = [], **kwargs): /*expected:*/ ImmutableList.of("bin java/foo/libb.jar", "bin java/foo/libb2.jar"), /*not expected:*/ ImmutableList.of("bin java/foo/liba.jar", "bin java/foo/liba2.jar")); } + + @Test + public void stringListDictTypeConcatConfigurable() throws Exception { + writeConfigRules(); + scratch.file( + "foo/BUILD", + """ + rule_with_string_list_dict_attr( + name = 'rule', + string_list_dict_attr = {'a': ['a.out']} | select({ + '//conditions:b': {'b': ['b.out']}, + })) + """); + + useConfiguration("--foo=b"); + ConfiguredTargetAndData ctad = getConfiguredTargetAndData("//foo:rule"); + AttributeMap attributes = getMapperFromConfiguredTargetAndTarget(ctad); + assertThat(attributes.get("string_list_dict_attr", Types.STRING_LIST_DICT)) + .isEqualTo(ImmutableMap.of("a", Arrays.asList("a.out"), "b", Arrays.asList("b.out"))); + } } diff --git a/src/test/java/com/google/devtools/build/lib/packages/TypeTest.java b/src/test/java/com/google/devtools/build/lib/packages/TypeTest.java index a15405b261ba94..db1c551af515a3 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/TypeTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/TypeTest.java @@ -374,6 +374,37 @@ public void testStringListDict() throws Exception { assertThat(collectLabels(Types.STRING_LIST_DICT, converted)).isEmpty(); } + @Test + public void testStringListDict_concat() throws Exception { + assertThat(Types.STRING_LIST_DICT.concat(ImmutableList.of())) + .isEqualTo(ImmutableMap.of()); + + Map> expected = + ImmutableMap.of( + "foo", Arrays.asList("foo", "bar"), + "wiz", Arrays.asList("bang")); + assertThat(Types.STRING_LIST_DICT.concat(ImmutableList.of(expected))) + .isEqualTo(expected); + + Map> map1 = + ImmutableMap.of( + "foo", Arrays.asList("a", "b"), + "bar", Arrays.asList("c", "d")); + Map> map2 = + ImmutableMap.of( + "bar", Arrays.asList("x", "y"), + "baz", Arrays.asList("z")); + + Map> expected_after_concat = + ImmutableMap.of( + "foo", Arrays.asList("a", "b"), + "bar", Arrays.asList("x", "y"), + "baz", Arrays.asList("z")); + + assertThat(Types.STRING_LIST_DICT.concat(ImmutableList.of(map1, map2))) + .isEqualTo(expected_after_concat); + } + @Test public void testStringListDictBadFirstElement() throws Exception { Object input =