From ce320f86a084cb29d5f4e24aee221e9e4dd4fb47 Mon Sep 17 00:00:00 2001
From: Jihoon Kang <jihoonkang@google.com>
Date: Tue, 7 May 2024 00:53:28 +0000
Subject: [PATCH] Pass --non-updatable-system flag to aapt2 when versionCode is
 unspecified

This change modifies the flags passed to aapt2 when generating the APKs.
Currently, the version code of the platform sdk version is passed to
aapt2 when the bp module definition does not explicitly specify the
`--version-code` flag in "aaptflags" parameter. This change modifies
such behavior so that the newly introduced `--non-updatable-system` flag
is passed instead of implicitly passing the `--version-code`.

If "versionCode" is explicitly specified in the app manifest, the
`--non-updatable-system` flag is overriden and is a no-op. This way, the
build continues to stay agnostic to the content of the manifest files.

This flag is not passed for build actions of android_test modules, as
test targets do not set `versionCode`.

Test: m nothing --no-skip-soong-tests &&  manually inspect aapt2 build rules
Bug: 311724570
Change-Id: Ie3e50506d90da1d28b8039e29d76859b1927b5e2
---
 java/aar.go      | 10 +++++++---
 java/app.go      |  2 ++
 java/app_test.go | 51 +++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/java/aar.go b/java/aar.go
index 07392f6e5..1d05e4816 100644
--- a/java/aar.go
+++ b/java/aar.go
@@ -107,6 +107,10 @@ type aaptProperties struct {
 
 	// Names of aconfig_declarations modules that specify aconfig flags that the module depends on.
 	Flags_packages []string
+
+	// If set, `--non-updatable-system` flag is not passed to aapt2 even when
+	// `--version-code` is not specified in the `aaptFlags` property list or in the manifest.
+	DisableNonUpdatableSystem bool `blueprint:"mutated"`
 }
 
 type aapt struct {
@@ -308,9 +312,9 @@ func (a *aapt) aapt2Flags(ctx android.ModuleContext, sdkContext android.SdkConte
 	// This behavior has been copied from Make.
 	linkFlags = append(linkFlags, "--target-sdk-version "+minSdkVersion)
 
-	// Version code
-	if !hasVersionCode {
-		linkFlags = append(linkFlags, "--version-code", ctx.Config().PlatformSdkVersion().String())
+	// Mark non updatable when the module does not specify a version code
+	if !a.aaptProperties.DisableNonUpdatableSystem && !hasVersionCode {
+		linkFlags = append(linkFlags, "--non-updatable-system")
 	}
 
 	if !hasVersionName {
diff --git a/java/app.go b/java/app.go
index a24099c28..cd30dc126 100644
--- a/java/app.go
+++ b/java/app.go
@@ -1452,6 +1452,7 @@ func AndroidTestFactory() android.Module {
 	module.appProperties.AlwaysPackageNativeLibs = true
 	module.Module.dexpreopter.isTest = true
 	module.Module.linter.properties.Lint.Test = proptools.BoolPtr(true)
+	module.aaptProperties.DisableNonUpdatableSystem = true
 
 	module.addHostAndDeviceProperties()
 	module.AddProperties(
@@ -1508,6 +1509,7 @@ func AndroidTestHelperAppFactory() android.Module {
 	module.appProperties.AlwaysPackageNativeLibs = true
 	module.Module.dexpreopter.isTest = true
 	module.Module.linter.properties.Lint.Test = proptools.BoolPtr(true)
+	module.aaptProperties.DisableNonUpdatableSystem = true
 
 	module.addHostAndDeviceProperties()
 	module.AddProperties(
diff --git a/java/app_test.go b/java/app_test.go
index 804949435..988a5b17d 100644
--- a/java/app_test.go
+++ b/java/app_test.go
@@ -4477,5 +4477,54 @@ func TestAppMinSdkVersionOverride(t *testing.T) {
 		fooOverride.BuildParams.Args["args"],
 		"--minSdkVersion  33",
 	)
-
+}
+
+func TestNonUpdatableSystem(t *testing.T) {
+	ctx := testApp(t, `
+		android_app {
+			name: "foo",
+			srcs: ["a.java"],
+			sdk_version: "current",
+			aaptflags: [
+				"--version-code 1",
+			],
+		}
+		android_app {
+			name: "bar",
+			srcs: ["a.java"],
+			sdk_version: "current",
+		}
+		android_test {
+			name: "baz",
+			srcs: ["a.java"],
+			sdk_version: "current",
+		}
+		android_test_helper_app {
+			name: "baz_helper",
+			srcs: ["a.java"],
+			sdk_version: "current",
+		}
+	`)
+
+	hasNonUpdatableSystemFlag := func(module android.TestingModule) bool {
+		moduleAapt2LinkRule := module.Rule("android/soong/java.aapt2Link")
+		linkFlags := moduleAapt2LinkRule.Args["flags"]
+		return strings.Contains(linkFlags, "--non-updatable-system")
+	}
+
+	foo := ctx.ModuleForTests("foo", "android_common")
+	android.AssertBoolEquals(t, "app should not pass `--non-updatable-flags` when --version-code is specified",
+		false, hasNonUpdatableSystemFlag(foo))
+
+	bar := ctx.ModuleForTests("bar", "android_common")
+	android.AssertBoolEquals(t, "app should pass `--non-updatable-flags` when --version-code is not specified",
+		true, hasNonUpdatableSystemFlag(bar))
+
+	baz := ctx.ModuleForTests("baz", "android_common")
+	android.AssertBoolEquals(t, "test should not pass `--non-updatable-flags` even if --version-code is not specified",
+		false, hasNonUpdatableSystemFlag(baz))
+
+	baz_helper := ctx.ModuleForTests("baz_helper", "android_common")
+	android.AssertBoolEquals(t, "test should not pass `--non-updatable-flags` even if --version-code is not specified",
+		false, hasNonUpdatableSystemFlag(baz_helper))
 }