Fix nondeterminism in bp2build
This fixes two main sources of nondeterminism:
1. Fix a bug in the ConfigurationAxis comparator (which caused
ConfigurationAxis sorting to be nondeterministic)
2. Process C++ dependencies using the sorted ConfigurationAxis order. In
theory, the order in which dependencies are processed shouldn't
matter (as they should end up in different select stanzas). However,
in the case of InApex stubs, this is not the case; we now ensure
that lists are concatenated in a predictable order.
Added bonus: Some cleanup with SortConfigurationAxes which
makes use of go generics (this made it easier to debug this issue).
Will follow-up with regression tests.
Test: Manually verified that build.ninja checksum and BUILD.bazel checksums do not change after running `m nothing` 6 times in AOSP (with comment-only Android.bp changes in between each run)
Change-Id: I81168e45bdbbcd61ea95ff665cf6c4bc180aa4e0
diff --git a/bazel/configurability.go b/bazel/configurability.go
index 2b8753b..4680256 100644
--- a/bazel/configurability.go
+++ b/bazel/configurability.go
@@ -343,8 +343,8 @@
}
func (ca *ConfigurationAxis) less(other ConfigurationAxis) bool {
- if ca.configurationType < other.configurationType {
- return true
+ if ca.configurationType == other.configurationType {
+ return ca.subType < other.subType
}
- return ca.subType < other.subType
+ return ca.configurationType < other.configurationType
}
diff --git a/bazel/properties.go b/bazel/properties.go
index 0fca60b..eb20eec 100644
--- a/bazel/properties.go
+++ b/bazel/properties.go
@@ -73,6 +73,16 @@
}
}
+func SortedConfigurationAxes[T any](m map[ConfigurationAxis]T) []ConfigurationAxis {
+ keys := make([]ConfigurationAxis, 0, len(m))
+ for k := range m {
+ keys = append(keys, k)
+ }
+
+ sort.Slice(keys, func(i, j int) bool { return keys[i].less(keys[j]) })
+ return keys
+}
+
// MakeLabelListFromTargetNames creates a LabelList from unqualified target names
// This is a utiltity function for bp2build converters of Soong modules that have 1:many generated targets
func MakeLabelListFromTargetNames(targetNames []string) LabelList {
@@ -412,13 +422,7 @@
// SortedConfigurationAxes returns all the used ConfigurationAxis in sorted order.
func (la *LabelAttribute) SortedConfigurationAxes() []ConfigurationAxis {
- keys := make([]ConfigurationAxis, 0, len(la.ConfigurableValues))
- for k := range la.ConfigurableValues {
- keys = append(keys, k)
- }
-
- sort.Slice(keys, func(i, j int) bool { return keys[i].less(keys[j]) })
- return keys
+ return SortedConfigurationAxes(la.ConfigurableValues)
}
// MakeLabelAttribute turns a string into a LabelAttribute
@@ -608,13 +612,7 @@
// SortedConfigurationAxes returns all the used ConfigurationAxis in sorted order.
func (ba *BoolAttribute) SortedConfigurationAxes() []ConfigurationAxis {
- keys := make([]ConfigurationAxis, 0, len(ba.ConfigurableValues))
- for k := range ba.ConfigurableValues {
- keys = append(keys, k)
- }
-
- sort.Slice(keys, func(i, j int) bool { return keys[i].less(keys[j]) })
- return keys
+ return SortedConfigurationAxes(ba.ConfigurableValues)
}
// labelListSelectValues supports config-specific label_list typed Bazel attribute values.
@@ -761,13 +759,7 @@
// SortedConfigurationAxes returns all the used ConfigurationAxis in sorted order.
func (lla *LabelListAttribute) SortedConfigurationAxes() []ConfigurationAxis {
- keys := make([]ConfigurationAxis, 0, len(lla.ConfigurableValues))
- for k := range lla.ConfigurableValues {
- keys = append(keys, k)
- }
-
- sort.Slice(keys, func(i, j int) bool { return keys[i].less(keys[j]) })
- return keys
+ return SortedConfigurationAxes(lla.ConfigurableValues)
}
// Append all values, including os and arch specific ones, from another
@@ -1145,13 +1137,7 @@
// SortedConfigurationAxes returns all the used ConfigurationAxis in sorted order.
func (sa *StringAttribute) SortedConfigurationAxes() []ConfigurationAxis {
- keys := make([]ConfigurationAxis, 0, len(sa.ConfigurableValues))
- for k := range sa.ConfigurableValues {
- keys = append(keys, k)
- }
-
- sort.Slice(keys, func(i, j int) bool { return keys[i].less(keys[j]) })
- return keys
+ return SortedConfigurationAxes(sa.ConfigurableValues)
}
// Collapse reduces the configurable axes of the string attribute to a single axis.
@@ -1353,13 +1339,7 @@
// SortedConfigurationAxes returns all the used ConfigurationAxis in sorted order.
func (sla *StringListAttribute) SortedConfigurationAxes() []ConfigurationAxis {
- keys := make([]ConfigurationAxis, 0, len(sla.ConfigurableValues))
- for k := range sla.ConfigurableValues {
- keys = append(keys, k)
- }
-
- sort.Slice(keys, func(i, j int) bool { return keys[i].less(keys[j]) })
- return keys
+ return SortedConfigurationAxes(sla.ConfigurableValues)
}
// DeduplicateAxesFromBase ensures no duplication of items between the no-configuration value and
diff --git a/bp2build/configurability.go b/bp2build/configurability.go
index 4244956..2a0a78e 100644
--- a/bp2build/configurability.go
+++ b/bp2build/configurability.go
@@ -160,6 +160,12 @@
// select statements.
func prettyPrintAttribute(v bazel.Attribute, indent int) (string, error) {
var value reflect.Value
+ // configurableAttrs is the list of individual select statements to be
+ // concatenated together. These select statements should be along different
+ // axes. For example, one element may be
+ // `select({"//color:red": "one", "//color:green": "two"})`, and the second
+ // element may be `select({"//animal:cat": "three", "//animal:dog": "four"}).
+ // These selects should be sorted by axis identifier.
var configurableAttrs []selects
var prepend bool
var defaultSelectValue *string
diff --git a/cc/bp2build.go b/cc/bp2build.go
index aea1fa1..f788c96 100644
--- a/cc/bp2build.go
+++ b/cc/bp2build.go
@@ -702,7 +702,13 @@
compilerAttrs := compilerAttributes{}
linkerAttrs := linkerAttributes{}
- for axis, configs := range axisToConfigs {
+ // Iterate through these axes in a deterministic order. This is required
+ // because processing certain dependencies may result in concatenating
+ // elements along other axes. (For example, processing NoConfig may result
+ // in elements being added to InApex). This is thus the only way to ensure
+ // that the order of entries in each list is in a predictable order.
+ for _, axis := range bazel.SortedConfigurationAxes(axisToConfigs) {
+ configs := axisToConfigs[axis]
for cfg := range configs {
var allHdrs []string
if baseCompilerProps, ok := archVariantCompilerProps[axis][cfg].(*BaseCompilerProperties); ok {