Revert^2 "Only allow setting presigned without preprocessed on targetSdk < 30"

This reverts commit d293e28f52.

Reason for revert: The underlying issue was fixed in ag/24685010

Change-Id: I06810d37dba37aa12f9a1e14b0749f1e1eb41136

java/app_import.go

@@ -277,6 +277,14 @@ 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++
@@ -288,11 +296,9 @@ func (a *AndroidAppImport) generateAndroidBuildActions(ctx android.ModuleContext
 		numCertPropsSet++
 	}
 	if numCertPropsSet != 1 {
-		ctx.ModuleErrorf("One and only one of certficate, presigned, and default_dev_cert properties must be set")
+		ctx.ModuleErrorf("One and only one of certficate, presigned (implied by preprocessed), 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
 
@@ -365,6 +371,7 @@ 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
@@ -377,8 +384,13 @@ 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)
+		TransformZipAlign(ctx, alignedApk, jnisUncompressed, []android.Path{targetSdkCheck})
 		a.outputFile = alignedApk
 		a.certificate = PresignedCertificate
 	}
@@ -432,6 +444,16 @@ 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
 }

java/app_import_test.go

@@ -629,31 +629,21 @@ 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,
-		}
 		`)
 
-	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)
-		}
+	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)
+	}
 
-		// 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")
 	}
 }
 

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)
+					TransformZipAlign(ctx, combinedAlignedJar, combinedJar, nil)
 					dexOutputFile = combinedAlignedJar
 				} else {
 					dexOutputFile = combinedJar

java/builder.go

@@ -277,6 +277,14 @@ 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() {
@@ -689,12 +697,13 @@ 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) {
+func TransformZipAlign(ctx android.ModuleContext, outputFile android.WritablePath, inputFile android.Path, validations android.Paths) {
 	ctx.Build(pctx, android.BuildParams{
 		Rule:        zipalign,
 		Description: "align",
 		Input:       inputFile,
 		Output:      outputFile,
+		Validations: validations,
 	})
 }
 

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)
+		TransformZipAlign(ctx, alignedJavalibJar, javalibJar, nil)
 		javalibJar = alignedJavalibJar
 	}
 

java/hiddenapi.go

@@ -305,7 +305,7 @@ func hiddenAPIEncodeDex(ctx android.ModuleContext, dexInput, flagsCSV android.Pa
 	})
 
 	if uncompressDex {
-		TransformZipAlign(ctx, output, encodeRuleOutput)
+		TransformZipAlign(ctx, output, encodeRuleOutput, nil)
 	}
 
 	return output

scripts/check_target_sdk_less_than_30.py

@@ -0,0 +1,30 @@
+#!/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()