From 6c5fae512be51e9d13390b21b7def412ee718b0e Mon Sep 17 00:00:00 2001 From: Motomu Utsumi Date: Wed, 19 Oct 2022 01:50:10 +0000 Subject: Revert "update MultiAbiTargeting matching logic" This reverts commit 9c94a64e4b5905ee73b0af0a13a9ac7f619fcf31. Reason for revert: b/254332635 build falure Change-Id: I4ed1f59fd28ac7bb60939a10d15311cb7a5bfa42 --- cmd/extract_apks/main.go | 90 ++--------- cmd/extract_apks/main_test.go | 364 ------------------------------------------ 2 files changed, 16 insertions(+), 438 deletions(-) diff --git a/cmd/extract_apks/main.go b/cmd/extract_apks/main.go index e39f8d765..1cf64de8b 100644 --- a/cmd/extract_apks/main.go +++ b/cmd/extract_apks/main.go @@ -29,7 +29,6 @@ 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" ) @@ -198,49 +197,6 @@ 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 @@ -248,45 +204,31 @@ func (t multiAbiTargetingMatcher) matches(config TargetConfig) bool { if _, ok := config.abis[android_bundle_proto.Abi_UNSPECIFIED_CPU_ARCHITECTURE]; ok { return true } - - multiAbiIsValid := func(m multiAbiValue) bool { - for _, abi := range m { - if _, ok := config.abis[abi.Alias]; !ok { - return false + // 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] + } } } - return true - } - - // 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 { + if highestPriority == 0 { return false } - // See if there are any matching alternatives with a higher priority. - 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 + 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 + } } } } - return true } diff --git a/cmd/extract_apks/main_test.go b/cmd/extract_apks/main_test.go index c1d712df4..f5e40466a 100644 --- a/cmd/extract_apks/main_test.go +++ b/cmd/extract_apks/main_test.go @@ -420,370 +420,6 @@ 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 } -- cgit v1.2.3