diff --git a/android/apex.go b/android/apex.go index 87599051a..c0acada13 100644 --- a/android/apex.go +++ b/android/apex.go @@ -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 { diff --git a/android/apex_test.go b/android/apex_test.go index ddc730d05..347bf7d98 100644 --- a/android/apex_test.go +++ b/android/apex_test.go @@ -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) }