Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9c94a64e authored by Sam Delmerico's avatar Sam Delmerico Committed by satayev
Browse files

update MultiAbiTargeting matching logic 

The logic here has diverged from the logic in bundletool and resulted in
the wrong APEX variant being chosen for a 64bit-only product.

Bug: 246476965
Test: go test .
Change-Id: Ic3b067e98a65146cfa399e7c9b231f397e51c23e
Merged-In: Ic3b067e98a65146cfa399e7c9b231f397e51c23e
parent 175515bf

cmd/extract_apks/main.go

+74 −16
Original line number Diff line number Diff line
@@ -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

}

cmd/extract_apks/main_test.go

+364 −0
Original line number Diff line number Diff line
@@ -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

}