Revert^3 "Revert "update MultiAbiTargeting matching logic""

1cc8ed5dee39e5a4c1def3e0304c28d284fb1368

Bug: 263438687
Change-Id: I7d8585c97ca1e9a960d9725135f99579b0ae5762
Merged-In: Ic3b067e98a65146cfa399e7c9b231f397e51c23e

2 files changed, 438 insertions, 16 deletions

diff --git a/cmd/extract_apks/main.go b/cmd/extract_apks/main.go
index 1cf64de8b..e39f8d765 100644
--- a/cmd/extract_apks/main.go
+++ b/cmd/extract_apks/main.go
@@ -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
+	}
+
+	// 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 highestPriority == 0 {
+
+	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
 }
 
diff --git a/cmd/extract_apks/main_test.go b/cmd/extract_apks/main_test.go
index f5e40466a..c1d712df4 100644
--- a/cmd/extract_apks/main_test.go
+++ b/cmd/extract_apks/main_test.go
@@ -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
 }