Merge "Revert "update MultiAbiTargeting matching logic"" into tm-qpr-dev

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
 }