From 8ce1efc5af753b74fbf7f073489f1e6fdedaf372 Mon Sep 17 00:00:00 2001 From: Jingwen Chen Date: Tue, 19 Apr 2022 13:57:01 +0000 Subject: [PATCH] Suffix the build ID to the dirname of APK-in-APEX files. This fixes an issue with package manager's cache invalidation. Test: CI Bug: 226559955 Bug: 224589412 Change-Id: I8af49d51ff99cf8184d0e4d1136fff1cdb29c23e Merged-In: I8af49d51ff99cf8184d0e4d1136fff1cdb29c23e --- apex/apex.go | 31 +++++++++++++++++++++++--- apex/apex_test.go | 57 ++++++++++++++++++++++++++++++++++++----------- 2 files changed, 72 insertions(+), 16 deletions(-) diff --git a/apex/apex.go b/apex/apex.go index a7b0a4fe2..62013cf63 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -19,6 +19,7 @@ package apex import ( "fmt" "path/filepath" + "regexp" "sort" "strings" @@ -1656,13 +1657,33 @@ type androidApp interface { var _ androidApp = (*java.AndroidApp)(nil) var _ androidApp = (*java.AndroidAppImport)(nil) +func sanitizedBuildIdForPath(ctx android.BaseModuleContext) string { + buildId := ctx.Config().BuildId() + + // The build ID is used as a suffix for a filename, so ensure that + // the set of characters being used are sanitized. + // - any word character: [a-zA-Z0-9_] + // - dots: . + // - dashes: - + validRegex := regexp.MustCompile(`^[\w\.\-\_]+$`) + if !validRegex.MatchString(buildId) { + ctx.ModuleErrorf("Unable to use build id %s as filename suffix, valid characters are [a-z A-Z 0-9 _ . -].", buildId) + } + return buildId +} + func apexFileForAndroidApp(ctx android.BaseModuleContext, aapp androidApp) apexFile { appDir := "app" if aapp.Privileged() { appDir = "priv-app" } - dirInApex := filepath.Join(appDir, aapp.InstallApkName()) + + // TODO(b/224589412, b/226559955): Ensure that the subdirname is suffixed + // so that PackageManager correctly invalidates the existing installed apk + // in favour of the new APK-in-APEX. See bugs for more information. + dirInApex := filepath.Join(appDir, aapp.InstallApkName()+"@"+sanitizedBuildIdForPath(ctx)) fileToCopy := aapp.OutputFile() + af := newApexFile(ctx, fileToCopy, aapp.BaseModuleName(), dirInApex, app, aapp) af.jacocoReportClassesFile = aapp.JacocoReportClassesFile() af.lintDepSets = aapp.LintDepSets() @@ -1895,8 +1916,12 @@ func (a *apexBundle) GenerateAndroidBuildActions(ctx android.ModuleContext) { if ap.Privileged() { appDir = "priv-app" } - af := newApexFile(ctx, ap.OutputFile(), ap.BaseModuleName(), - filepath.Join(appDir, ap.BaseModuleName()), appSet, ap) + // TODO(b/224589412, b/226559955): Ensure that the dirname is + // suffixed so that PackageManager correctly invalidates the + // existing installed apk in favour of the new APK-in-APEX. + // See bugs for more information. + appDirName := filepath.Join(appDir, ap.BaseModuleName()+"@"+sanitizedBuildIdForPath(ctx)) + af := newApexFile(ctx, ap.OutputFile(), ap.BaseModuleName(), appDirName, appSet, ap) af.certificate = java.PresignedCertificate filesInfo = append(filesInfo, af) } else { diff --git a/apex/apex_test.go b/apex/apex_test.go index 77cbb58c3..7b2905884 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -223,6 +223,7 @@ var prepareForApexTest = android.GroupFixturePreparers( // not because of these tests specifically (it's not used by the tests) variables.Platform_version_active_codenames = []string{"Q", "Tiramisu"} variables.Platform_vndk_version = proptools.StringPtr("29") + variables.BuildId = proptools.StringPtr("TEST.BUILD_ID") }), ) @@ -682,7 +683,7 @@ func TestDefaults(t *testing.T) { "etc/myetc", "javalib/myjar.jar", "lib64/mylib.so", - "app/AppFoo/AppFoo.apk", + "app/AppFoo@TEST.BUILD_ID/AppFoo.apk", "overlay/blue/rro.apk", "etc/bpf/bpf.o", "etc/bpf/bpf2.o", @@ -5682,8 +5683,8 @@ func TestApexWithApps(t *testing.T) { apexRule := module.Rule("apexRule") copyCmds := apexRule.Args["copy_commands"] - ensureContains(t, copyCmds, "image.apex/app/AppFoo/AppFoo.apk") - ensureContains(t, copyCmds, "image.apex/priv-app/AppFooPriv/AppFooPriv.apk") + ensureContains(t, copyCmds, "image.apex/app/AppFoo@TEST.BUILD_ID/AppFoo.apk") + ensureContains(t, copyCmds, "image.apex/priv-app/AppFooPriv@TEST.BUILD_ID/AppFooPriv.apk") appZipRule := ctx.ModuleForTests("AppFoo", "android_common_apex10000").Description("zip jni libs") // JNI libraries are uncompressed @@ -5700,6 +5701,36 @@ func TestApexWithApps(t *testing.T) { } } +func TestApexWithAppImportBuildId(t *testing.T) { + invalidBuildIds := []string{"../", "a b", "a/b", "a/b/../c", "/a"} + for _, id := range invalidBuildIds { + message := fmt.Sprintf("Unable to use build id %s as filename suffix", id) + fixture := android.FixtureModifyProductVariables(func(variables android.FixtureProductVariables) { + variables.BuildId = proptools.StringPtr(id) + }) + testApexError(t, message, `apex { + name: "myapex", + key: "myapex.key", + apps: ["AppFooPrebuilt"], + updatable: false, + } + + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + } + + android_app_import { + name: "AppFooPrebuilt", + apk: "PrebuiltAppFoo.apk", + presigned: true, + apex_available: ["myapex"], + } + `, fixture) + } +} + func TestApexWithAppImports(t *testing.T) { ctx := testApex(t, ` apex { @@ -5745,8 +5776,8 @@ func TestApexWithAppImports(t *testing.T) { apexRule := module.Rule("apexRule") copyCmds := apexRule.Args["copy_commands"] - ensureContains(t, copyCmds, "image.apex/app/AppFooPrebuilt/AppFooPrebuilt.apk") - ensureContains(t, copyCmds, "image.apex/priv-app/AppFooPrivPrebuilt/AwesomePrebuiltAppFooPriv.apk") + ensureContains(t, copyCmds, "image.apex/app/AppFooPrebuilt@TEST.BUILD_ID/AppFooPrebuilt.apk") + ensureContains(t, copyCmds, "image.apex/priv-app/AppFooPrivPrebuilt@TEST.BUILD_ID/AwesomePrebuiltAppFooPriv.apk") } func TestApexWithAppImportsPrefer(t *testing.T) { @@ -5787,7 +5818,7 @@ func TestApexWithAppImportsPrefer(t *testing.T) { })) ensureExactContents(t, ctx, "myapex", "android_common_myapex_image", []string{ - "app/AppFoo/AppFooPrebuilt.apk", + "app/AppFoo@TEST.BUILD_ID/AppFooPrebuilt.apk", }) } @@ -5820,7 +5851,7 @@ func TestApexWithTestHelperApp(t *testing.T) { apexRule := module.Rule("apexRule") copyCmds := apexRule.Args["copy_commands"] - ensureContains(t, copyCmds, "image.apex/app/TesterHelpAppFoo/TesterHelpAppFoo.apk") + ensureContains(t, copyCmds, "image.apex/app/TesterHelpAppFoo@TEST.BUILD_ID/TesterHelpAppFoo.apk") } func TestApexPropertiesShouldBeDefaultable(t *testing.T) { @@ -6263,8 +6294,8 @@ func TestOverrideApex(t *testing.T) { apexRule := module.Rule("apexRule") copyCmds := apexRule.Args["copy_commands"] - ensureNotContains(t, copyCmds, "image.apex/app/app/app.apk") - ensureContains(t, copyCmds, "image.apex/app/override_app/override_app.apk") + ensureNotContains(t, copyCmds, "image.apex/app/app@TEST.BUILD_ID/app.apk") + ensureContains(t, copyCmds, "image.apex/app/override_app@TEST.BUILD_ID/override_app.apk") ensureNotContains(t, copyCmds, "image.apex/etc/bpf/bpf.o") ensureContains(t, copyCmds, "image.apex/etc/bpf/override_bpf.o") @@ -7168,7 +7199,7 @@ func TestAppBundle(t *testing.T) { content := bundleConfigRule.Args["content"] ensureContains(t, content, `"compression":{"uncompressed_glob":["apex_payload.img","apex_manifest.*"]}`) - ensureContains(t, content, `"apex_config":{"apex_embedded_apk_config":[{"package_name":"com.android.foo","path":"app/AppFoo/AppFoo.apk"}]}`) + ensureContains(t, content, `"apex_config":{"apex_embedded_apk_config":[{"package_name":"com.android.foo","path":"app/AppFoo@TEST.BUILD_ID/AppFoo.apk"}]}`) } func TestAppSetBundle(t *testing.T) { @@ -7199,9 +7230,9 @@ func TestAppSetBundle(t *testing.T) { 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$") + ensureMatches(t, copyCmds[0], "^rm -rf .*/app/AppSet@TEST.BUILD_ID$") + ensureMatches(t, copyCmds[1], "^mkdir -p .*/app/AppSet@TEST.BUILD_ID$") + ensureMatches(t, copyCmds[2], "^unzip .*-d .*/app/AppSet@TEST.BUILD_ID .*/AppSet.zip$") } func TestAppSetBundlePrebuilt(t *testing.T) {