Revert "Revert "update MultiAbiTargeting matching logic""

This reverts commit 6c5fae512b.

Reason for revert: re-submitting in a topic

Fix: b/246476965
Change-Id: I442e34d035da867ba36462f8e714c4d3c655af2f
Merged-In: Ic3b067e98a65146cfa399e7c9b231f397e51c23e
This commit is contained in:
Artur Satayev 2022-10-19 10:16:23 +00:00 committed by Paul Duffin
parent 6c5fae512b
commit be8bf85b03
2 changed files with 438 additions and 16 deletions

View file

@ -29,6 +29,7 @@ import (
"google.golang.org/protobuf/proto"
"android/soong/cmd/extract_apks/bundle_proto"
android_bundle_proto "android/soong/cmd/extract_apks/bundle_proto"
"android/soong/third_party/zip"
)
@ -197,6 +198,49 @@ type multiAbiTargetingMatcher struct {
*android_bundle_proto.MultiAbiTargeting
}
type multiAbiValue []*bundle_proto.Abi
func (m multiAbiValue) compare(other multiAbiValue) int {
min := func(a, b int) int {
if a < b {
return a
}
return b
}
sortAbis := func(abiSlice multiAbiValue) func(i, j int) bool {
return func(i, j int) bool {
// sort priorities greatest to least
return multiAbiPriorities[abiSlice[i].Alias] > multiAbiPriorities[abiSlice[j].Alias]
}
}
m = append(multiAbiValue{}, m...)
sort.Slice(m, sortAbis(m))
other = append(multiAbiValue{}, other...)
sort.Slice(other, sortAbis(other))
for i := 0; i < min(len(m), len(other)); i++ {
if multiAbiPriorities[m[i].Alias] > multiAbiPriorities[other[i].Alias] {
return 1
}
if multiAbiPriorities[m[i].Alias] < multiAbiPriorities[other[i].Alias] {
return -1
}
}
if len(m) == len(other) {
return 0
}
if len(m) > len(other) {
return 1
}
return -1
}
// this logic should match the logic in bundletool at
// https://github.com/google/bundletool/blob/ae0fc0162fd80d92ef8f4ef4527c066f0106942f/src/main/java/com/android/tools/build/bundletool/device/MultiAbiMatcher.java#L43
// (note link is the commit at time of writing; but logic should always match the latest)
func (t multiAbiTargetingMatcher) matches(config TargetConfig) bool {
if t.MultiAbiTargeting == nil {
return true
@ -204,31 +248,45 @@ func (t multiAbiTargetingMatcher) matches(config TargetConfig) bool {
if _, ok := config.abis[android_bundle_proto.Abi_UNSPECIFIED_CPU_ARCHITECTURE]; ok {
return true
}
// Find the one with the highest priority.
highestPriority := 0
for _, v := range t.GetValue() {
for _, a := range v.GetAbi() {
if _, ok := config.abis[a.Alias]; ok {
if highestPriority < multiAbiPriorities[a.Alias] {
highestPriority = multiAbiPriorities[a.Alias]
}
multiAbiIsValid := func(m multiAbiValue) bool {
for _, abi := range m {
if _, ok := config.abis[abi.Alias]; !ok {
return false
}
}
return true
}
if highestPriority == 0 {
// ensure that the current value is valid for our config
valueSetContainsViableAbi := false
multiAbiSet := t.GetValue()
for _, multiAbi := range multiAbiSet {
if multiAbiIsValid(multiAbi.GetAbi()) {
valueSetContainsViableAbi = true
}
}
if !valueSetContainsViableAbi {
return false
}
// See if there are any matching alternatives with a higher priority.
for _, v := range t.GetAlternatives() {
for _, a := range v.GetAbi() {
if _, ok := config.abis[a.Alias]; ok {
if highestPriority < multiAbiPriorities[a.Alias] {
// There's a better one. Skip this one.
return false
}
for _, altMultiAbi := range t.GetAlternatives() {
if !multiAbiIsValid(altMultiAbi.GetAbi()) {
continue
}
for _, multiAbi := range multiAbiSet {
valueAbis := multiAbiValue(multiAbi.GetAbi())
altAbis := multiAbiValue(altMultiAbi.GetAbi())
if valueAbis.compare(altAbis) < 0 {
// An alternative has a higher priority, don't use this one
return false
}
}
}
return true
}

View file

@ -420,6 +420,370 @@ bundletool {
}
}
func TestSelectApks_ApexSet_Variants(t *testing.T) {
testCases := []testDesc{
{
protoText: `
variant {
targeting {
sdk_version_targeting {value {min {value: 29}}}
multi_abi_targeting {
value {abi {alias: ARMEABI_V7A}}
alternatives {
abi {alias: ARMEABI_V7A}
abi {alias: ARM64_V8A}
}
alternatives {abi {alias: ARM64_V8A}}
alternatives {abi {alias: X86}}
alternatives {
abi {alias: X86}
abi {alias: X86_64}
}
}
}
apk_set {
module_metadata {
name: "base"
delivery_type: INSTALL_TIME
}
apk_description {
targeting {
multi_abi_targeting {
value {abi {alias: ARMEABI_V7A}}
alternatives {
abi {alias: ARMEABI_V7A}
abi {alias: ARM64_V8A}
}
alternatives {abi {alias: ARM64_V8A}}
alternatives {abi {alias: X86}}
alternatives {
abi {alias: X86}
abi {alias: X86_64}
}
}
}
path: "standalones/standalone-armeabi_v7a.apex"
}
}
variant_number: 0
}
variant {
targeting {
sdk_version_targeting {value {min {value: 29}}}
multi_abi_targeting {
value {abi {alias: ARM64_V8A}}
alternatives {abi {alias: ARMEABI_V7A}}
alternatives {
abi {alias: ARMEABI_V7A}
abi {alias: ARM64_V8A}
}
alternatives {abi {alias: X86}}
alternatives {
abi {alias: X86}
abi {alias: X86_64}
}
}
}
apk_set {
module_metadata {
name: "base"
delivery_type: INSTALL_TIME
}
apk_description {
targeting {
multi_abi_targeting {
value {abi {alias: ARM64_V8A}}
alternatives {abi {alias: ARMEABI_V7A}}
alternatives {
abi {alias: ARMEABI_V7A}
abi {alias: ARM64_V8A}
}
alternatives {abi {alias: X86}}
alternatives {
abi {alias: X86}
abi {alias: X86_64}
}
}
}
path: "standalones/standalone-arm64_v8a.apex"
}
}
variant_number: 1
}
variant {
targeting {
sdk_version_targeting {value {min {value: 29}}}
multi_abi_targeting {
value {
abi {alias: ARMEABI_V7A}
abi {alias: ARM64_V8A}
}
alternatives {abi {alias: ARMEABI_V7A}}
alternatives {abi {alias: ARM64_V8A}}
alternatives {abi {alias: X86}}
alternatives {
abi {alias: X86}
abi {alias: X86_64}
}
}
}
apk_set {
module_metadata {
name: "base"
delivery_type: INSTALL_TIME
}
apk_description {
targeting {
multi_abi_targeting {
value {
abi {alias: ARMEABI_V7A}
abi {alias: ARM64_V8A}
}
alternatives {abi {alias: ARMEABI_V7A}}
alternatives {abi {alias: ARM64_V8A}}
alternatives {abi {alias: X86}}
alternatives {
abi {alias: X86}
abi {alias: X86_64}
}
}
}
path: "standalones/standalone-armeabi_v7a.arm64_v8a.apex"
}
}
variant_number: 2
}
variant {
targeting {
sdk_version_targeting {value {min {value: 29}}}
multi_abi_targeting {
value {abi {alias: X86}}
alternatives {abi {alias: ARMEABI_V7A}}
alternatives {
abi {alias: ARMEABI_V7A}
abi {alias: ARM64_V8A}
}
alternatives {abi {alias: ARM64_V8A}}
alternatives {
abi {alias: X86}
abi {alias: X86_64}
}
}
}
apk_set {
module_metadata {
name: "base"
delivery_type: INSTALL_TIME
}
apk_description {
targeting {
multi_abi_targeting {
value {abi {alias: X86}}
alternatives {abi {alias: ARMEABI_V7A}}
alternatives {
abi {alias: ARMEABI_V7A}
abi {alias: ARM64_V8A}
}
alternatives {abi {alias: ARM64_V8A}}
alternatives {
abi {alias: X86}
abi {alias: X86_64}
}
}
}
path: "standalones/standalone-x86.apex"
}
}
variant_number: 3
}
variant {
targeting {
sdk_version_targeting {value {min {value: 29}}}
multi_abi_targeting {
value {
abi {alias: X86}
abi {alias: X86_64}
}
alternatives {abi {alias: ARMEABI_V7A}}
alternatives {
abi {alias: ARMEABI_V7A}
abi {alias: ARM64_V8A}
}
alternatives {abi {alias: ARM64_V8A}}
alternatives {abi {alias: X86}}
}
}
apk_set {
module_metadata {
name: "base"
delivery_type: INSTALL_TIME
}
apk_description {
targeting {
multi_abi_targeting {
value {
abi {alias: X86}
abi {alias: X86_64}
}
alternatives {abi {alias: ARMEABI_V7A}}
alternatives {
abi {alias: ARMEABI_V7A}
abi {alias: ARM64_V8A}
}
alternatives {abi {alias: ARM64_V8A}}
alternatives {abi {alias: X86}}
}
}
path: "standalones/standalone-x86.x86_64.apex"
}
}
variant_number: 4
}
`,
configs: []testConfigDesc{
{
name: "multi-variant multi-target ARM",
targetConfig: TargetConfig{
sdkVersion: 33,
screenDpi: map[bp.ScreenDensity_DensityAlias]bool{
bp.ScreenDensity_DENSITY_UNSPECIFIED: true,
},
abis: map[bp.Abi_AbiAlias]int{
bp.Abi_ARM64_V8A: 0,
bp.Abi_ARMEABI_V7A: 1,
},
},
expected: SelectionResult{
"base",
[]string{
"standalones/standalone-armeabi_v7a.arm64_v8a.apex",
},
},
},
{
name: "multi-variant single-target arm",
targetConfig: TargetConfig{
sdkVersion: 33,
screenDpi: map[bp.ScreenDensity_DensityAlias]bool{
bp.ScreenDensity_DENSITY_UNSPECIFIED: true,
},
abis: map[bp.Abi_AbiAlias]int{
bp.Abi_ARMEABI_V7A: 0,
},
},
expected: SelectionResult{
"base",
[]string{
"standalones/standalone-armeabi_v7a.apex",
},
},
},
{
name: "multi-variant single-target arm64",
targetConfig: TargetConfig{
sdkVersion: 33,
screenDpi: map[bp.ScreenDensity_DensityAlias]bool{
bp.ScreenDensity_DENSITY_UNSPECIFIED: true,
},
abis: map[bp.Abi_AbiAlias]int{
bp.Abi_ARM64_V8A: 0,
},
},
expected: SelectionResult{
"base",
[]string{
"standalones/standalone-arm64_v8a.apex",
},
},
},
{
name: "multi-variant multi-target x86",
targetConfig: TargetConfig{
sdkVersion: 33,
screenDpi: map[bp.ScreenDensity_DensityAlias]bool{
bp.ScreenDensity_DENSITY_UNSPECIFIED: true,
},
abis: map[bp.Abi_AbiAlias]int{
bp.Abi_X86: 0,
bp.Abi_X86_64: 1,
},
},
expected: SelectionResult{
"base",
[]string{
"standalones/standalone-x86.x86_64.apex",
},
},
},
{
name: "multi-variant single-target x86",
targetConfig: TargetConfig{
sdkVersion: 33,
screenDpi: map[bp.ScreenDensity_DensityAlias]bool{
bp.ScreenDensity_DENSITY_UNSPECIFIED: true,
},
abis: map[bp.Abi_AbiAlias]int{
bp.Abi_X86: 0,
},
},
expected: SelectionResult{
"base",
[]string{
"standalones/standalone-x86.apex",
},
},
},
{
name: "multi-variant single-target x86_64",
targetConfig: TargetConfig{
sdkVersion: 33,
screenDpi: map[bp.ScreenDensity_DensityAlias]bool{
bp.ScreenDensity_DENSITY_UNSPECIFIED: true,
},
abis: map[bp.Abi_AbiAlias]int{
bp.Abi_X86_64: 0,
},
},
expected: SelectionResult{},
},
{
name: "multi-variant multi-target cross-target",
targetConfig: TargetConfig{
sdkVersion: 33,
screenDpi: map[bp.ScreenDensity_DensityAlias]bool{
bp.ScreenDensity_DENSITY_UNSPECIFIED: true,
},
abis: map[bp.Abi_AbiAlias]int{
bp.Abi_ARM64_V8A: 0,
bp.Abi_X86_64: 1,
},
},
expected: SelectionResult{
"base",
[]string{
"standalones/standalone-arm64_v8a.apex",
},
},
},
},
},
}
for _, testCase := range testCases {
var toc bp.BuildApksResult
if err := prototext.Unmarshal([]byte(testCase.protoText), &toc); err != nil {
t.Fatal(err)
}
for _, config := range testCase.configs {
t.Run(config.name, func(t *testing.T) {
actual := selectApks(&toc, config.targetConfig)
if !reflect.DeepEqual(config.expected, actual) {
t.Errorf("expected %v, got %v", config.expected, actual)
}
})
}
}
}
type testZip2ZipWriter struct {
entries map[string]string
}