From 5c4791c71e613ef607aaa07a931fd966e45d233d Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 1 Feb 2019 11:44:44 -0800 Subject: [PATCH 1/3] Move TestEnforceRRO test cases into test function The test cases will need to reference buildDir, which is not yet set at global variable initialization time. Bug: 123510624 Test: TestEnforceRRO Change-Id: I0dda0184dfab496c820e11ed76b7594a60d5d587 --- java/app_test.go | 124 +++++++++++++++++++++++------------------------ 1 file changed, 62 insertions(+), 62 deletions(-) diff --git a/java/app_test.go b/java/app_test.go index 2455145fa..e3b78e4e0 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -106,68 +106,68 @@ func TestApp(t *testing.T) { } } -var testEnforceRROTests = []struct { - name string - enforceRROTargets []string - enforceRROExcludedOverlays []string - overlayFiles map[string][]string - rroDirs map[string][]string -}{ - { - name: "no RRO", - enforceRROTargets: nil, - enforceRROExcludedOverlays: nil, - overlayFiles: map[string][]string{ - "foo": []string{ - "device/vendor/blah/static_overlay/foo/res/values/strings.xml", - "device/vendor/blah/overlay/foo/res/values/strings.xml", - }, - "bar": []string{ - "device/vendor/blah/static_overlay/bar/res/values/strings.xml", - "device/vendor/blah/overlay/bar/res/values/strings.xml", - }, - }, - rroDirs: map[string][]string{ - "foo": nil, - "bar": nil, - }, - }, - { - name: "enforce RRO on foo", - enforceRROTargets: []string{"foo"}, - enforceRROExcludedOverlays: []string{"device/vendor/blah/static_overlay"}, - overlayFiles: map[string][]string{ - "foo": []string{"device/vendor/blah/static_overlay/foo/res/values/strings.xml"}, - "bar": []string{ - "device/vendor/blah/static_overlay/bar/res/values/strings.xml", - "device/vendor/blah/overlay/bar/res/values/strings.xml", - }, - }, - rroDirs: map[string][]string{ - "foo": []string{"device/vendor/blah/overlay/foo/res"}, - "bar": nil, - }, - }, - { - name: "enforce RRO on all", - enforceRROTargets: []string{"*"}, - enforceRROExcludedOverlays: []string{ - // Excluding specific apps/res directories also allowed. - "device/vendor/blah/static_overlay/foo", - "device/vendor/blah/static_overlay/bar/res", - }, - overlayFiles: map[string][]string{ - "foo": []string{"device/vendor/blah/static_overlay/foo/res/values/strings.xml"}, - "bar": []string{"device/vendor/blah/static_overlay/bar/res/values/strings.xml"}, - }, - rroDirs: map[string][]string{ - "foo": []string{"device/vendor/blah/overlay/foo/res"}, - "bar": []string{"device/vendor/blah/overlay/bar/res"}, - }, - }, -} - func TestEnforceRRO(t *testing.T) { + testCases := []struct { + name string + enforceRROTargets []string + enforceRROExcludedOverlays []string + overlayFiles map[string][]string + rroDirs map[string][]string + }{ + { + name: "no RRO", + enforceRROTargets: nil, + enforceRROExcludedOverlays: nil, + overlayFiles: map[string][]string{ + "foo": []string{ + "device/vendor/blah/static_overlay/foo/res/values/strings.xml", + "device/vendor/blah/overlay/foo/res/values/strings.xml", + }, + "bar": []string{ + "device/vendor/blah/static_overlay/bar/res/values/strings.xml", + "device/vendor/blah/overlay/bar/res/values/strings.xml", + }, + }, + rroDirs: map[string][]string{ + "foo": nil, + "bar": nil, + }, + }, + { + name: "enforce RRO on foo", + enforceRROTargets: []string{"foo"}, + enforceRROExcludedOverlays: []string{"device/vendor/blah/static_overlay"}, + overlayFiles: map[string][]string{ + "foo": []string{"device/vendor/blah/static_overlay/foo/res/values/strings.xml"}, + "bar": []string{ + "device/vendor/blah/static_overlay/bar/res/values/strings.xml", + "device/vendor/blah/overlay/bar/res/values/strings.xml", + }, + }, + rroDirs: map[string][]string{ + "foo": []string{"device/vendor/blah/overlay/foo/res"}, + "bar": nil, + }, + }, + { + name: "enforce RRO on all", + enforceRROTargets: []string{"*"}, + enforceRROExcludedOverlays: []string{ + // Excluding specific apps/res directories also allowed. + "device/vendor/blah/static_overlay/foo", + "device/vendor/blah/static_overlay/bar/res", + }, + overlayFiles: map[string][]string{ + "foo": []string{"device/vendor/blah/static_overlay/foo/res/values/strings.xml"}, + "bar": []string{"device/vendor/blah/static_overlay/bar/res/values/strings.xml"}, + }, + rroDirs: map[string][]string{ + "foo": []string{"device/vendor/blah/overlay/foo/res"}, + "bar": []string{"device/vendor/blah/overlay/bar/res"}, + }, + }, + } + resourceOverlays := []string{ "device/vendor/blah/overlay", "device/vendor/blah/overlay2", @@ -196,7 +196,7 @@ func TestEnforceRRO(t *testing.T) { } ` - for _, testCase := range testEnforceRROTests { + for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { config := testConfig(nil) config.TestProductVariables.ResourceOverlays = resourceOverlays From 6ed7deaf33135dc2f90e99a59cd822c58e5b9035 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 31 Jan 2019 14:44:30 -0800 Subject: [PATCH 2/3] Add a static lib to TestEnforceRRO Add a static lib dependency to TestEnforceRRO in preparation for capturing static dependencies in rroDirs. Bug: 123510624 Test: TestEnforceRRO Change-Id: I9754ebf02866e8b3e4ad0c55ff099e546f8e2bc2 --- java/app_test.go | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/java/app_test.go b/java/app_test.go index e3b78e4e0..6bbaac09b 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -120,6 +120,8 @@ func TestEnforceRRO(t *testing.T) { enforceRROExcludedOverlays: nil, overlayFiles: map[string][]string{ "foo": []string{ + buildDir + "/.intermediates/lib/android_common/package-res.apk", + "foo/res/res/values/strings.xml", "device/vendor/blah/static_overlay/foo/res/values/strings.xml", "device/vendor/blah/overlay/foo/res/values/strings.xml", }, @@ -138,7 +140,11 @@ func TestEnforceRRO(t *testing.T) { enforceRROTargets: []string{"foo"}, enforceRROExcludedOverlays: []string{"device/vendor/blah/static_overlay"}, overlayFiles: map[string][]string{ - "foo": []string{"device/vendor/blah/static_overlay/foo/res/values/strings.xml"}, + "foo": []string{ + buildDir + "/.intermediates/lib/android_common/package-res.apk", + "foo/res/res/values/strings.xml", + "device/vendor/blah/static_overlay/foo/res/values/strings.xml", + }, "bar": []string{ "device/vendor/blah/static_overlay/bar/res/values/strings.xml", "device/vendor/blah/overlay/bar/res/values/strings.xml", @@ -158,7 +164,11 @@ func TestEnforceRRO(t *testing.T) { "device/vendor/blah/static_overlay/bar/res", }, overlayFiles: map[string][]string{ - "foo": []string{"device/vendor/blah/static_overlay/foo/res/values/strings.xml"}, + "foo": []string{ + buildDir + "/.intermediates/lib/android_common/package-res.apk", + "foo/res/res/values/strings.xml", + "device/vendor/blah/static_overlay/foo/res/values/strings.xml", + }, "bar": []string{"device/vendor/blah/static_overlay/bar/res/values/strings.xml"}, }, rroDirs: map[string][]string{ @@ -177,8 +187,10 @@ func TestEnforceRRO(t *testing.T) { fs := map[string][]byte{ "foo/res/res/values/strings.xml": nil, "bar/res/res/values/strings.xml": nil, + "lib/res/res/values/strings.xml": nil, "device/vendor/blah/overlay/foo/res/values/strings.xml": nil, "device/vendor/blah/overlay/bar/res/values/strings.xml": nil, + "device/vendor/blah/overlay/lib/res/values/strings.xml": nil, "device/vendor/blah/static_overlay/foo/res/values/strings.xml": nil, "device/vendor/blah/static_overlay/bar/res/values/strings.xml": nil, "device/vendor/blah/overlay2/res/values/strings.xml": nil, @@ -188,12 +200,18 @@ func TestEnforceRRO(t *testing.T) { android_app { name: "foo", resource_dirs: ["foo/res"], + static_libs: ["lib"], } android_app { name: "bar", resource_dirs: ["bar/res"], } + + android_library { + name: "lib", + resource_dirs: ["lib/res"], + } ` for _, testCase := range testCases { @@ -216,7 +234,15 @@ func TestEnforceRRO(t *testing.T) { var overlayFiles []string if overlayFile.Rule != nil { for _, o := range overlayFile.Inputs.Strings() { - overlayFiles = append(overlayFiles, module.Output(o).Inputs.Strings()...) + overlayOutput := module.MaybeOutput(o) + if overlayOutput.Rule != nil { + // If the overlay is compiled as part of this module (i.e. a .arsc.flat file), + // verify the inputs to the .arsc.flat rule. + overlayFiles = append(overlayFiles, overlayOutput.Inputs.Strings()...) + } else { + // Otherwise, verify the full path to the output of the other module + overlayFiles = append(overlayFiles, o) + } } } From c1c3755b3912bf99b2b345c667becb5a6ecd3e03 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 31 Jan 2019 11:42:41 -0800 Subject: [PATCH 3/3] Export RRO resource dirs from static android_library dependencies RRO dirs from static android_library dependencies should be included in the final module. Bug: 123510624 Test: TestEnforceRRO Change-Id: I28c45e139b187894a4ebc43d573eab5ea1be9861 --- java/aar.go | 30 ++++++++++++++++++++++-------- java/app.go | 4 ---- java/app_test.go | 12 ++++++++++-- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/java/aar.go b/java/aar.go index d08e48761..b96f1272b 100644 --- a/java/aar.go +++ b/java/aar.go @@ -26,6 +26,7 @@ type AndroidLibraryDependency interface { Dependency ExportPackage() android.Path ExportedProguardFlagFiles() android.Paths + ExportedRRODirs() android.Paths ExportedStaticPackages() android.Paths ExportedManifest() android.Path } @@ -80,6 +81,14 @@ func (a *aapt) ExportPackage() android.Path { return a.exportPackage } +func (a *aapt) ExportedRRODirs() android.Paths { + return a.rroDirs +} + +func (a *aapt) ExportedManifest() android.Path { + return a.manifestPath +} + func (a *aapt) aapt2Flags(ctx android.ModuleContext, sdkContext sdkContext, manifestPath android.Path) (flags []string, deps android.Paths, resDirs, overlayDirs []globbedResourceDir, rroDirs android.Paths) { @@ -164,7 +173,7 @@ func (a *aapt) deps(ctx android.BottomUpMutatorContext, sdkContext sdkContext) { } func (a *aapt) buildActions(ctx android.ModuleContext, sdkContext sdkContext, extraLinkFlags ...string) { - transitiveStaticLibs, staticLibManifests, libDeps, libFlags := aaptLibs(ctx, sdkContext) + transitiveStaticLibs, staticLibManifests, staticRRODirs, libDeps, libFlags := aaptLibs(ctx, sdkContext) // App manifest file manifestFile := proptools.StringDefault(a.aaptProperties.Manifest, "AndroidManifest.xml") @@ -174,6 +183,8 @@ func (a *aapt) buildActions(ctx android.ModuleContext, sdkContext sdkContext, ex linkFlags, linkDeps, resDirs, overlayDirs, rroDirs := a.aapt2Flags(ctx, sdkContext, manifestPath) + rroDirs = append(rroDirs, staticRRODirs...) + linkFlags = append(linkFlags, libFlags...) linkDeps = append(linkDeps, libDeps...) linkFlags = append(linkFlags, extraLinkFlags...) @@ -235,7 +246,7 @@ func (a *aapt) buildActions(ctx android.ModuleContext, sdkContext sdkContext, ex // aaptLibs collects libraries from dependencies and sdk_version and converts them into paths func aaptLibs(ctx android.ModuleContext, sdkContext sdkContext) (transitiveStaticLibs, staticLibManifests, - deps android.Paths, flags []string) { + staticRRODirs, deps android.Paths, flags []string) { var sharedLibs android.Paths @@ -263,6 +274,7 @@ func aaptLibs(ctx android.ModuleContext, sdkContext sdkContext) (transitiveStati transitiveStaticLibs = append(transitiveStaticLibs, exportPackage) transitiveStaticLibs = append(transitiveStaticLibs, aarDep.ExportedStaticPackages()...) staticLibManifests = append(staticLibManifests, aarDep.ExportedManifest()) + staticRRODirs = append(staticRRODirs, aarDep.ExportedRRODirs()...) } } }) @@ -279,8 +291,9 @@ func aaptLibs(ctx android.ModuleContext, sdkContext sdkContext) (transitiveStati } transitiveStaticLibs = android.FirstUniquePaths(transitiveStaticLibs) + staticRRODirs = android.FirstUniquePaths(staticRRODirs) - return transitiveStaticLibs, staticLibManifests, deps, flags + return transitiveStaticLibs, staticLibManifests, staticRRODirs, deps, flags } type AndroidLibrary struct { @@ -303,10 +316,6 @@ func (a *AndroidLibrary) ExportedStaticPackages() android.Paths { return a.exportedStaticPackages } -func (a *AndroidLibrary) ExportedManifest() android.Path { - return a.manifestPath -} - var _ AndroidLibraryDependency = (*AndroidLibrary)(nil) func (a *AndroidLibrary) DepsMutator(ctx android.BottomUpMutatorContext) { @@ -426,6 +435,10 @@ func (a *AARImport) ExportedProguardFlagFiles() android.Paths { return android.Paths{a.proguardFlags} } +func (a *AARImport) ExportedRRODirs() android.Paths { + return nil +} + func (a *AARImport) ExportedStaticPackages() android.Paths { return a.exportedStaticPackages } @@ -518,9 +531,10 @@ func (a *AARImport) GenerateAndroidBuildActions(ctx android.ModuleContext) { linkFlags = append(linkFlags, "--manifest "+a.manifest.String()) linkDeps = append(linkDeps, a.manifest) - transitiveStaticLibs, staticLibManifests, libDeps, libFlags := aaptLibs(ctx, sdkContext(a)) + transitiveStaticLibs, staticLibManifests, staticRRODirs, libDeps, libFlags := aaptLibs(ctx, sdkContext(a)) _ = staticLibManifests + _ = staticRRODirs linkDeps = append(linkDeps, libDeps...) linkFlags = append(linkFlags, libFlags...) diff --git a/java/app.go b/java/app.go index e9c12e0a7..08714f2c9 100644 --- a/java/app.go +++ b/java/app.go @@ -95,10 +95,6 @@ func (a *AndroidApp) ExportedStaticPackages() android.Paths { return nil } -func (a *AndroidApp) ExportedManifest() android.Path { - return a.manifestPath -} - var _ AndroidLibraryDependency = (*AndroidApp)(nil) type Certificate struct { diff --git a/java/app_test.go b/java/app_test.go index 6bbaac09b..93d20d09a 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -150,8 +150,13 @@ func TestEnforceRRO(t *testing.T) { "device/vendor/blah/overlay/bar/res/values/strings.xml", }, }, + rroDirs: map[string][]string{ - "foo": []string{"device/vendor/blah/overlay/foo/res"}, + "foo": []string{ + "device/vendor/blah/overlay/foo/res", + // Enforce RRO on "foo" could imply RRO on static dependencies, but for now it doesn't. + // "device/vendor/blah/overlay/lib/res", + }, "bar": nil, }, }, @@ -172,7 +177,10 @@ func TestEnforceRRO(t *testing.T) { "bar": []string{"device/vendor/blah/static_overlay/bar/res/values/strings.xml"}, }, rroDirs: map[string][]string{ - "foo": []string{"device/vendor/blah/overlay/foo/res"}, + "foo": []string{ + "device/vendor/blah/overlay/foo/res", + "device/vendor/blah/overlay/lib/res", + }, "bar": []string{"device/vendor/blah/overlay/bar/res"}, }, },