From 555c52491d60fe44b24fbae7f15955419d38e48d Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 19 Nov 2021 21:32:54 +0000 Subject: [PATCH] Revert "Move apex module installation into Soong" This reverts commit 246e3eed6706afbd8c0cf976f6350ebeceb67d7a. Reason for revert: broke aosp_bonito-userdebug build Change-Id: Ib613f8f22532fe4588d3ec96aa001de869f8a633 --- apex/androidmk.go | 34 +++++++++++++++--- apex/apex.go | 20 ++++------- apex/builder.go | 88 ++++++++++++----------------------------------- apex/prebuilt.go | 43 ++++++++++------------- apex/vndk.go | 43 +++++++++-------------- 5 files changed, 95 insertions(+), 133 deletions(-) diff --git a/apex/androidmk.go b/apex/androidmk.go index 832faf2de..204842992 100644 --- a/apex/androidmk.go +++ b/apex/androidmk.go @@ -108,6 +108,18 @@ func (a *apexBundle) androidMkForFiles(w io.Writer, apexBundleName, apexName, mo return moduleNames } + var postInstallCommands []string + for _, fi := range a.filesInfo { + if a.linkToSystemLib && fi.transitiveDep && fi.availableToPlatform() { + // TODO(jiyong): pathOnDevice should come from fi.module, not being calculated here + linkTarget := filepath.Join("/system", fi.path()) + linkPath := filepath.Join(a.installDir.ToMakePath().String(), apexBundleName, fi.path()) + mkdirCmd := "mkdir -p " + filepath.Dir(linkPath) + linkCmd := "ln -sfn " + linkTarget + " " + linkPath + postInstallCommands = append(postInstallCommands, mkdirCmd, linkCmd) + } + } + seenDataOutPaths := make(map[string]bool) for _, fi := range a.filesInfo { @@ -181,8 +193,6 @@ func (a *apexBundle) androidMkForFiles(w io.Writer, apexBundleName, apexName, mo // we will have duplicated notice entries. fmt.Fprintln(w, "LOCAL_NO_NOTICE_FILE := true") } - fmt.Fprintln(w, "LOCAL_SOONG_INSTALLED_MODULE :=", filepath.Join(modulePath, fi.stem())) - fmt.Fprintln(w, "LOCAL_SOONG_INSTALL_PAIRS :=", fi.builtFile.String()+":"+filepath.Join(modulePath, fi.stem())) fmt.Fprintln(w, "LOCAL_PREBUILT_MODULE_FILE :=", fi.builtFile.String()) fmt.Fprintln(w, "LOCAL_MODULE_CLASS :=", fi.class.nameInMake()) if fi.module != nil { @@ -292,10 +302,18 @@ func (a *apexBundle) androidMkForFiles(w io.Writer, apexBundleName, apexName, mo if len(patterns) > 0 { fmt.Fprintln(w, "LOCAL_OVERRIDES_MODULES :=", strings.Join(patterns, " ")) } + if len(a.compatSymlinks) > 0 { + // For flattened apexes, compat symlinks are attached to apex_manifest.json which is guaranteed for every apex + postInstallCommands = append(postInstallCommands, a.compatSymlinks...) + } } // File_contexts of flattened APEXes should be merged into file_contexts.bin fmt.Fprintln(w, "LOCAL_FILE_CONTEXTS :=", a.fileContexts) + + if len(postInstallCommands) > 0 { + fmt.Fprintln(w, "LOCAL_POST_INSTALL_CMD :=", strings.Join(postInstallCommands, " && ")) + } } fmt.Fprintln(w, "include $(BUILD_PREBUILT)") } @@ -376,8 +394,6 @@ func (a *apexBundle) androidMkForType() android.AndroidMkData { } fmt.Fprintln(w, "LOCAL_MODULE_STEM :=", name+stemSuffix) fmt.Fprintln(w, "LOCAL_UNINSTALLABLE_MODULE :=", !a.installable()) - fmt.Fprintln(w, "LOCAL_SOONG_INSTALLED_MODULE :=", a.installedFile.String()) - fmt.Fprintln(w, "LOCAL_SOONG_INSTALL_PAIRS :=", a.outputFile.String()+":"+a.installedFile.String()) // Because apex writes .mk with Custom(), we need to write manually some common properties // which are available via data.Entries @@ -402,6 +418,16 @@ func (a *apexBundle) androidMkForType() android.AndroidMkData { fmt.Fprintln(w, "LOCAL_REQUIRED_MODULES +=", strings.Join(a.requiredDeps, " ")) } a.writeRequiredModules(w, name) + var postInstallCommands []string + if a.prebuiltFileToDelete != "" { + postInstallCommands = append(postInstallCommands, "rm -rf "+ + filepath.Join(a.installDir.ToMakePath().String(), a.prebuiltFileToDelete)) + } + // For unflattened apexes, compat symlinks are attached to apex package itself as LOCAL_POST_INSTALL_CMD + postInstallCommands = append(postInstallCommands, a.compatSymlinks...) + if len(postInstallCommands) > 0 { + fmt.Fprintln(w, "LOCAL_POST_INSTALL_CMD :=", strings.Join(postInstallCommands, " && ")) + } if a.mergedNotices.Merged.Valid() { fmt.Fprintln(w, "LOCAL_NOTICE_FILE :=", a.mergedNotices.Merged.Path().String()) diff --git a/apex/apex.go b/apex/apex.go index 3f0f2470e..378efe65d 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -409,14 +409,14 @@ type apexBundle struct { // vendor/google/build/build_unbundled_mainline_module.sh for more detail. bundleModuleFile android.WritablePath - // Target directory to install this APEX. Usually out/target/product///apex. + // Target path to install this APEX. Usually out/target/product///apex. installDir android.InstallPath - // Path where this APEX was installed. - installedFile android.InstallPath - - // Installed locations of symlinks for backward compatibility. - compatSymlinks android.InstallPaths + // List of commands to create symlinks for backward compatibility. These commands will be + // attached as LOCAL_POST_INSTALL_CMD to apex package itself (for unflattened build) or + // apex_manifest (for flattened build) so that compat symlinks are always installed + // regardless of TARGET_FLATTEN_APEX setting. + compatSymlinks []string // Text file having the list of individual files that are included in this APEX. Used for // debugging purpose. @@ -442,10 +442,6 @@ type apexBundle struct { modulePaths []string } -func (*apexBundle) InstallBypassMake() bool { - return true -} - // apexFileClass represents a type of file that can be included in APEX. type apexFileClass int @@ -2101,9 +2097,7 @@ func (a *apexBundle) GenerateAndroidBuildActions(ctx android.ModuleContext) { a.linkToSystemLib = false } - if a.properties.ApexType != zipApex { - a.compatSymlinks = makeCompatSymlinks(a.BaseModuleName(), ctx, a.primaryApexType) - } + a.compatSymlinks = makeCompatSymlinks(a.BaseModuleName(), ctx) //////////////////////////////////////////////////////////////////////////////////////////// // 4) generate the build rules to create the APEX. This is done in builder.go. diff --git a/apex/builder.go b/apex/builder.go index aa8ce9dcc..3599c5d01 100644 --- a/apex/builder.go +++ b/apex/builder.go @@ -414,35 +414,18 @@ func (a *apexBundle) buildBundleConfig(ctx android.ModuleContext) android.Output func (a *apexBundle) buildUnflattenedApex(ctx android.ModuleContext) { apexType := a.properties.ApexType suffix := apexType.suffix() - apexName := proptools.StringDefault(a.properties.Apex_name, a.BaseModuleName()) //////////////////////////////////////////////////////////////////////////////////////////// // Step 1: copy built files to appropriate directories under the image directory imageDir := android.PathForModuleOut(ctx, "image"+suffix) - installSymbolFiles := !ctx.Config().KatiEnabled() || a.ExportedToMake() - - // 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 - } - // TODO(jiyong): use the RuleBuilder var copyCommands []string var implicitInputs []android.Path - pathWhenActivated := android.PathForModuleInPartitionInstall(ctx, "apex", apexName) for _, fi := range a.filesInfo { destPath := imageDir.Join(ctx, fi.path()).String() - var installedPath android.InstallPath + // Prepare the destination path destPathDir := filepath.Dir(destPath) if fi.class == appSet { @@ -461,20 +444,10 @@ func (a *apexBundle) buildUnflattenedApex(ctx android.ModuleContext) { copyCommands = append(copyCommands, fmt.Sprintf("unzip -qDD -d %s %s", destPathDir, fi.module.(*java.AndroidAppSet).PackedAdditionalOutputs().String())) - if installSymbolFiles { - installedPath = ctx.InstallFileWithExtraFilesZip(pathWhenActivated.Join(ctx, fi.installDir), - fi.stem(), fi.builtFile, fi.module.(*java.AndroidAppSet).PackedAdditionalOutputs()) - } } else { copyCommands = append(copyCommands, "cp -f "+fi.builtFile.String()+" "+destPath) - if installSymbolFiles { - installedPath = ctx.InstallFile(pathWhenActivated.Join(ctx, fi.installDir), fi.stem(), fi.builtFile) - } } implicitInputs = append(implicitInputs, fi.builtFile) - if installSymbolFiles { - implicitInputs = append(implicitInputs, installedPath) - } } // Create additional symlinks pointing the file inside the APEX (if any). Note that @@ -482,10 +455,6 @@ func (a *apexBundle) buildUnflattenedApex(ctx android.ModuleContext) { for _, symlinkPath := range fi.symlinkPaths() { symlinkDest := imageDir.Join(ctx, symlinkPath).String() copyCommands = append(copyCommands, "ln -sfn "+filepath.Base(destPath)+" "+symlinkDest) - if installSymbolFiles { - installedSymlink := ctx.InstallSymlink(pathWhenActivated.Join(ctx, filepath.Dir(symlinkPath)), filepath.Base(symlinkPath), installedPath) - implicitInputs = append(implicitInputs, installedSymlink) - } } // Copy the test files (if any) @@ -504,11 +473,6 @@ func (a *apexBundle) buildUnflattenedApex(ctx android.ModuleContext) { } } implicitInputs = append(implicitInputs, a.manifestPbOut) - if installSymbolFiles { - installedManifest := ctx.InstallFile(pathWhenActivated, "apex_manifest.pb", a.manifestPbOut) - installedKey := ctx.InstallFile(pathWhenActivated, "apex_pubkey", a.publicKeyFile) - implicitInputs = append(implicitInputs, installedManifest, installedKey) - } //////////////////////////////////////////////////////////////////////////////////////////// // Step 1.a: Write the list of files in this APEX to a txt file and compare it against @@ -877,55 +841,47 @@ func (a *apexBundle) buildUnflattenedApex(ctx android.ModuleContext) { a.outputFile = signedCompressedOutputFile } - installSuffix := suffix - if a.isCompressed { - installSuffix = imageCapexSuffix - } - // Install to $OUT/soong/{target,host}/.../apex. - a.installedFile = ctx.InstallFile(a.installDir, a.Name()+installSuffix, a.outputFile, - a.compatSymlinks.Paths()...) + ctx.InstallFile(a.installDir, a.Name()+suffix, a.outputFile) // installed-files.txt is dist'ed a.installedFilesFile = a.buildInstalledFilesFile(ctx, a.outputFile, imageDir) } +// Context "decorator", overriding the InstallBypassMake method to always reply `true`. +type flattenedApexContext struct { + android.ModuleContext +} + +func (c *flattenedApexContext) InstallBypassMake() bool { + return true +} + // buildFlattenedApex creates rules for a flattened APEX. Flattened APEX actually doesn't have a // single output file. It is a phony target for all the files under /system/apex/ directory. // This function creates the installation rules for the files. func (a *apexBundle) buildFlattenedApex(ctx android.ModuleContext) { bundleName := a.Name() - installedSymlinks := append(android.InstallPaths(nil), a.compatSymlinks...) if a.installable() { for _, fi := range a.filesInfo { dir := filepath.Join("apex", bundleName, fi.installDir) - installDir := android.PathForModuleInstall(ctx, dir) - if a.linkToSystemLib && fi.transitiveDep && fi.availableToPlatform() { - // TODO(jiyong): pathOnDevice should come from fi.module, not being calculated here - pathOnDevice := filepath.Join("/system", fi.path()) - installedSymlinks = append(installedSymlinks, - ctx.InstallAbsoluteSymlink(installDir, fi.stem(), pathOnDevice)) - } else { - target := ctx.InstallFile(installDir, fi.stem(), fi.builtFile) - for _, sym := range fi.symlinks { - installedSymlinks = append(installedSymlinks, - ctx.InstallSymlink(installDir, sym, target)) - } + target := ctx.InstallFile(android.PathForModuleInstall(ctx, dir), fi.stem(), fi.builtFile) + for _, sym := range fi.symlinks { + ctx.InstallSymlink(android.PathForModuleInstall(ctx, dir), sym, target) } } - - // Create install rules for the files added in GenerateAndroidBuildActions after - // buildFlattenedApex is called. Add the links to system libs (if any) as dependencies - // of the apex_manifest.pb file since it is always present. - dir := filepath.Join("apex", bundleName) - installDir := android.PathForModuleInstall(ctx, dir) - ctx.InstallFile(installDir, "apex_manifest.pb", a.manifestPbOut, installedSymlinks.Paths()...) - ctx.InstallFile(installDir, "apex_pubkey", a.publicKeyFile) } a.fileContexts = a.buildFileContexts(ctx) - a.outputFile = android.PathForModuleInstall(ctx, "apex", bundleName) + // Temporarily wrap the original `ctx` into a `flattenedApexContext` to have it reply true + // to `InstallBypassMake()` (thus making the call `android.PathForModuleInstall` below use + // `android.pathForInstallInMakeDir` instead of `android.PathForOutput`) to return the + // correct path to the flattened APEX (as its contents is installed by Make, not Soong). + // TODO(jiyong): Why do we need to set outputFile for flattened APEX? We don't seem to use + // it and it actually points to a path that can never be built. Remove this. + factx := flattenedApexContext{ctx} + a.outputFile = android.PathForModuleInstall(&factx, "apex", bundleName) } // getCertificateAndPrivateKey retrieves the cert and the private key that will be used to sign diff --git a/apex/prebuilt.go b/apex/prebuilt.go index 5b070c6fb..61e7a0be3 100644 --- a/apex/prebuilt.go +++ b/apex/prebuilt.go @@ -17,13 +17,11 @@ package apex import ( "fmt" "io" - "path/filepath" "strconv" "strings" "android/soong/android" "android/soong/java" - "github.com/google/blueprint" "github.com/google/blueprint/proptools" ) @@ -55,17 +53,18 @@ type prebuiltCommon struct { installDir android.InstallPath installFilename string - installedFile android.InstallPath outputApex android.WritablePath // A list of apexFile objects created in prebuiltCommon.initApexFilesForAndroidMk which are used // to create make modules in prebuiltCommon.AndroidMkEntries. apexFilesForAndroidMk []apexFile - // Installed locations of symlinks for backward compatibility. - compatSymlinks android.InstallPaths + // list of commands to create symlinks for backward compatibility. + // these commands will be attached as LOCAL_POST_INSTALL_CMD + compatSymlinks []string - hostRequired []string + hostRequired []string + postInstallCommands []string } type sanitizedPrebuilt interface { @@ -224,10 +223,13 @@ func (p *prebuiltCommon) AndroidMkEntries() []android.AndroidMkEntries { func(ctx android.AndroidMkExtraEntriesContext, entries *android.AndroidMkEntries) { entries.SetString("LOCAL_MODULE_PATH", p.installDir.ToMakePath().String()) entries.SetString("LOCAL_MODULE_STEM", p.installFilename) - entries.SetPath("LOCAL_SOONG_INSTALLED_MODULE", p.installedFile) - entries.SetString("LOCAL_SOONG_INSTALL_PAIRS", p.outputApex.String()+":"+p.installedFile.String()) entries.SetBoolIfTrue("LOCAL_UNINSTALLABLE_MODULE", !p.installable()) entries.AddStrings("LOCAL_OVERRIDES_MODULES", p.prebuiltCommonProperties.Overrides...) + postInstallCommands := append([]string{}, p.postInstallCommands...) + postInstallCommands = append(postInstallCommands, p.compatSymlinks...) + if len(postInstallCommands) > 0 { + entries.SetString("LOCAL_POST_INSTALL_CMD", strings.Join(postInstallCommands, " && ")) + } p.addRequiredModules(entries) }, }, @@ -257,9 +259,6 @@ func (p *prebuiltCommon) createEntriesForApexFile(fi apexFile, apexName string) ExtraEntries: []android.AndroidMkExtraEntriesFunc{ func(ctx android.AndroidMkExtraEntriesContext, entries *android.AndroidMkEntries) { entries.SetString("LOCAL_MODULE_PATH", p.installDir.ToMakePath().String()) - entries.SetString("LOCAL_SOONG_INSTALLED_MODULE :=", filepath.Join(p.installDir.String(), fi.stem())) - entries.SetString("LOCAL_SOONG_INSTALL_PAIRS :=", - fi.builtFile.String()+":"+filepath.Join(p.installDir.String(), fi.stem())) // soong_java_prebuilt.mk sets LOCAL_MODULE_SUFFIX := .jar Therefore // we need to remove the suffix from LOCAL_MODULE_STEM, otherwise @@ -472,10 +471,6 @@ type Prebuilt struct { inputApex android.Path } -func (p *Prebuilt) InstallBypassMake() bool { - return true -} - type ApexFileProperties struct { // the path to the prebuilt .apex file to import. // @@ -761,15 +756,15 @@ func (p *Prebuilt) GenerateAndroidBuildActions(ctx android.ModuleContext) { // Save the files that need to be made available to Make. p.initApexFilesForAndroidMk(ctx) - // in case that prebuilt_apex replaces source apex (using prefer: prop) - p.compatSymlinks = makeCompatSymlinks(p.BaseModuleName(), ctx, true) - // or that prebuilt_apex overrides other apexes (using overrides: prop) - for _, overridden := range p.prebuiltCommonProperties.Overrides { - p.compatSymlinks = append(p.compatSymlinks, makeCompatSymlinks(overridden, ctx, true)...) + if p.installable() { + ctx.InstallFile(p.installDir, p.installFilename, p.inputApex) } - if p.installable() { - p.installedFile = ctx.InstallFile(p.installDir, p.installFilename, p.inputApex, p.compatSymlinks.Paths()...) + // in case that prebuilt_apex replaces source apex (using prefer: prop) + p.compatSymlinks = makeCompatSymlinks(p.BaseModuleName(), ctx) + // or that prebuilt_apex overrides other apexes (using overrides: prop) + for _, overridden := range p.prebuiltCommonProperties.Overrides { + p.compatSymlinks = append(p.compatSymlinks, makeCompatSymlinks(overridden, ctx)...) } } @@ -969,10 +964,10 @@ func (a *ApexSet) GenerateAndroidBuildActions(ctx android.ModuleContext) { } // in case that apex_set replaces source apex (using prefer: prop) - a.compatSymlinks = makeCompatSymlinks(a.BaseModuleName(), ctx, true) + a.compatSymlinks = makeCompatSymlinks(a.BaseModuleName(), ctx) // or that apex_set overrides other apexes (using overrides: prop) for _, overridden := range a.prebuiltCommonProperties.Overrides { - a.compatSymlinks = append(a.compatSymlinks, makeCompatSymlinks(overridden, ctx, true)...) + a.compatSymlinks = append(a.compatSymlinks, makeCompatSymlinks(overridden, ctx)...) } } diff --git a/apex/vndk.go b/apex/vndk.go index cf525a874..75c0fb01b 100644 --- a/apex/vndk.go +++ b/apex/vndk.go @@ -15,6 +15,7 @@ package apex import ( + "path/filepath" "strings" "android/soong/android" @@ -95,14 +96,11 @@ func apexVndkDepsMutator(mctx android.BottomUpMutatorContext) { } // name is module.BaseModuleName() which is used as LOCAL_MODULE_NAME and also LOCAL_OVERRIDES_* -func makeCompatSymlinks(name string, ctx android.ModuleContext, primaryApex bool) (symlinks android.InstallPaths) { +func makeCompatSymlinks(name string, ctx android.ModuleContext) (symlinks []string) { // small helper to add symlink commands - addSymlink := func(target string, dir android.InstallPath, linkName string) { - if primaryApex { - symlinks = append(symlinks, ctx.InstallAbsoluteSymlink(dir, linkName, target)) - } else { - symlinks = append(symlinks, dir.Join(ctx, linkName)) - } + addSymlink := func(target, dir, linkName string) { + link := filepath.Join(dir, linkName) + symlinks = append(symlinks, "mkdir -p "+dir+" && rm -rf "+link+" && ln -sf "+target+" "+link) } // TODO(b/142911355): [VNDK APEX] Fix hard-coded references to /system/lib/vndk @@ -120,15 +118,14 @@ func makeCompatSymlinks(name string, ctx android.ModuleContext, primaryApex bool // the name of vndk apex is formatted "com.android.vndk.v" + version apexName := vndkApexNamePrefix + vndkVersion if ctx.Config().Android64() { - dir := android.PathForModuleInPartitionInstall(ctx, "system", "lib64") - addSymlink("/apex/"+apexName+"/lib64", dir, "vndk-sp-"+vndkVersion) - addSymlink("/apex/"+apexName+"/lib64", dir, "vndk-"+vndkVersion) + addSymlink("/apex/"+apexName+"/lib64", "$(TARGET_OUT)/lib64", "vndk-sp-"+vndkVersion) + addSymlink("/apex/"+apexName+"/lib64", "$(TARGET_OUT)/lib64", "vndk-"+vndkVersion) } if !ctx.Config().Android64() || ctx.DeviceConfig().DeviceSecondaryArch() != "" { - dir := android.PathForModuleInPartitionInstall(ctx, "system", "lib") - addSymlink("/apex/"+apexName+"/lib", dir, "vndk-sp-"+vndkVersion) - addSymlink("/apex/"+apexName+"/lib", dir, "vndk-"+vndkVersion) + addSymlink("/apex/"+apexName+"/lib", "$(TARGET_OUT)/lib", "vndk-sp-"+vndkVersion) + addSymlink("/apex/"+apexName+"/lib", "$(TARGET_OUT)/lib", "vndk-"+vndkVersion) } + return } // http://b/121248172 - create a link from /system/usr/icu to @@ -136,25 +133,19 @@ func makeCompatSymlinks(name string, ctx android.ModuleContext, primaryApex bool // A symlink can't overwrite a directory and the /system/usr/icu directory once // existed so the required structure must be created whatever we find. if name == "com.android.i18n" { - dir := android.PathForModuleInPartitionInstall(ctx, "system", "usr") - addSymlink("/apex/com.android.i18n/etc/icu", dir, "icu") + addSymlink("/apex/com.android.i18n/etc/icu", "$(TARGET_OUT)/usr", "icu") + return } // TODO(b/124106384): Clean up compat symlinks for ART binaries. - if name == "com.android.art" { - dir := android.PathForModuleInPartitionInstall(ctx, "system", "bin") - addSymlink("/apex/com.android.art/bin/dalvikvm", dir, "dalvikvm") + if name == "com.android.art" || strings.HasPrefix(name, "com.android.art.") { + addSymlink("/apex/com.android.art/bin/dalvikvm", "$(TARGET_OUT)/bin", "dalvikvm") dex2oat := "dex2oat32" if ctx.Config().Android64() { dex2oat = "dex2oat64" } - addSymlink("/apex/com.android.art/bin/"+dex2oat, dir, "dex2oat") - } else if name == "com.android.art" || strings.HasPrefix(name, "com.android.art.") { - dir := android.PathForModuleInPartitionInstall(ctx, "system", "bin") - symlinks = append(symlinks, - dir.Join(ctx, "dalvikvm"), - dir.Join(ctx, "dex2oat")) + addSymlink("/apex/com.android.art/bin/"+dex2oat, "$(TARGET_OUT)/bin", "dex2oat") + return } - - return symlinks + return }