Fix //conditions:default excludes computation for LabelListAttribute.

Previously, excludes computation for the default condition in label list
selects was clobbering pre-existing values. This CL fixes it by
performing a union of the new values with existing ones instead.

Test: properties_test.go
Bug: 198556411
Change-Id: Id11f4fb14e359201304afde0d8ba856851d41395
This commit is contained in:
Jingwen Chen
2021-11-02 10:27:17 +00:00
parent 55bc820d66
commit 9af49a49f8
2 changed files with 36 additions and 19 deletions

View File

@@ -534,9 +534,13 @@ func (lla *LabelListAttribute) ResolveExcludes() {
lla.ConfigurableValues[axis][config] = SubtractBazelLabelList(val, lla.Value) lla.ConfigurableValues[axis][config] = SubtractBazelLabelList(val, lla.Value)
} }
// Now that the Value list is finalized for this axis, compare it with the original // Now that the Value list is finalized for this axis, compare it with
// list, and put the difference into the default condition for the axis. // the original list, and union the difference with the default
lla.ConfigurableValues[axis][ConditionsDefaultConfigKey] = SubtractBazelLabelList(baseLabels, lla.Value) // condition for the axis.
difference := SubtractBazelLabelList(baseLabels, lla.Value)
existingDefaults := lla.ConfigurableValues[axis][ConditionsDefaultConfigKey]
existingDefaults.Append(difference)
lla.ConfigurableValues[axis][ConditionsDefaultConfigKey] = FirstUniqueBazelLabelList(existingDefaults)
// if everything ends up without includes, just delete the axis // if everything ends up without includes, just delete the axis
if !lla.ConfigurableValues[axis].HasConfigurableValues() { if !lla.ConfigurableValues[axis].HasConfigurableValues() {

View File

@@ -236,8 +236,9 @@ func TestResolveExcludes(t *testing.T) {
), ),
ConfigurableValues: configurableLabelLists{ ConfigurableValues: configurableLabelLists{
ArchConfigurationAxis: labelListSelectValues{ ArchConfigurationAxis: labelListSelectValues{
"arm": makeLabelList([]string{}, []string{"arm_exclude"}), "arm": makeLabelList([]string{}, []string{"arm_exclude"}),
"x86": makeLabelList([]string{"x86_include"}, []string{}), "x86": makeLabelList([]string{"x86_include"}, []string{}),
ConditionsDefaultConfigKey: makeLabelList([]string{"default_include"}, []string{}),
}, },
OsConfigurationAxis: labelListSelectValues{ OsConfigurationAxis: labelListSelectValues{
"android": makeLabelList([]string{}, []string{"android_exclude"}), "android": makeLabelList([]string{}, []string{"android_exclude"}),
@@ -246,7 +247,13 @@ func TestResolveExcludes(t *testing.T) {
OsArchConfigurationAxis: labelListSelectValues{ OsArchConfigurationAxis: labelListSelectValues{
"linux_x86": makeLabelList([]string{"linux_x86_include"}, []string{}), "linux_x86": makeLabelList([]string{"linux_x86_include"}, []string{}),
}, },
ProductVariableConfigurationAxis("a"): labelListSelectValues{ ProductVariableConfigurationAxis("product_with_defaults"): labelListSelectValues{
"a": makeLabelList([]string{}, []string{"not_in_value"}),
"b": makeLabelList([]string{"b_val"}, []string{}),
"c": makeLabelList([]string{"c_val"}, []string{}),
ConditionsDefaultConfigKey: makeLabelList([]string{"c_val", "default", "default2"}, []string{}),
},
ProductVariableConfigurationAxis("product_only_with_excludes"): labelListSelectValues{
"a": makeLabelList([]string{}, []string{"not_in_value"}), "a": makeLabelList([]string{}, []string{"not_in_value"}),
}, },
}, },
@@ -254,25 +261,31 @@ func TestResolveExcludes(t *testing.T) {
attr.ResolveExcludes() attr.ResolveExcludes()
expectedBaseIncludes := []Label{Label{Label: "all_include"}} expectedBaseIncludes := []Label{{Label: "all_include"}}
if !reflect.DeepEqual(expectedBaseIncludes, attr.Value.Includes) { if !reflect.DeepEqual(expectedBaseIncludes, attr.Value.Includes) {
t.Errorf("Expected Value includes %q, got %q", attr.Value.Includes, expectedBaseIncludes) t.Errorf("Expected Value includes %q, got %q", attr.Value.Includes, expectedBaseIncludes)
} }
var nilLabels []Label var nilLabels []Label
expectedConfiguredIncludes := map[ConfigurationAxis]map[string][]Label{ expectedConfiguredIncludes := map[ConfigurationAxis]map[string][]Label{
ArchConfigurationAxis: map[string][]Label{ ArchConfigurationAxis: {
"arm": nilLabels, "arm": nilLabels,
"x86": makeLabels("arm_exclude", "x86_include"), "x86": makeLabels("arm_exclude", "x86_include"),
"conditions_default": makeLabels("arm_exclude"), ConditionsDefaultConfigKey: makeLabels("arm_exclude", "default_include"),
}, },
OsConfigurationAxis: map[string][]Label{ OsConfigurationAxis: {
"android": nilLabels, "android": nilLabels,
"linux": makeLabels("android_exclude", "linux_include"), "linux": makeLabels("android_exclude", "linux_include"),
"conditions_default": makeLabels("android_exclude"), ConditionsDefaultConfigKey: makeLabels("android_exclude"),
}, },
OsArchConfigurationAxis: map[string][]Label{ OsArchConfigurationAxis: {
"linux_x86": makeLabels("linux_x86_include"), "linux_x86": makeLabels("linux_x86_include"),
"conditions_default": nilLabels, ConditionsDefaultConfigKey: nilLabels,
},
ProductVariableConfigurationAxis("product_with_defaults"): {
"a": nilLabels,
"b": makeLabels("b_val"),
"c": makeLabels("c_val"),
ConditionsDefaultConfigKey: makeLabels("c_val", "default", "default2"),
}, },
} }
for _, axis := range attr.SortedConfigurationAxes() { for _, axis := range attr.SortedConfigurationAxes() {
@@ -288,7 +301,7 @@ func TestResolveExcludes(t *testing.T) {
for config, value := range gotForAxis { for config, value := range gotForAxis {
if expected, ok := expectedForAxis[config]; ok { if expected, ok := expectedForAxis[config]; ok {
if !reflect.DeepEqual(expected, value.Includes) { if !reflect.DeepEqual(expected, value.Includes) {
t.Errorf("For %s, expected: %#v, got %#v", axis, expected, value.Includes) t.Errorf("For %s,\nexpected: %#v\ngot %#v", axis, expected, value.Includes)
} }
} else { } else {
t.Errorf("Got unexpected config %q for %s", config, axis) t.Errorf("Got unexpected config %q for %s", config, axis)