From 18d98bc3e0d50801b76ba53a2c9a445c16e8c57f Mon Sep 17 00:00:00 2001 From: Sasha Smundak Date: Wed, 27 May 2020 16:36:07 -0700 Subject: [PATCH] Allow apex module to have android_app_set as its constituent. Fixes: 157166068 Test: treehugger & manual Change-Id: I9f91f1b761286f489d175eb0772f78f702e8a2d6 --- apex/androidmk.go | 30 ++++++++++------- apex/apex.go | 12 ++++++- apex/apex_test.go | 84 ++++++++++++++++++++++++++++++++++++----------- apex/builder.go | 35 +++++++++++++++----- java/app.go | 28 +++++++--------- java/app_test.go | 4 +-- 6 files changed, 135 insertions(+), 58 deletions(-) diff --git a/apex/androidmk.go b/apex/androidmk.go index 9321ad261..1b3a4ba10 100644 --- a/apex/androidmk.go +++ b/apex/androidmk.go @@ -81,7 +81,7 @@ func (a *apexBundle) androidMkForFiles(w io.Writer, apexBundleName, apexName, mo seenDataOutPaths := make(map[string]bool) for _, fi := range a.filesInfo { - if cc, ok := fi.module.(*cc.Module); ok && cc.Properties.HideFromMake { + if ccMod, ok := fi.module.(*cc.Module); ok && ccMod.Properties.HideFromMake { continue } @@ -178,7 +178,8 @@ func (a *apexBundle) androidMkForFiles(w io.Writer, apexBundleName, apexName, mo if fi.jacocoReportClassesFile != nil { fmt.Fprintln(w, "LOCAL_SOONG_JACOCO_REPORT_CLASSES_JAR :=", fi.jacocoReportClassesFile.String()) } - if fi.class == javaSharedLib { + switch fi.class { + case javaSharedLib: javaModule := fi.module.(java.Dependency) // soong_java_prebuilt.mk sets LOCAL_MODULE_SUFFIX := .jar Therefore // we need to remove the suffix from LOCAL_MODULE_STEM, otherwise @@ -189,7 +190,7 @@ func (a *apexBundle) androidMkForFiles(w io.Writer, apexBundleName, apexName, mo fmt.Fprintln(w, "LOCAL_SOONG_DEX_JAR :=", fi.builtFile.String()) fmt.Fprintln(w, "LOCAL_DEX_PREOPT := false") fmt.Fprintln(w, "include $(BUILD_SYSTEM)/soong_java_prebuilt.mk") - } else if fi.class == app { + case app: fmt.Fprintln(w, "LOCAL_CERTIFICATE :=", fi.certificate.AndroidMkString()) // soong_app_prebuilt.mk sets LOCAL_MODULE_SUFFIX := .apk Therefore // we need to remove the suffix from LOCAL_MODULE_STEM, otherwise @@ -199,19 +200,26 @@ func (a *apexBundle) androidMkForFiles(w io.Writer, apexBundleName, apexName, mo fmt.Fprintln(w, "LOCAL_PREBUILT_COVERAGE_ARCHIVE :=", strings.Join(app.JniCoverageOutputs().Strings(), " ")) } fmt.Fprintln(w, "include $(BUILD_SYSTEM)/soong_app_prebuilt.mk") - } else if fi.class == nativeSharedLib || fi.class == nativeExecutable || fi.class == nativeTest { + case appSet: + as, ok := fi.module.(*java.AndroidAppSet) + if !ok { + panic(fmt.Sprintf("Expected %s to be AndroidAppSet", fi.module)) + } + fmt.Fprintln(w, "LOCAL_APK_SET_MASTER_FILE :=", as.MasterFile()) + fmt.Fprintln(w, "include $(BUILD_SYSTEM)/soong_android_app_set.mk") + case nativeSharedLib, nativeExecutable, nativeTest: fmt.Fprintln(w, "LOCAL_MODULE_STEM :=", fi.Stem()) - if cc, ok := fi.module.(*cc.Module); ok { - if cc.UnstrippedOutputFile() != nil { - fmt.Fprintln(w, "LOCAL_SOONG_UNSTRIPPED_BINARY :=", cc.UnstrippedOutputFile().String()) + if ccMod, ok := fi.module.(*cc.Module); ok { + if ccMod.UnstrippedOutputFile() != nil { + fmt.Fprintln(w, "LOCAL_SOONG_UNSTRIPPED_BINARY :=", ccMod.UnstrippedOutputFile().String()) } - cc.AndroidMkWriteAdditionalDependenciesForSourceAbiDiff(w) - if cc.CoverageOutputFile().Valid() { - fmt.Fprintln(w, "LOCAL_PREBUILT_COVERAGE_ARCHIVE :=", cc.CoverageOutputFile().String()) + ccMod.AndroidMkWriteAdditionalDependenciesForSourceAbiDiff(w) + if ccMod.CoverageOutputFile().Valid() { + fmt.Fprintln(w, "LOCAL_PREBUILT_COVERAGE_ARCHIVE :=", ccMod.CoverageOutputFile().String()) } } fmt.Fprintln(w, "include $(BUILD_SYSTEM)/soong_cc_prebuilt.mk") - } else { + default: fmt.Fprintln(w, "LOCAL_MODULE_STEM :=", fi.Stem()) if fi.builtFile == a.manifestPbOut && apexType == flattenedApex { if a.primaryApexType { diff --git a/apex/apex.go b/apex/apex.go index 8fd80fcaf..67bec0629 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -1115,6 +1115,7 @@ const ( javaSharedLib nativeTest app + appSet ) func (class apexFileClass) NameInMake() string { @@ -1129,7 +1130,7 @@ func (class apexFileClass) NameInMake() string { return "JAVA_LIBRARIES" case nativeTest: return "NATIVE_TESTS" - case app: + case app, appSet: // b/142537672 Why isn't this APP? We want to have full control over // the paths and file names of the apk file under the flattend APEX. // If this is set to APP, then the paths and file names are modified @@ -1998,6 +1999,15 @@ func (a *apexBundle) GenerateAndroidBuildActions(ctx android.ModuleContext) { filesInfo = append(filesInfo, apexFileForAndroidApp(ctx, ap)) } else if ap, ok := child.(*java.AndroidTestHelperApp); ok { filesInfo = append(filesInfo, apexFileForAndroidApp(ctx, ap)) + } else if ap, ok := child.(*java.AndroidAppSet); ok { + appDir := "app" + if ap.Privileged() { + appDir = "priv-app" + } + af := newApexFile(ctx, ap.OutputFile(), ap.Name(), + filepath.Join(appDir, ap.BaseModuleName()), appSet, ap) + af.certificate = java.PresignedCertificate + filesInfo = append(filesInfo, af) } else { ctx.PropertyErrorf("apps", "%q is not an android_app module", depName) } diff --git a/apex/apex_test.go b/apex/apex_test.go index e45cd0091..d6a5d0909 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -96,7 +96,7 @@ func withTargets(targets map[android.OsType][]android.Target) testCustomizer { // - X86 (secondary) // - Arm64 on X86_64 (native bridge) // - Arm on X86 (native bridge) -func withNativeBridgeEnabled(fs map[string][]byte, config android.Config) { +func withNativeBridgeEnabled(_ map[string][]byte, config android.Config) { config.Targets[android.Android] = []android.Target{ {Os: android.Android, Arch: android.Arch{ArchType: android.X86_64, ArchVariant: "silvermont", Abi: []string{"arm64-v8a"}}, NativeBridge: android.NativeBridgeDisabled, NativeBridgeHostArchName: "", NativeBridgeRelativePath: ""}, @@ -115,15 +115,15 @@ func withManifestPackageNameOverrides(specs []string) testCustomizer { } } -func withBinder32bit(fs map[string][]byte, config android.Config) { +func withBinder32bit(_ map[string][]byte, config android.Config) { config.TestProductVariables.Binder32bit = proptools.BoolPtr(true) } -func withUnbundledBuild(fs map[string][]byte, config android.Config) { +func withUnbundledBuild(_ map[string][]byte, config android.Config) { config.TestProductVariables.Unbundled_build = proptools.BoolPtr(true) } -func testApexContext(t *testing.T, bp string, handlers ...testCustomizer) (*android.TestContext, android.Config) { +func testApexContext(_ *testing.T, bp string, handlers ...testCustomizer) (*android.TestContext, android.Config) { android.ClearApexDependency() bp = bp + ` @@ -187,6 +187,7 @@ func testApexContext(t *testing.T, bp string, handlers ...testCustomizer) (*andr "baz": nil, "bar/baz": nil, "testdata/baz": nil, + "AppSet.apks": nil, } cc.GatherRequiredFilesForTest(fs) @@ -258,7 +259,7 @@ func setUp() { } func tearDown() { - os.RemoveAll(buildDir) + _ = os.RemoveAll(buildDir) } // ensure that 'result' equals 'expected' @@ -294,6 +295,17 @@ func ensureNotContains(t *testing.T, result string, notExpected string) { } } +func ensureMatches(t *testing.T, result string, expectedRex string) { + ok, err := regexp.MatchString(expectedRex, result) + if err != nil { + t.Fatalf("regexp failure trying to match %s against `%s` expression: %s", result, expectedRex, err) + return + } + if !ok { + t.Errorf("%s does not match regular expession %s", result, expectedRex) + } +} + func ensureListContains(t *testing.T, result []string, expected string) { t.Helper() if !android.InList(expected, result) { @@ -4772,6 +4784,38 @@ func TestAppBundle(t *testing.T) { ensureContains(t, content, `"apex_config":{"apex_embedded_apk_config":[{"package_name":"com.android.foo","path":"app/AppFoo/AppFoo.apk"}]}`) } +func TestAppSetBundle(t *testing.T) { + ctx, _ := testApex(t, ` + apex { + name: "myapex", + key: "myapex.key", + apps: ["AppSet"], + } + + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + } + + android_app_set { + name: "AppSet", + set: "AppSet.apks", + }`) + mod := ctx.ModuleForTests("myapex", "android_common_myapex_image") + bundleConfigRule := mod.Description("Bundle Config") + content := bundleConfigRule.Args["content"] + ensureContains(t, content, `"compression":{"uncompressed_glob":["apex_payload.img","apex_manifest.*"]}`) + s := mod.Rule("apexRule").Args["copy_commands"] + copyCmds := regexp.MustCompile(" *&& *").Split(s, -1) + if len(copyCmds) != 3 { + t.Fatalf("Expected 3 commands, got %d in:\n%s", len(copyCmds), s) + } + ensureMatches(t, copyCmds[0], "^rm -rf .*/app/AppSet$") + ensureMatches(t, copyCmds[1], "^mkdir -p .*/app/AppSet$") + ensureMatches(t, copyCmds[2], "^unzip .*-d .*/app/AppSet .*/AppSet.zip$") +} + func testNoUpdatableJarsInBootImage(t *testing.T, errmsg string, transformDexpreoptConfig func(*dexpreopt.GlobalConfig)) { t.Helper() @@ -4931,7 +4975,7 @@ func TestUpdatable_should_set_min_sdk_version(t *testing.T) { func TestNoUpdatableJarsInBootImage(t *testing.T) { - var error string + var err string var transform func(*dexpreopt.GlobalConfig) t.Run("updatable jar from ART apex in the ART boot image => ok", func(t *testing.T) { @@ -4942,35 +4986,35 @@ func TestNoUpdatableJarsInBootImage(t *testing.T) { }) t.Run("updatable jar from ART apex in the framework boot image => error", func(t *testing.T) { - error = "module 'some-art-lib' from updatable apex 'com.android.art.something' is not allowed in the framework boot image" + err = "module 'some-art-lib' from updatable apex 'com.android.art.something' is not allowed in the framework boot image" transform = func(config *dexpreopt.GlobalConfig) { config.BootJars = []string{"com.android.art.something:some-art-lib"} } - testNoUpdatableJarsInBootImage(t, error, transform) + testNoUpdatableJarsInBootImage(t, err, transform) }) t.Run("updatable jar from some other apex in the ART boot image => error", func(t *testing.T) { - error = "module 'some-updatable-apex-lib' from updatable apex 'some-updatable-apex' is not allowed in the ART boot image" + err = "module 'some-updatable-apex-lib' from updatable apex 'some-updatable-apex' is not allowed in the ART boot image" transform = func(config *dexpreopt.GlobalConfig) { config.ArtApexJars = []string{"some-updatable-apex:some-updatable-apex-lib"} } - testNoUpdatableJarsInBootImage(t, error, transform) + testNoUpdatableJarsInBootImage(t, err, transform) }) t.Run("non-updatable jar from some other apex in the ART boot image => error", func(t *testing.T) { - error = "module 'some-non-updatable-apex-lib' is not allowed in the ART boot image" + err = "module 'some-non-updatable-apex-lib' is not allowed in the ART boot image" transform = func(config *dexpreopt.GlobalConfig) { config.ArtApexJars = []string{"some-non-updatable-apex:some-non-updatable-apex-lib"} } - testNoUpdatableJarsInBootImage(t, error, transform) + testNoUpdatableJarsInBootImage(t, err, transform) }) t.Run("updatable jar from some other apex in the framework boot image => error", func(t *testing.T) { - error = "module 'some-updatable-apex-lib' from updatable apex 'some-updatable-apex' is not allowed in the framework boot image" + err = "module 'some-updatable-apex-lib' from updatable apex 'some-updatable-apex' is not allowed in the framework boot image" transform = func(config *dexpreopt.GlobalConfig) { config.BootJars = []string{"some-updatable-apex:some-updatable-apex-lib"} } - testNoUpdatableJarsInBootImage(t, error, transform) + testNoUpdatableJarsInBootImage(t, err, transform) }) t.Run("non-updatable jar from some other apex in the framework boot image => ok", func(t *testing.T) { @@ -4981,27 +5025,27 @@ func TestNoUpdatableJarsInBootImage(t *testing.T) { }) t.Run("nonexistent jar in the ART boot image => error", func(t *testing.T) { - error = "failed to find a dex jar path for module 'nonexistent'" + err = "failed to find a dex jar path for module 'nonexistent'" transform = func(config *dexpreopt.GlobalConfig) { config.ArtApexJars = []string{"platform:nonexistent"} } - testNoUpdatableJarsInBootImage(t, error, transform) + testNoUpdatableJarsInBootImage(t, err, transform) }) t.Run("nonexistent jar in the framework boot image => error", func(t *testing.T) { - error = "failed to find a dex jar path for module 'nonexistent'" + err = "failed to find a dex jar path for module 'nonexistent'" transform = func(config *dexpreopt.GlobalConfig) { config.BootJars = []string{"platform:nonexistent"} } - testNoUpdatableJarsInBootImage(t, error, transform) + testNoUpdatableJarsInBootImage(t, err, transform) }) t.Run("platform jar in the ART boot image => error", func(t *testing.T) { - error = "module 'some-platform-lib' is not allowed in the ART boot image" + err = "module 'some-platform-lib' is not allowed in the ART boot image" transform = func(config *dexpreopt.GlobalConfig) { config.ArtApexJars = []string{"platform:some-platform-lib"} } - testNoUpdatableJarsInBootImage(t, error, transform) + testNoUpdatableJarsInBootImage(t, err, transform) }) t.Run("platform jar in the framework boot image => ok", func(t *testing.T) { diff --git a/apex/builder.go b/apex/builder.go index 0108d068a..17eac1a39 100644 --- a/apex/builder.go +++ b/apex/builder.go @@ -69,11 +69,14 @@ var ( // by default set to (uid/gid/mode) = (1000/1000/0644) // TODO(b/113082813) make this configurable using config.fs syntax generateFsConfig = pctx.StaticRule("generateFsConfig", blueprint.RuleParams{ - Command: `echo '/ 1000 1000 0755' > ${out} && ` + - `echo ${ro_paths} | tr ' ' '\n' | awk '{print "/"$$1 " 1000 1000 0644"}' >> ${out} && ` + - `echo ${exec_paths} | tr ' ' '\n' | awk '{print "/"$$1 " 0 2000 0755"}' >> ${out}`, - Description: "fs_config ${out}", - }, "ro_paths", "exec_paths") + Command: `( echo '/ 1000 1000 0755' ` + + `&& for i in ${ro_paths}; do echo "/$$i 1000 1000 0644"; done ` + + `&& for i in ${exec_paths}; do echo "/$$i 0 2000 0755"; done ` + + `&& ( tr ' ' '\n' <${out}.apklist | for i in ${apk_paths}; do read apk; echo "/$$i 0 2000 0755"; zipinfo -1 $$apk | sed "s:\(.*\):/$$i/\1 1000 1000 0644:"; done ) ) > ${out}`, + Description: "fs_config ${out}", + Rspfile: "$out.apklist", + RspfileContent: "$in", + }, "ro_paths", "exec_paths", "apk_paths") apexManifestRule = pctx.StaticRule("apexManifestRule", blueprint.RuleParams{ Command: `rm -f $out && ${jsonmodify} $in ` + @@ -287,7 +290,7 @@ func (a *apexBundle) buildBundleConfig(ctx android.ModuleContext) android.Output // collect the manifest names and paths of android apps // if their manifest names are overridden for _, fi := range a.filesInfo { - if fi.class != app { + if fi.class != app && fi.class != appSet { continue } packageName := fi.overriddenPackageName @@ -337,13 +340,22 @@ func (a *apexBundle) buildUnflattenedApex(ctx android.ModuleContext) { var copyCommands []string for _, fi := range a.filesInfo { destPath := android.PathForModuleOut(ctx, "image"+suffix, fi.Path()).String() - copyCommands = append(copyCommands, "mkdir -p "+filepath.Dir(destPath)) + destPathDir := filepath.Dir(destPath) + if fi.class == appSet { + copyCommands = append(copyCommands, "rm -rf "+destPathDir) + } + copyCommands = append(copyCommands, "mkdir -p "+destPathDir) 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()) copyCommands = append(copyCommands, "ln -sfn "+pathOnDevice+" "+destPath) } else { - copyCommands = append(copyCommands, "cp -f "+fi.builtFile.String()+" "+destPath) + if fi.class == appSet { + copyCommands = append(copyCommands, + fmt.Sprintf("unzip -q -d %s %s", destPathDir, fi.builtFile.String())) + } else { + copyCommands = append(copyCommands, "cp -f "+fi.builtFile.String()+" "+destPath) + } implicitInputs = append(implicitInputs, fi.builtFile) } // create additional symlinks pointing the file inside the APEX @@ -416,6 +428,8 @@ func (a *apexBundle) buildUnflattenedApex(ctx android.ModuleContext) { // files and dirs that will be created in APEX var readOnlyPaths = []string{"apex_manifest.json", "apex_manifest.pb"} var executablePaths []string // this also includes dirs + var extractedAppSetPaths android.Paths + var extractedAppSetDirs []string for _, f := range a.filesInfo { pathInApex := f.Path() if f.installDir == "bin" || strings.HasPrefix(f.installDir, "bin/") { @@ -426,6 +440,9 @@ func (a *apexBundle) buildUnflattenedApex(ctx android.ModuleContext) { for _, s := range f.symlinks { executablePaths = append(executablePaths, filepath.Join(f.installDir, s)) } + } else if f.class == appSet { + extractedAppSetPaths = append(extractedAppSetPaths, f.builtFile) + extractedAppSetDirs = append(extractedAppSetDirs, f.installDir) } else { readOnlyPaths = append(readOnlyPaths, pathInApex) } @@ -446,9 +463,11 @@ func (a *apexBundle) buildUnflattenedApex(ctx android.ModuleContext) { Rule: generateFsConfig, Output: cannedFsConfig, Description: "generate fs config", + Inputs: extractedAppSetPaths, Args: map[string]string{ "ro_paths": strings.Join(readOnlyPaths, " "), "exec_paths": strings.Join(executablePaths, " "), + "apk_paths": strings.Join(extractedAppSetDirs, " "), }, }) diff --git a/java/app.go b/java/app.go index a45ab6f99..374710b53 100755 --- a/java/app.go +++ b/java/app.go @@ -96,6 +96,14 @@ func (as *AndroidAppSet) Privileged() bool { return Bool(as.properties.Privileged) } +func (as *AndroidAppSet) OutputFile() android.Path { + return as.packedOutput +} + +func (as *AndroidAppSet) MasterFile() string { + return as.masterFile +} + var TargetCpuAbi = map[string]string{ "arm": "ARMEABI_V7A", "arm64": "ARM64_V8A", @@ -120,7 +128,7 @@ func SupportedAbis(ctx android.ModuleContext) []string { } func (as *AndroidAppSet) GenerateAndroidBuildActions(ctx android.ModuleContext) { - as.packedOutput = android.PathForModuleOut(ctx, "extracted.zip") + as.packedOutput = android.PathForModuleOut(ctx, ctx.ModuleName()+".zip") // We are assuming here that the master file in the APK // set has `.apk` suffix. If it doesn't the build will fail. // APK sets containing APEX files are handled elsewhere. @@ -145,18 +153,6 @@ func (as *AndroidAppSet) GenerateAndroidBuildActions(ctx android.ModuleContext) "stem": ctx.ModuleName(), }, }) - // TODO(asmundak): add this (it's wrong now, will cause copying extracted.zip) - /* - var installDir android.InstallPath - if Bool(as.properties.Privileged) { - installDir = android.PathForModuleInstall(ctx, "priv-app", as.BaseModuleName()) - } else if ctx.InstallInTestcases() { - installDir = android.PathForModuleInstall(ctx, as.BaseModuleName(), ctx.DeviceConfig().DeviceArch()) - } else { - installDir = android.PathForModuleInstall(ctx, "app", as.BaseModuleName()) - } - ctx.InstallFile(installDir, as.masterFile", as.packedOutput) - */ } // android_app_set extracts a set of APKs based on the target device @@ -335,7 +331,7 @@ type Certificate struct { presigned bool } -var presignedCertificate = Certificate{presigned: true} +var PresignedCertificate = Certificate{presigned: true} func (c Certificate) AndroidMkString() string { if c.presigned { @@ -1474,7 +1470,7 @@ func (a *AndroidAppImport) generateAndroidBuildActions(ctx android.ModuleContext // Sign or align the package if package has not been preprocessed if a.preprocessed { a.outputFile = srcApk - a.certificate = presignedCertificate + a.certificate = PresignedCertificate } else if !Bool(a.properties.Presigned) { // If the certificate property is empty at this point, default_dev_cert must be set to true. // Which makes processMainCert's behavior for the empty cert string WAI. @@ -1494,7 +1490,7 @@ func (a *AndroidAppImport) generateAndroidBuildActions(ctx android.ModuleContext alignedApk := android.PathForModuleOut(ctx, "zip-aligned", apkFilename) TransformZipAlign(ctx, alignedApk, dexOutput) a.outputFile = alignedApk - a.certificate = presignedCertificate + a.certificate = PresignedCertificate } // TODO: Optionally compress the output apk. diff --git a/java/app_test.go b/java/app_test.go index eb583bea6..eeba161b3 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -149,7 +149,7 @@ func TestAndroidAppSet(t *testing.T) { prerelease: true, }`) module := ctx.ModuleForTests("foo", "android_common") - const packedSplitApks = "extracted.zip" + const packedSplitApks = "foo.zip" params := module.Output(packedSplitApks) if params.Rule == nil { t.Errorf("expected output %s is missing", packedSplitApks) @@ -218,7 +218,7 @@ func TestAndroidAppSet_Variants(t *testing.T) { ctx := testContext() run(t, ctx, config) module := ctx.ModuleForTests("foo", "android_common") - const packedSplitApks = "extracted.zip" + const packedSplitApks = "foo.zip" params := module.Output(packedSplitApks) for k, v := range test.expected { if actual := params.Args[k]; actual != v {