From 15809f8101dc408512fc63215ac1d4131cea2feb Mon Sep 17 00:00:00 2001
From: Sam Delmerico <delmerico@google.com>
Date: Mon, 15 May 2023 17:21:47 -0400
Subject: [PATCH] don't require package_name for non-override android_apps

For a non-override android_app, we can assume that the privapp_allowlist
already contains the correct package_name, and so we don't need to
overwrite it in this case.

Bug: 242509786
Test: go test
Merged-In: I0f137e34cae3478dc8b9178d138121ff1d936f07
Change-Id: I0f137e34cae3478dc8b9178d138121ff1d936f07
---
 apex/apex_test.go |  3 +--
 java/app.go       | 10 +++++++++-
 java/app_test.go  | 18 ++++++------------
 3 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/apex/apex_test.go b/apex/apex_test.go
index c1d80a39f..2dad22b06 100644
--- a/apex/apex_test.go
+++ b/apex/apex_test.go
@@ -6260,8 +6260,7 @@ func TestApexWithApps(t *testing.T) {
 			sdk_version: "current",
 			system_modules: "none",
 			privileged: true,
-			privapp_allowlist: "perms.xml",
-			package_name: "com.android.AppFooPriv",
+			privapp_allowlist: "privapp_allowlist_com.android.AppFooPriv.xml",
 			stl: "none",
 			apex_available: [ "myapex" ],
 		}
diff --git a/java/app.go b/java/app.go
index 366005ce3..d0bde6f22 100755
--- a/java/app.go
+++ b/java/app.go
@@ -621,13 +621,21 @@ func (a *AndroidApp) InstallApkName() string {
 	return a.installApkName
 }
 
-func (a *AndroidApp) createPrivappAllowlist(ctx android.ModuleContext) *android.OutputPath {
+func (a *AndroidApp) createPrivappAllowlist(ctx android.ModuleContext) android.Path {
 	if a.appProperties.Privapp_allowlist == nil {
 		return nil
 	}
+
+	isOverrideApp := a.GetOverriddenBy() != ""
+	if !isOverrideApp {
+		// if this is not an override, we don't need to rewrite the existing privapp allowlist
+		return android.PathForModuleSrc(ctx, *a.appProperties.Privapp_allowlist)
+	}
+
 	if a.overridableAppProperties.Package_name == nil {
 		ctx.PropertyErrorf("privapp_allowlist", "package_name must be set to use privapp_allowlist")
 	}
+
 	packageName := *a.overridableAppProperties.Package_name
 	fileName := "privapp_allowlist_" + packageName + ".xml"
 	outPath := android.PathForModuleOut(ctx, fileName).OutputPath
diff --git a/java/app_test.go b/java/app_test.go
index c485478eb..4b78c48a5 100644
--- a/java/app_test.go
+++ b/java/app_test.go
@@ -3563,9 +3563,8 @@ func TestPrivappAllowlist(t *testing.T) {
 		android_app {
 			name: "foo",
 			srcs: ["a.java"],
-			privapp_allowlist: "perms.xml",
+			privapp_allowlist: "privapp_allowlist_com.android.foo.xml",
 			privileged: true,
-			package_name: "com.android.foo",
 			sdk_version: "current",
 		}
 		override_android_app {
@@ -3578,17 +3577,12 @@ func TestPrivappAllowlist(t *testing.T) {
 	app := result.ModuleForTests("foo", "android_common")
 	overrideApp := result.ModuleForTests("foo", "android_common_bar")
 
-	// verify that privapp allowlist is created
-	app.Output("out/soong/.intermediates/foo/android_common/privapp_allowlist_com.android.foo.xml")
+	// verify that privapp allowlist is created for override apps
 	overrideApp.Output("out/soong/.intermediates/foo/android_common_bar/privapp_allowlist_com.google.android.foo.xml")
-	expectedAllowlist := "perms.xml"
-	actualAllowlist := app.Rule("modifyAllowlist").Input.String()
-	if expectedAllowlist != actualAllowlist {
-		t.Errorf("expected allowlist to be %q; got %q", expectedAllowlist, actualAllowlist)
-	}
-	overrideActualAllowlist := overrideApp.Rule("modifyAllowlist").Input.String()
-	if expectedAllowlist != overrideActualAllowlist {
-		t.Errorf("expected override allowlist to be %q; got %q", expectedAllowlist, overrideActualAllowlist)
+	expectedAllowlistInput := "privapp_allowlist_com.android.foo.xml"
+	overrideActualAllowlistInput := overrideApp.Rule("modifyAllowlist").Input.String()
+	if expectedAllowlistInput != overrideActualAllowlistInput {
+		t.Errorf("expected override allowlist to be %q; got %q", expectedAllowlistInput, overrideActualAllowlistInput)
 	}
 
 	// verify that permissions are copied to device