From 2cd2f9ad8969ef817f8606fc5cc8988f8e706dba Mon Sep 17 00:00:00 2001 From: Jooyung Han Date: Mon, 6 Feb 2023 18:29:08 +0900 Subject: [PATCH] VNDK APEX doesn't use "apex_name" property VNDK APEX has been using "apex_name" property to set "name" field in apex_manifest. But setting apex_name had a side-effect of emitting duplicate rules for symbol files for VNDK APEX and its override_apex. (Please see the removed comments for more details about error) Not using "apex_name" fixes the duplicate errors. There're a few more vendor apexes with the same issue, which I will handle in follow-ups. Bug: 267581665 Test: m (soong test) && boot Change-Id: I00f617cef8af4d21880d4e8b9c0b9ab9322ba15c --- apex/androidmk.go | 11 +---------- apex/apex_test.go | 11 +++-------- apex/builder.go | 17 +++++------------ apex/vndk.go | 4 ---- 4 files changed, 9 insertions(+), 34 deletions(-) diff --git a/apex/androidmk.go b/apex/androidmk.go index 12faf2242..b830dcf4b 100644 --- a/apex/androidmk.go +++ b/apex/androidmk.go @@ -95,15 +95,6 @@ func (a *apexBundle) androidMkForFiles(w io.Writer, apexBundleName, apexName, mo return moduleNames } - // b/140136207. When there are overriding APEXes for a VNDK APEX, the symbols file for the overridden - // APEX and the overriding APEX will have the same installation paths at /apex/com.android.vndk.v - // as their apexName will be the same. To avoid the path conflicts, skip installing the symbol files - // for the overriding VNDK APEXes. - symbolFilesNotNeeded := a.vndkApex && len(a.overridableProperties.Overrides) > 0 - if symbolFilesNotNeeded && apexType != flattenedApex { - return moduleNames - } - // Avoid creating duplicate build rules for multi-installed APEXes. if proptools.BoolDefault(a.properties.Multi_install_skip_symbol_files, false) { return moduleNames @@ -152,7 +143,7 @@ func (a *apexBundle) androidMkForFiles(w io.Writer, apexBundleName, apexName, mo // /system/apex//{lib|framework|...} modulePath = filepath.Join(a.installDir.String(), apexBundleName, fi.installDir) fmt.Fprintln(w, "LOCAL_MODULE_PATH :=", modulePath) - if a.primaryApexType && !symbolFilesNotNeeded { + if a.primaryApexType { fmt.Fprintln(w, "LOCAL_SOONG_SYMBOL_PATH :=", pathWhenActivated) } android.AndroidMkEmitAssignList(w, "LOCAL_MODULE_SYMLINKS", fi.symlinks) diff --git a/apex/apex_test.go b/apex/apex_test.go index 4f300e7a2..e558fee03 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -3806,11 +3806,9 @@ func TestVndkApexNameRule(t *testing.T) { }`+vndkLibrariesTxtFiles("28", "current")) assertApexName := func(expected, moduleName string) { - bundle := ctx.ModuleForTests(moduleName, "android_common_image").Module().(*apexBundle) - actual := proptools.String(bundle.properties.Apex_name) - if !reflect.DeepEqual(actual, expected) { - t.Errorf("Got '%v', expected '%v'", actual, expected) - } + module := ctx.ModuleForTests(moduleName, "android_common_image") + apexManifestRule := module.Rule("apexManifestRule") + ensureContains(t, apexManifestRule.Args["opt"], "-v name "+expected) } assertApexName("com.android.vndk.v29", "com.android.vndk.current") @@ -4136,9 +4134,6 @@ func TestApexName(t *testing.T) { `) module := ctx.ModuleForTests("myapex", "android_common_com.android.myapex_image") - apexManifestRule := module.Rule("apexManifestRule") - ensureContains(t, apexManifestRule.Args["opt"], "-v name com.android.myapex") - apexBundle := module.Module().(*apexBundle) data := android.AndroidMkDataForTest(t, ctx, apexBundle) name := apexBundle.BaseModuleName() diff --git a/apex/builder.go b/apex/builder.go index 4aef3c182..06f1dd0f5 100644 --- a/apex/builder.go +++ b/apex/builder.go @@ -241,10 +241,11 @@ func (a *apexBundle) buildManifest(ctx android.ModuleContext, provideNativeLibs, provideNativeLibs = android.SortedUniqueStrings(provideNativeLibs) requireNativeLibs = android.SortedUniqueStrings(android.RemoveListFromList(requireNativeLibs, provideNativeLibs)) - // APEX name can be overridden + // VNDK APEX name is determined at runtime, so update "name" in apex_manifest optCommands := []string{} - if a.properties.Apex_name != nil { - optCommands = append(optCommands, "-v name "+*a.properties.Apex_name) + if a.vndkApex { + apexName := vndkApexNamePrefix + a.vndkVersion(ctx.DeviceConfig()) + optCommands = append(optCommands, "-v name "+apexName) } // Collect jniLibs. Notice that a.filesInfo is already sorted @@ -454,14 +455,6 @@ func (a *apexBundle) buildUnflattenedApex(ctx android.ModuleContext) { installSymbolFiles := (!ctx.Config().KatiEnabled() || a.ExportedToMake()) && a.installable() - // b/140136207. When there are overriding APEXes for a VNDK APEX, the symbols file for the overridden - // APEX and the overriding APEX will have the same installation paths at /apex/com.android.vndk.v - // as their apexName will be the same. To avoid the path conflicts, skip installing the symbol files - // for the overriding VNDK APEXes. - if a.vndkApex && len(a.overridableProperties.Overrides) > 0 { - installSymbolFiles = false - } - // Avoid creating duplicate build rules for multi-installed APEXes. if proptools.BoolDefault(a.properties.Multi_install_skip_symbol_files, false) { installSymbolFiles = false @@ -1012,7 +1005,7 @@ func (a *apexBundle) getOverrideManifestPackageName(ctx android.ModuleContext) s if a.vndkApex { overrideName, overridden := ctx.DeviceConfig().OverrideManifestPackageNameFor(vndkApexName) if overridden { - return strings.Replace(*a.properties.Apex_name, vndkApexName, overrideName, 1) + return overrideName + ".v" + a.vndkVersion(ctx.DeviceConfig()) } return "" } diff --git a/apex/vndk.go b/apex/vndk.go index 80560cf0c..c0be753d1 100644 --- a/apex/vndk.go +++ b/apex/vndk.go @@ -65,10 +65,6 @@ func apexVndkMutator(mctx android.TopDownMutatorContext) { } vndkVersion := ab.vndkVersion(mctx.DeviceConfig()) - - // Ensure VNDK APEX mount point is formatted as com.android.vndk.v### - ab.properties.Apex_name = proptools.StringPtr(vndkApexNamePrefix + vndkVersion) - apiLevel, err := android.ApiLevelFromUser(mctx, vndkVersion) if err != nil { mctx.PropertyErrorf("vndk_version", "%s", err.Error())