From d293e28f521f37d4e07c307090e5db829545d774 Mon Sep 17 00:00:00 2001 From: Karl Shaffer Date: Thu, 7 Sep 2023 00:49:34 +0000 Subject: [PATCH] Revert "Only allow setting presigned without preprocessed on targetSdk < 30" This reverts commit 6158528e152304f1259e31ff343358ff0f50c0e4. Reason for revert: DroidMonitor-triggered revert due to breakage https://android-build.corp.google.com/builds/quarterdeck?branch=git_udc-d1-dev-plus-aosp&target=aosp_bramble-trunk_staging-userdebug&lkgb=10771573&lkbb=10771600&fkbb=10771587, bug https://buganizer.corp.google.com/issues/299369971 BUG: 299369971 Change-Id: I6bf6eb5c0fb9e30197e145121adc7ed58871526f --- java/app_import.go | 30 +++------------------ java/app_import_test.go | 34 +++++++++++++++--------- java/base.go | 2 +- java/builder.go | 11 +------- java/dex.go | 2 +- java/hiddenapi.go | 2 +- scripts/check_target_sdk_less_than_30.py | 30 --------------------- 7 files changed, 30 insertions(+), 81 deletions(-) delete mode 100755 scripts/check_target_sdk_less_than_30.py diff --git a/java/app_import.go b/java/app_import.go index 1718d936c..ad1765e9d 100644 --- a/java/app_import.go +++ b/java/app_import.go @@ -277,14 +277,6 @@ func (a *AndroidAppImport) generateAndroidBuildActions(ctx android.ModuleContext a.hideApexVariantFromMake = true } - if Bool(a.properties.Preprocessed) { - if a.properties.Presigned != nil && !*a.properties.Presigned { - ctx.ModuleErrorf("Setting preprocessed: true implies presigned: true, so you cannot set presigned to false") - } - t := true - a.properties.Presigned = &t - } - numCertPropsSet := 0 if String(a.properties.Certificate) != "" { numCertPropsSet++ @@ -296,9 +288,11 @@ func (a *AndroidAppImport) generateAndroidBuildActions(ctx android.ModuleContext numCertPropsSet++ } if numCertPropsSet != 1 { - ctx.ModuleErrorf("One and only one of certficate, presigned (implied by preprocessed), and default_dev_cert properties must be set") + ctx.ModuleErrorf("One and only one of certficate, presigned, and default_dev_cert properties must be set") } + _, _, certificates := collectAppDeps(ctx, a, false, false) + // TODO: LOCAL_EXTRACT_APK/LOCAL_EXTRACT_DPI_APK // TODO: LOCAL_PACKAGE_SPLITS @@ -371,7 +365,6 @@ func (a *AndroidAppImport) generateAndroidBuildActions(ctx android.ModuleContext } 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. - _, _, certificates := collectAppDeps(ctx, a, false, false) a.certificate, certificates = processMainCert(a.ModuleBase, String(a.properties.Certificate), certificates, ctx) signed := android.PathForModuleOut(ctx, "signed", apkFilename) var lineageFile android.Path @@ -384,13 +377,8 @@ func (a *AndroidAppImport) generateAndroidBuildActions(ctx android.ModuleContext SignAppPackage(ctx, signed, jnisUncompressed, certificates, nil, lineageFile, rotationMinSdkVersion) a.outputFile = signed } else { - // Presigned without Preprocessed shouldn't really be a thing, currently we disallow - // it for apps with targetSdk >= 30, because on those targetSdks you must be using signature - // v2 or later, and signature v2 would be wrecked by uncompressing libs / zipaligning. - // But ideally we would disallow it for all prebuilt apks, and remove the presigned property. - targetSdkCheck := a.validateTargetSdkLessThan30(ctx, srcApk) alignedApk := android.PathForModuleOut(ctx, "zip-aligned", apkFilename) - TransformZipAlign(ctx, alignedApk, jnisUncompressed, []android.Path{targetSdkCheck}) + TransformZipAlign(ctx, alignedApk, jnisUncompressed) a.outputFile = alignedApk a.certificate = PresignedCertificate } @@ -444,16 +432,6 @@ func (a *AndroidAppImport) validatePreprocessedApk(ctx android.ModuleContext, sr }) } -func (a *AndroidAppImport) validateTargetSdkLessThan30(ctx android.ModuleContext, srcApk android.Path) android.Path { - alignmentStamp := android.PathForModuleOut(ctx, "validated-prebuilt", "old_target_sdk.stamp") - ctx.Build(pctx, android.BuildParams{ - Rule: checkBelowTargetSdk30ForNonPreprocessedApks, - Input: srcApk, - Output: alignmentStamp, - }) - return alignmentStamp -} - func (a *AndroidAppImport) Prebuilt() *android.Prebuilt { return &a.prebuilt } diff --git a/java/app_import_test.go b/java/app_import_test.go index 506c7344b..bb8fab93b 100644 --- a/java/app_import_test.go +++ b/java/app_import_test.go @@ -629,21 +629,31 @@ func TestAndroidTestImport_Preprocessed(t *testing.T) { presigned: true, preprocessed: true, } + + android_test_import { + name: "foo_cert", + apk: "prebuilts/apk/app.apk", + certificate: "cert/new_cert", + preprocessed: true, + } `) - apkName := "foo.apk" - variant := ctx.ModuleForTests("foo", "android_common") - jniRule := variant.Output("jnis-uncompressed/" + apkName).BuildParams.Rule.String() - if jniRule != android.Cp.String() { - t.Errorf("Unexpected JNI uncompress rule: " + jniRule) - } + testModules := []string{"foo", "foo_cert"} + for _, m := range testModules { + apkName := m + ".apk" + variant := ctx.ModuleForTests(m, "android_common") + jniRule := variant.Output("jnis-uncompressed/" + apkName).BuildParams.Rule.String() + if jniRule != android.Cp.String() { + t.Errorf("Unexpected JNI uncompress rule: " + jniRule) + } - // Make sure signing and aligning were skipped. - if variant.MaybeOutput("signed/"+apkName).Rule != nil { - t.Errorf("signing rule shouldn't be included for preprocessed.") - } - if variant.MaybeOutput("zip-aligned/"+apkName).Rule != nil { - t.Errorf("aligning rule shouldn't be for preprocessed") + // Make sure signing and aligning were skipped. + if variant.MaybeOutput("signed/"+apkName).Rule != nil { + t.Errorf("signing rule shouldn't be included for preprocessed.") + } + if variant.MaybeOutput("zip-aligned/"+apkName).Rule != nil { + t.Errorf("aligning rule shouldn't be for preprocessed") + } } } diff --git a/java/base.go b/java/base.go index 8f483989e..2c0b3ea7c 100644 --- a/java/base.go +++ b/java/base.go @@ -1608,7 +1608,7 @@ func (j *Module) compile(ctx android.ModuleContext, extraSrcJars, extraClasspath false, nil, nil) if *j.dexProperties.Uncompress_dex { combinedAlignedJar := android.PathForModuleOut(ctx, "dex-withres-aligned", jarName).OutputPath - TransformZipAlign(ctx, combinedAlignedJar, combinedJar, nil) + TransformZipAlign(ctx, combinedAlignedJar, combinedJar) dexOutputFile = combinedAlignedJar } else { dexOutputFile = combinedJar diff --git a/java/builder.go b/java/builder.go index bf919175d..debf49a00 100644 --- a/java/builder.go +++ b/java/builder.go @@ -277,14 +277,6 @@ var ( Command: `${config.Zip2ZipCmd} -i ${in} -o ${out} -x 'META-INF/services/**/*'`, CommandDeps: []string{"${config.Zip2ZipCmd}"}, }) - - checkBelowTargetSdk30ForNonPreprocessedApks = pctx.AndroidStaticRule("checkBelowTargetSdk30ForNonPreprocessedApks", - blueprint.RuleParams{ - Command: "build/soong/scripts/check_target_sdk_less_than_30.py ${config.Aapt2Cmd} $in $out", - CommandDeps: []string{"build/soong/scripts/check_target_sdk_less_than_30.py", "${config.Aapt2Cmd}"}, - Description: "Check prebuilt target sdk version", - }, - ) ) func init() { @@ -697,13 +689,12 @@ func GenerateMainClassManifest(ctx android.ModuleContext, outputFile android.Wri android.WriteFileRule(ctx, outputFile, "Main-Class: "+mainClass+"\n") } -func TransformZipAlign(ctx android.ModuleContext, outputFile android.WritablePath, inputFile android.Path, validations android.Paths) { +func TransformZipAlign(ctx android.ModuleContext, outputFile android.WritablePath, inputFile android.Path) { ctx.Build(pctx, android.BuildParams{ Rule: zipalign, Description: "align", Input: inputFile, Output: outputFile, - Validations: validations, }) } diff --git a/java/dex.go b/java/dex.go index 511ae5eb3..df501bffa 100644 --- a/java/dex.go +++ b/java/dex.go @@ -438,7 +438,7 @@ func (d *dexer) compileDex(ctx android.ModuleContext, dexParams *compileDexParam } if proptools.Bool(d.dexProperties.Uncompress_dex) { alignedJavalibJar := android.PathForModuleOut(ctx, "aligned", dexParams.jarName).OutputPath - TransformZipAlign(ctx, alignedJavalibJar, javalibJar, nil) + TransformZipAlign(ctx, alignedJavalibJar, javalibJar) javalibJar = alignedJavalibJar } diff --git a/java/hiddenapi.go b/java/hiddenapi.go index fe3fe7b61..4d08b8307 100644 --- a/java/hiddenapi.go +++ b/java/hiddenapi.go @@ -305,7 +305,7 @@ func hiddenAPIEncodeDex(ctx android.ModuleContext, dexInput, flagsCSV android.Pa }) if uncompressDex { - TransformZipAlign(ctx, output, encodeRuleOutput, nil) + TransformZipAlign(ctx, output, encodeRuleOutput) } return output diff --git a/scripts/check_target_sdk_less_than_30.py b/scripts/check_target_sdk_less_than_30.py deleted file mode 100755 index 69b0bf011..000000000 --- a/scripts/check_target_sdk_less_than_30.py +++ /dev/null @@ -1,30 +0,0 @@ -#!/usr/bin/env python3 - -import subprocess -import argparse -import re -import sys - -def main(): - parser = argparse.ArgumentParser() - parser.add_argument('aapt2', help = "the path to the aapt2 executable") - parser.add_argument('apk', help = "the apk to check") - parser.add_argument('stampfile', help = "a file to touch if successful") - args = parser.parse_args() - - regex = re.compile(r"targetSdkVersion: *'([0-9]+)'") - output = subprocess.check_output([args.aapt2, "dump", "badging", args.apk], text=True) - targetSdkVersion = None - for line in output.splitlines(): - match = regex.fullmatch(line.strip()) - if match: - targetSdkVersion = int(match.group(1)) - break - - if targetSdkVersion is None or targetSdkVersion >= 30: - sys.exit(args.apk + ": Prebuilt, presigned apks with targetSdkVersion >= 30 (or a codename targetSdkVersion) must set preprocessed: true in the Android.bp definition (because they must be signed with signature v2, and the build system would wreck that signature otherwise)") - - subprocess.check_call(["touch", args.stampfile]) - -if __name__ == "__main__": - main()