Remove sort from mergeApexVariations

Remove the sort from mergeApexVariations, and instead sort before
calling it as sorting will break the next change that calls
mergeApexVariations on a provider field that must not be modified.

Also remove the unused ctx PathContext parameter, and use
slices.SortFunc.

Bug: 319288033
Test: Test_mergeApexVariations
Change-Id: I4a044e86a8eb262b54af50afe14c678616c499d1
This commit is contained in:
Colin Cross 2024-04-15 15:32:38 -07:00
parent af333f5aed
commit 9132ced405
2 changed files with 191 additions and 50 deletions

View file

@ -16,6 +16,7 @@ package android
import (
"fmt"
"slices"
"sort"
"strconv"
"strings"
@ -108,7 +109,7 @@ func (i ApexInfo) AddJSONData(d *map[string]interface{}) {
// are configured to have the same alias variation named apex29. Whether platform APIs is allowed
// or not also matters; if two APEXes don't have the same allowance, they get different names and
// thus wouldn't be merged.
func (i ApexInfo) mergedName(ctx PathContext) string {
func (i ApexInfo) mergedName() string {
name := "apex" + strconv.Itoa(i.MinSdkVersion.FinalOrFutureInt())
return name
}
@ -543,17 +544,10 @@ func AvailableToSameApexes(mod1, mod2 ApexModule) bool {
return true
}
type byApexName []ApexInfo
func (a byApexName) Len() int { return len(a) }
func (a byApexName) Swap(i, j int) { a[i], a[j] = a[j], a[i] }
func (a byApexName) Less(i, j int) bool { return a[i].ApexVariationName < a[j].ApexVariationName }
// mergeApexVariations deduplicates apex variations that would build identically into a common
// variation. It returns the reduced list of variations and a list of aliases from the original
// variation names to the new variation names.
func mergeApexVariations(ctx PathContext, apexInfos []ApexInfo) (merged []ApexInfo, aliases [][2]string) {
sort.Sort(byApexName(apexInfos))
func mergeApexVariations(apexInfos []ApexInfo) (merged []ApexInfo, aliases [][2]string) {
seen := make(map[string]int)
for _, apexInfo := range apexInfos {
// If this is for a prebuilt apex then use the actual name of the apex variation to prevent this
@ -567,7 +561,7 @@ func mergeApexVariations(ctx PathContext, apexInfos []ApexInfo) (merged []ApexIn
// this one into it, otherwise create a new merged ApexInfo from this one and save it away so
// other ApexInfo instances can be merged into it.
variantName := apexInfo.ApexVariationName
mergedName := apexInfo.mergedName(ctx)
mergedName := apexInfo.mergedName()
if index, exists := seen[mergedName]; exists {
// Variants having the same mergedName are deduped
merged[index].InApexVariants = append(merged[index].InApexVariants, variantName)
@ -606,18 +600,20 @@ func CreateApexVariations(mctx BottomUpMutatorContext, module ApexModule) []Modu
// TODO(jiyong): is this the right place?
base.checkApexAvailableProperty(mctx)
var apexInfos []ApexInfo
var aliases [][2]string
if !mctx.Module().(ApexModule).UniqueApexVariations() && !base.ApexProperties.UniqueApexVariationsForDeps {
apexInfos, aliases = mergeApexVariations(mctx, base.apexInfos)
} else {
apexInfos = base.apexInfos
}
apexInfos := base.apexInfos
// base.apexInfos is only needed to propagate the list of apexes from apexInfoMutator to
// apexMutator. It is no longer accurate after mergeApexVariations, and won't be copied to
// all but the first created variant. Clear it so it doesn't accidentally get used later.
base.apexInfos = nil
sort.Sort(byApexName(apexInfos))
slices.SortFunc(apexInfos, func(a, b ApexInfo) int {
return strings.Compare(a.ApexVariationName, b.ApexVariationName)
})
var aliases [][2]string
if !mctx.Module().(ApexModule).UniqueApexVariations() && !base.ApexProperties.UniqueApexVariationsForDeps {
apexInfos, aliases = mergeApexVariations(apexInfos)
}
var inApex ApexMembership
for _, a := range apexInfos {

View file

@ -33,10 +33,22 @@ func Test_mergeApexVariations(t *testing.T) {
{
name: "single",
in: []ApexInfo{
{"foo", FutureApiLevel, false, false, []string{"foo"}, []string{"foo"}, nil, NotForPrebuiltApex, nil},
{
ApexVariationName: "foo",
MinSdkVersion: FutureApiLevel,
InApexVariants: []string{"foo"},
InApexModules: []string{"foo"},
ForPrebuiltApex: NotForPrebuiltApex,
},
},
wantMerged: []ApexInfo{
{"apex10000", FutureApiLevel, false, false, []string{"foo"}, []string{"foo"}, nil, NotForPrebuiltApex, nil},
{
ApexVariationName: "apex10000",
MinSdkVersion: FutureApiLevel,
InApexVariants: []string{"foo"},
InApexModules: []string{"foo"},
ForPrebuiltApex: NotForPrebuiltApex,
},
},
wantAliases: [][2]string{
{"foo", "apex10000"},
@ -45,98 +57,231 @@ func Test_mergeApexVariations(t *testing.T) {
{
name: "merge",
in: []ApexInfo{
{"foo", FutureApiLevel, false, false, []string{"foo"}, []string{"foo"}, nil, NotForPrebuiltApex, nil},
{"bar", FutureApiLevel, false, false, []string{"bar"}, []string{"bar"}, nil, NotForPrebuiltApex, nil},
{
ApexVariationName: "foo",
MinSdkVersion: FutureApiLevel,
InApexVariants: []string{"foo"},
InApexModules: []string{"foo"},
ForPrebuiltApex: NotForPrebuiltApex,
},
{
ApexVariationName: "bar",
MinSdkVersion: FutureApiLevel,
InApexVariants: []string{"bar"},
InApexModules: []string{"bar"},
ForPrebuiltApex: NotForPrebuiltApex,
},
},
wantMerged: []ApexInfo{
{"apex10000", FutureApiLevel, false, false, []string{"bar", "foo"}, []string{"bar", "foo"}, nil, false, nil}},
{
ApexVariationName: "apex10000",
MinSdkVersion: FutureApiLevel,
InApexVariants: []string{"foo", "bar"},
InApexModules: []string{"foo", "bar"},
}},
wantAliases: [][2]string{
{"bar", "apex10000"},
{"foo", "apex10000"},
{"bar", "apex10000"},
},
},
{
name: "don't merge version",
in: []ApexInfo{
{"foo", FutureApiLevel, false, false, []string{"foo"}, []string{"foo"}, nil, NotForPrebuiltApex, nil},
{"bar", uncheckedFinalApiLevel(30), false, false, []string{"bar"}, []string{"bar"}, nil, NotForPrebuiltApex, nil},
{
ApexVariationName: "foo",
MinSdkVersion: FutureApiLevel,
InApexVariants: []string{"foo"},
InApexModules: []string{"foo"},
ForPrebuiltApex: NotForPrebuiltApex,
},
{
ApexVariationName: "bar",
MinSdkVersion: uncheckedFinalApiLevel(30),
InApexVariants: []string{"bar"},
InApexModules: []string{"bar"},
ForPrebuiltApex: NotForPrebuiltApex,
},
},
wantMerged: []ApexInfo{
{"apex30", uncheckedFinalApiLevel(30), false, false, []string{"bar"}, []string{"bar"}, nil, NotForPrebuiltApex, nil},
{"apex10000", FutureApiLevel, false, false, []string{"foo"}, []string{"foo"}, nil, NotForPrebuiltApex, nil},
{
ApexVariationName: "apex10000",
MinSdkVersion: FutureApiLevel,
InApexVariants: []string{"foo"},
InApexModules: []string{"foo"},
ForPrebuiltApex: NotForPrebuiltApex,
},
{
ApexVariationName: "apex30",
MinSdkVersion: uncheckedFinalApiLevel(30),
InApexVariants: []string{"bar"},
InApexModules: []string{"bar"},
ForPrebuiltApex: NotForPrebuiltApex,
},
},
wantAliases: [][2]string{
{"bar", "apex30"},
{"foo", "apex10000"},
{"bar", "apex30"},
},
},
{
name: "merge updatable",
in: []ApexInfo{
{"foo", FutureApiLevel, false, false, []string{"foo"}, []string{"foo"}, nil, NotForPrebuiltApex, nil},
{"bar", FutureApiLevel, true, false, []string{"bar"}, []string{"bar"}, nil, NotForPrebuiltApex, nil},
{
ApexVariationName: "foo",
MinSdkVersion: FutureApiLevel,
InApexVariants: []string{"foo"},
InApexModules: []string{"foo"},
ForPrebuiltApex: NotForPrebuiltApex,
},
{
ApexVariationName: "bar",
MinSdkVersion: FutureApiLevel,
Updatable: true,
InApexVariants: []string{"bar"},
InApexModules: []string{"bar"},
ForPrebuiltApex: NotForPrebuiltApex,
},
},
wantMerged: []ApexInfo{
{"apex10000", FutureApiLevel, true, false, []string{"bar", "foo"}, []string{"bar", "foo"}, nil, NotForPrebuiltApex, nil},
{
ApexVariationName: "apex10000",
MinSdkVersion: FutureApiLevel,
Updatable: true,
InApexVariants: []string{"foo", "bar"},
InApexModules: []string{"foo", "bar"},
ForPrebuiltApex: NotForPrebuiltApex,
},
},
wantAliases: [][2]string{
{"bar", "apex10000"},
{"foo", "apex10000"},
{"bar", "apex10000"},
},
},
{
name: "don't merge when for prebuilt_apex",
in: []ApexInfo{
{"foo", FutureApiLevel, false, false, []string{"foo"}, []string{"foo"}, nil, NotForPrebuiltApex, nil},
{"bar", FutureApiLevel, true, false, []string{"bar"}, []string{"bar"}, nil, NotForPrebuiltApex, nil},
{
ApexVariationName: "foo",
MinSdkVersion: FutureApiLevel,
InApexVariants: []string{"foo"},
InApexModules: []string{"foo"},
ForPrebuiltApex: NotForPrebuiltApex,
},
{
ApexVariationName: "bar",
MinSdkVersion: FutureApiLevel,
Updatable: true,
InApexVariants: []string{"bar"},
InApexModules: []string{"bar"},
ForPrebuiltApex: NotForPrebuiltApex,
},
// This one should not be merged in with the others because it is for
// a prebuilt_apex.
{"baz", FutureApiLevel, true, false, []string{"baz"}, []string{"baz"}, nil, ForPrebuiltApex, nil},
{
ApexVariationName: "baz",
MinSdkVersion: FutureApiLevel,
Updatable: true,
InApexVariants: []string{"baz"},
InApexModules: []string{"baz"},
ForPrebuiltApex: ForPrebuiltApex,
},
},
wantMerged: []ApexInfo{
{"apex10000", FutureApiLevel, true, false, []string{"bar", "foo"}, []string{"bar", "foo"}, nil, NotForPrebuiltApex, nil},
{"baz", FutureApiLevel, true, false, []string{"baz"}, []string{"baz"}, nil, ForPrebuiltApex, nil},
{
ApexVariationName: "apex10000",
MinSdkVersion: FutureApiLevel,
Updatable: true,
InApexVariants: []string{"foo", "bar"},
InApexModules: []string{"foo", "bar"},
ForPrebuiltApex: NotForPrebuiltApex,
},
{
ApexVariationName: "baz",
MinSdkVersion: FutureApiLevel,
Updatable: true,
InApexVariants: []string{"baz"},
InApexModules: []string{"baz"},
ForPrebuiltApex: ForPrebuiltApex,
},
},
wantAliases: [][2]string{
{"bar", "apex10000"},
{"foo", "apex10000"},
{"bar", "apex10000"},
},
},
{
name: "merge different UsePlatformApis but don't allow using platform api",
in: []ApexInfo{
{"foo", FutureApiLevel, false, false, []string{"foo"}, []string{"foo"}, nil, NotForPrebuiltApex, nil},
{"bar", FutureApiLevel, false, true, []string{"bar"}, []string{"bar"}, nil, NotForPrebuiltApex, nil},
{
ApexVariationName: "foo",
MinSdkVersion: FutureApiLevel,
InApexVariants: []string{"foo"},
InApexModules: []string{"foo"},
ForPrebuiltApex: NotForPrebuiltApex,
},
{
ApexVariationName: "bar",
MinSdkVersion: FutureApiLevel,
UsePlatformApis: true,
InApexVariants: []string{"bar"},
InApexModules: []string{"bar"},
ForPrebuiltApex: NotForPrebuiltApex,
},
},
wantMerged: []ApexInfo{
{"apex10000", FutureApiLevel, false, false, []string{"bar", "foo"}, []string{"bar", "foo"}, nil, NotForPrebuiltApex, nil},
{
ApexVariationName: "apex10000",
MinSdkVersion: FutureApiLevel,
InApexVariants: []string{"foo", "bar"},
InApexModules: []string{"foo", "bar"},
ForPrebuiltApex: NotForPrebuiltApex,
},
},
wantAliases: [][2]string{
{"bar", "apex10000"},
{"foo", "apex10000"},
{"bar", "apex10000"},
},
},
{
name: "merge same UsePlatformApis and allow using platform api",
in: []ApexInfo{
{"foo", FutureApiLevel, false, true, []string{"foo"}, []string{"foo"}, nil, NotForPrebuiltApex, nil},
{"bar", FutureApiLevel, false, true, []string{"bar"}, []string{"bar"}, nil, NotForPrebuiltApex, nil},
{
ApexVariationName: "foo",
MinSdkVersion: FutureApiLevel,
UsePlatformApis: true,
InApexVariants: []string{"foo"},
InApexModules: []string{"foo"},
ForPrebuiltApex: NotForPrebuiltApex,
},
{
ApexVariationName: "bar",
MinSdkVersion: FutureApiLevel,
UsePlatformApis: true,
InApexVariants: []string{"bar"},
InApexModules: []string{"bar"},
ForPrebuiltApex: NotForPrebuiltApex,
},
},
wantMerged: []ApexInfo{
{"apex10000", FutureApiLevel, false, true, []string{"bar", "foo"}, []string{"bar", "foo"}, nil, NotForPrebuiltApex, nil},
{
ApexVariationName: "apex10000",
MinSdkVersion: FutureApiLevel,
UsePlatformApis: true,
InApexVariants: []string{"foo", "bar"},
InApexModules: []string{"foo", "bar"},
ForPrebuiltApex: NotForPrebuiltApex,
},
},
wantAliases: [][2]string{
{"bar", "apex10000"},
{"foo", "apex10000"},
{"bar", "apex10000"},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
config := TestConfig(t.TempDir(), nil, "", nil)
ctx := &configErrorWrapper{config: config}
gotMerged, gotAliases := mergeApexVariations(ctx, tt.in)
gotMerged, gotAliases := mergeApexVariations(tt.in)
if !reflect.DeepEqual(gotMerged, tt.wantMerged) {
t.Errorf("mergeApexVariations() gotMerged = %v, want %v", gotMerged, tt.wantMerged)
}