From b48d57bdcdcae3c3eb0b823bdf952d74e5502412 Mon Sep 17 00:00:00 2001 From: Sam Delmerico Date: Tue, 22 Nov 2022 17:47:59 -0500 Subject: [PATCH] extract_apks matches APKs with >= 1 matching ABI Prior to this change, the bundletool and extract_apks tools require that all ABIs that an APEX or APK provides must be compatible with the TargetConfig. Instead, this change allows an APK to be selected if it has at least one compatible ABI with the TargetConfig. Bug: 260115309 Test: go test . Change-Id: If67ce8128099611257a834862295a2bf5fa427d3 --- cmd/extract_apks/main.go | 100 +++++++++++++++++++--------------- cmd/extract_apks/main_test.go | 6 +- 2 files changed, 62 insertions(+), 44 deletions(-) diff --git a/cmd/extract_apks/main.go b/cmd/extract_apks/main.go index c420567e4..82db634d0 100644 --- a/cmd/extract_apks/main.go +++ b/cmd/extract_apks/main.go @@ -132,21 +132,21 @@ type apkDescriptionMatcher struct { *android_bundle_proto.ApkDescription } -func (m apkDescriptionMatcher) matches(config TargetConfig) bool { - return m.ApkDescription == nil || (apkTargetingMatcher{m.Targeting}).matches(config) +func (m apkDescriptionMatcher) matches(config TargetConfig, allAbisMustMatch bool) bool { + return m.ApkDescription == nil || (apkTargetingMatcher{m.Targeting}).matches(config, allAbisMustMatch) } type apkTargetingMatcher struct { *android_bundle_proto.ApkTargeting } -func (m apkTargetingMatcher) matches(config TargetConfig) bool { +func (m apkTargetingMatcher) matches(config TargetConfig, allAbisMustMatch bool) bool { return m.ApkTargeting == nil || (abiTargetingMatcher{m.AbiTargeting}.matches(config) && languageTargetingMatcher{m.LanguageTargeting}.matches(config) && screenDensityTargetingMatcher{m.ScreenDensityTargeting}.matches(config) && sdkVersionTargetingMatcher{m.SdkVersionTargeting}.matches(config) && - multiAbiTargetingMatcher{m.MultiAbiTargeting}.matches(config)) + multiAbiTargetingMatcher{m.MultiAbiTargeting}.matches(config, allAbisMustMatch)) } type languageTargetingMatcher struct { @@ -215,33 +215,27 @@ func (m multiAbiValue) compare(other multiAbiValue) int { } } - m = append(multiAbiValue{}, m...) - sort.Slice(m, sortAbis(m)) - other = append(multiAbiValue{}, other...) - sort.Slice(other, sortAbis(other)) + sortedM := append(multiAbiValue{}, m...) + sort.Slice(sortedM, sortAbis(sortedM)) + sortedOther := append(multiAbiValue{}, other...) + sort.Slice(sortedOther, sortAbis(sortedOther)) - for i := 0; i < min(len(m), len(other)); i++ { - if multiAbiPriorities[m[i].Alias] > multiAbiPriorities[other[i].Alias] { + for i := 0; i < min(len(sortedM), len(sortedOther)); i++ { + if multiAbiPriorities[sortedM[i].Alias] > multiAbiPriorities[sortedOther[i].Alias] { return 1 } - if multiAbiPriorities[m[i].Alias] < multiAbiPriorities[other[i].Alias] { + if multiAbiPriorities[sortedM[i].Alias] < multiAbiPriorities[sortedOther[i].Alias] { return -1 } } - if len(m) == len(other) { - return 0 - } - if len(m) > len(other) { - return 1 - } - return -1 + return len(sortedM) - len(sortedOther) } // 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 { +func (t multiAbiTargetingMatcher) matches(config TargetConfig, allAbisMustMatch bool) bool { if t.MultiAbiTargeting == nil { return true } @@ -250,12 +244,19 @@ func (t multiAbiTargetingMatcher) matches(config TargetConfig) bool { } multiAbiIsValid := func(m multiAbiValue) bool { + numValid := 0 for _, abi := range m { - if _, ok := config.abis[abi.Alias]; !ok { - return false + if _, ok := config.abis[abi.Alias]; ok { + numValid += 1 } } - return true + if numValid == 0 { + return false + } else if numValid > 0 && !allAbisMustMatch { + return true + } else { + return numValid == len(m) + } } // ensure that the current value is valid for our config @@ -264,6 +265,7 @@ func (t multiAbiTargetingMatcher) matches(config TargetConfig) bool { for _, multiAbi := range multiAbiSet { if multiAbiIsValid(multiAbi.GetAbi()) { valueSetContainsViableAbi = true + break } } @@ -362,13 +364,13 @@ type variantTargetingMatcher struct { *android_bundle_proto.VariantTargeting } -func (m variantTargetingMatcher) matches(config TargetConfig) bool { +func (m variantTargetingMatcher) matches(config TargetConfig, allAbisMustMatch bool) bool { if m.VariantTargeting == nil { return true } return sdkVersionTargetingMatcher{m.SdkVersionTargeting}.matches(config) && abiTargetingMatcher{m.AbiTargeting}.matches(config) && - multiAbiTargetingMatcher{m.MultiAbiTargeting}.matches(config) && + multiAbiTargetingMatcher{m.MultiAbiTargeting}.matches(config, allAbisMustMatch) && screenDensityTargetingMatcher{m.ScreenDensityTargeting}.matches(config) && textureCompressionFormatTargetingMatcher{m.TextureCompressionFormatTargeting}.matches(config) } @@ -380,30 +382,42 @@ type SelectionResult struct { // Return all entries matching target configuration func selectApks(toc Toc, targetConfig TargetConfig) SelectionResult { - var result SelectionResult - for _, variant := range (*toc).GetVariant() { - if !(variantTargetingMatcher{variant.GetTargeting()}.matches(targetConfig)) { - continue - } - for _, as := range variant.GetApkSet() { - if !(moduleMetadataMatcher{as.ModuleMetadata}.matches(targetConfig)) { + checkMatching := func(allAbisMustMatch bool) SelectionResult { + var result SelectionResult + for _, variant := range (*toc).GetVariant() { + if !(variantTargetingMatcher{variant.GetTargeting()}.matches(targetConfig, allAbisMustMatch)) { continue } - for _, apkdesc := range as.GetApkDescription() { - if (apkDescriptionMatcher{apkdesc}).matches(targetConfig) { - result.entries = append(result.entries, apkdesc.GetPath()) - // TODO(asmundak): As it turns out, moduleName which we get from - // the ModuleMetadata matches the module names of the generated - // entry paths just by coincidence, only for the split APKs. We - // need to discuss this with bundletool folks. - result.moduleName = as.GetModuleMetadata().GetName() + for _, as := range variant.GetApkSet() { + if !(moduleMetadataMatcher{as.ModuleMetadata}.matches(targetConfig)) { + continue + } + for _, apkdesc := range as.GetApkDescription() { + if (apkDescriptionMatcher{apkdesc}).matches(targetConfig, allAbisMustMatch) { + result.entries = append(result.entries, apkdesc.GetPath()) + // TODO(asmundak): As it turns out, moduleName which we get from + // the ModuleMetadata matches the module names of the generated + // entry paths just by coincidence, only for the split APKs. We + // need to discuss this with bundletool folks. + result.moduleName = as.GetModuleMetadata().GetName() + } + } + // we allow only a single module, so bail out here if we found one + if result.moduleName != "" { + return result } } - // we allow only a single module, so bail out here if we found one - if result.moduleName != "" { - return result - } } + return result + } + result := checkMatching(true) + if result.moduleName == "" { + // if there are no matches where all of the ABIs are available in the + // TargetConfig, then search again with a looser requirement of at + // least one matching ABI + // NOTE(b/260130686): this logic diverges from the logic in bundletool + // https://github.com/google/bundletool/blob/ae0fc0162fd80d92ef8f4ef4527c066f0106942f/src/main/java/com/android/tools/build/bundletool/device/MultiAbiMatcher.java#L43 + result = checkMatching(false) } return result } diff --git a/cmd/extract_apks/main_test.go b/cmd/extract_apks/main_test.go index c1d712df4..9f52877c9 100644 --- a/cmd/extract_apks/main_test.go +++ b/cmd/extract_apks/main_test.go @@ -744,7 +744,11 @@ variant { bp.Abi_X86_64: 0, }, }, - expected: SelectionResult{}, + expected: SelectionResult{ + "base", + []string{ + "standalones/standalone-x86.x86_64.apex", + }}, }, { name: "multi-variant multi-target cross-target",