Convert RRO enforcement to transition mutator
Convert propagateRROEnforcementMutator to a transition mutator.
Bug: 367784740
Test: TestEnforceRRO_propagatesToDependencies
Test: TestAndroidResourceOverlays
Flag: EXEMPT refactor
Change-Id: I06c887f7203f3961f57bcaf458a191813e167561
diff --git a/java/aar.go b/java/aar.go
index b5e24c4..7d73b03 100644
--- a/java/aar.go
+++ b/java/aar.go
@@ -45,7 +45,7 @@
ctx.RegisterModuleType("android_library_import", AARImportFactory)
ctx.RegisterModuleType("android_library", AndroidLibraryFactory)
ctx.PostDepsMutators(func(ctx android.RegisterMutatorsContext) {
- ctx.TopDown("propagate_rro_enforcement", propagateRROEnforcementMutator)
+ ctx.Transition("propagate_rro_enforcement", &propagateRROEnforcementTransitionMutator{})
})
}
@@ -151,15 +151,67 @@
path android.Path
}
-// Propagate RRO enforcement flag to static lib dependencies transitively.
-func propagateRROEnforcementMutator(ctx android.TopDownMutatorContext) {
+// Propagate RRO enforcement flag to static lib dependencies transitively. If EnforceRROGlobally is set then
+// all modules will use the "" variant. If specific modules have RRO enforced, then modules (usually apps) with
+// RRO enabled will use the "" variation for themselves, but use the "rro" variant of direct and transitive static
+// android_library dependencies.
+type propagateRROEnforcementTransitionMutator struct{}
+
+func (p propagateRROEnforcementTransitionMutator) Split(ctx android.BaseModuleContext) []string {
+ // Never split modules, apps with or without RRO enabled use the "" variant, static android_library dependencies
+ // will use create the "rro" variant from incoming tranisitons.
+ return []string{""}
+}
+
+func (p propagateRROEnforcementTransitionMutator) OutgoingTransition(ctx android.OutgoingTransitionContext, sourceVariation string) string {
+ // Non-static dependencies are not involved in RRO and always use the empty variant.
+ if ctx.DepTag() != staticLibTag {
+ return ""
+ }
+
m := ctx.Module()
- if d, ok := m.(AndroidLibraryDependency); ok && d.IsRROEnforced(ctx) {
- ctx.VisitDirectDepsWithTag(staticLibTag, func(d android.Module) {
- if a, ok := d.(AndroidLibraryDependency); ok {
- a.SetRROEnforcedForDependent(true)
- }
- })
+ if _, ok := m.(AndroidLibraryDependency); ok {
+ // If RRO is enforced globally don't bother using "rro" variants, the empty variant will have RRO enabled.
+ if ctx.Config().EnforceRROGlobally() {
+ return ""
+ }
+
+ // If RRO is enabled for this module use the "rro" variants of static dependencies. IncomingTransition will
+ // rewrite this back to "" if the dependency is not an android_library.
+ if ctx.Config().EnforceRROForModule(ctx.Module().Name()) {
+ return "rro"
+ }
+ }
+
+ return sourceVariation
+}
+
+func (p propagateRROEnforcementTransitionMutator) IncomingTransition(ctx android.IncomingTransitionContext, incomingVariation string) string {
+ // Propagate the "rro" variant to android_library modules, but use the empty variant for everything else.
+ if incomingVariation == "rro" {
+ m := ctx.Module()
+ if _, ok := m.(AndroidLibraryDependency); ok {
+ return "rro"
+ }
+ return ""
+ }
+
+ return ""
+}
+
+func (p propagateRROEnforcementTransitionMutator) Mutate(ctx android.BottomUpMutatorContext, variation string) {
+ m := ctx.Module()
+ if d, ok := m.(AndroidLibraryDependency); ok {
+ if variation == "rro" {
+ // This is the "rro" variant of a module that has both variants, mark this one as RRO enabled and
+ // hide it from make to avoid collisions with the non-RRO empty variant.
+ d.SetRROEnforcedForDependent(true)
+ m.HideFromMake()
+ } else if ctx.Config().EnforceRROGlobally() {
+ // RRO is enabled globally, mark it enabled for this module, but there is only one variant so no
+ // need to hide it from make.
+ d.SetRROEnforcedForDependent(true)
+ }
}
}
diff --git a/java/app_test.go b/java/app_test.go
index ec97a55..b4b82e6 100644
--- a/java/app_test.go
+++ b/java/app_test.go
@@ -1419,26 +1419,31 @@
}
func TestAndroidResourceOverlays(t *testing.T) {
+ type moduleAndVariant struct {
+ module string
+ variant string
+ }
+
testCases := []struct {
name string
enforceRROTargets []string
enforceRROExcludedOverlays []string
- resourceFiles map[string][]string
- overlayFiles map[string][]string
- rroDirs map[string][]string
+ resourceFiles map[moduleAndVariant][]string
+ overlayFiles map[moduleAndVariant][]string
+ rroDirs map[moduleAndVariant][]string
}{
{
name: "no RRO",
enforceRROTargets: nil,
enforceRROExcludedOverlays: nil,
- resourceFiles: map[string][]string{
- "foo": nil,
- "bar": {"bar/res/res/values/strings.xml"},
- "lib": nil,
- "lib2": {"lib2/res/res/values/strings.xml"},
+ resourceFiles: map[moduleAndVariant][]string{
+ {"foo", "android_common"}: nil,
+ {"bar", "android_common"}: {"bar/res/res/values/strings.xml"},
+ {"lib", "android_common"}: nil,
+ {"lib2", "android_common"}: {"lib2/res/res/values/strings.xml"},
},
- overlayFiles: map[string][]string{
- "foo": {
+ overlayFiles: map[moduleAndVariant][]string{
+ {"foo", "android_common"}: {
"out/soong/.intermediates/lib2/android_common/package-res.apk",
"out/soong/.intermediates/lib/android_common/package-res.apk",
"out/soong/.intermediates/lib3/android_common/package-res.apk",
@@ -1447,57 +1452,65 @@
"device/vendor/blah/overlay/foo/res/values/strings.xml",
"product/vendor/blah/overlay/foo/res/values/strings.xml",
},
- "bar": {
+ {"bar", "android_common"}: {
"device/vendor/blah/static_overlay/bar/res/values/strings.xml",
"device/vendor/blah/overlay/bar/res/values/strings.xml",
},
- "lib": {
+ {"lib", "android_common"}: {
"out/soong/.intermediates/lib2/android_common/package-res.apk",
"lib/res/res/values/strings.xml",
"device/vendor/blah/overlay/lib/res/values/strings.xml",
},
},
- rroDirs: map[string][]string{
- "foo": nil,
- "bar": nil,
+ rroDirs: map[moduleAndVariant][]string{
+ {"foo", "android_common"}: nil,
+ {"bar", "android_common"}: nil,
},
},
{
name: "enforce RRO on foo",
enforceRROTargets: []string{"foo"},
enforceRROExcludedOverlays: []string{"device/vendor/blah/static_overlay"},
- resourceFiles: map[string][]string{
- "foo": nil,
- "bar": {"bar/res/res/values/strings.xml"},
- "lib": nil,
- "lib2": {"lib2/res/res/values/strings.xml"},
+ resourceFiles: map[moduleAndVariant][]string{
+ {"foo", "android_common"}: nil,
+ {"bar", "android_common"}: {"bar/res/res/values/strings.xml"},
+ {"lib", "android_common"}: nil,
+ {"lib", "android_common_rro"}: nil,
+ {"lib2", "android_common"}: {"lib2/res/res/values/strings.xml"},
+ {"lib2", "android_common_rro"}: {"lib2/res/res/values/strings.xml"},
},
- overlayFiles: map[string][]string{
- "foo": {
- "out/soong/.intermediates/lib2/android_common/package-res.apk",
- "out/soong/.intermediates/lib/android_common/package-res.apk",
- "out/soong/.intermediates/lib3/android_common/package-res.apk",
+ overlayFiles: map[moduleAndVariant][]string{
+ {"foo", "android_common"}: {
+ "out/soong/.intermediates/lib2/android_common_rro/package-res.apk",
+ "out/soong/.intermediates/lib/android_common_rro/package-res.apk",
+ "out/soong/.intermediates/lib3/android_common_rro/package-res.apk",
"foo/res/res/values/strings.xml",
"device/vendor/blah/static_overlay/foo/res/values/strings.xml",
},
- "bar": {
+ {"bar", "android_common"}: {
"device/vendor/blah/static_overlay/bar/res/values/strings.xml",
"device/vendor/blah/overlay/bar/res/values/strings.xml",
},
- "lib": {
+ {"lib", "android_common"}: {
"out/soong/.intermediates/lib2/android_common/package-res.apk",
"lib/res/res/values/strings.xml",
+ "device/vendor/blah/overlay/lib/res/values/strings.xml",
+ },
+ {"lib", "android_common_rro"}: {
+ "out/soong/.intermediates/lib2/android_common_rro/package-res.apk",
+ "lib/res/res/values/strings.xml",
},
},
- rroDirs: map[string][]string{
- "foo": {
+ rroDirs: map[moduleAndVariant][]string{
+ {"foo", "android_common"}: {
"device:device/vendor/blah/overlay/foo/res",
"product:product/vendor/blah/overlay/foo/res",
"device:device/vendor/blah/overlay/lib/res",
},
- "bar": nil,
- "lib": {"device:device/vendor/blah/overlay/lib/res"},
+ {"bar", "android_common"}: nil,
+ {"lib", "android_common"}: nil,
+ {"lib", "android_common_rro"}: {"device:device/vendor/blah/overlay/lib/res"},
},
},
{
@@ -1508,35 +1521,35 @@
"device/vendor/blah/static_overlay/foo",
"device/vendor/blah/static_overlay/bar/res",
},
- resourceFiles: map[string][]string{
- "foo": nil,
- "bar": {"bar/res/res/values/strings.xml"},
- "lib": nil,
- "lib2": {"lib2/res/res/values/strings.xml"},
+ resourceFiles: map[moduleAndVariant][]string{
+ {"foo", "android_common"}: nil,
+ {"bar", "android_common"}: {"bar/res/res/values/strings.xml"},
+ {"lib", "android_common"}: nil,
+ {"lib2", "android_common"}: {"lib2/res/res/values/strings.xml"},
},
- overlayFiles: map[string][]string{
- "foo": {
+ overlayFiles: map[moduleAndVariant][]string{
+ {"foo", "android_common"}: {
"out/soong/.intermediates/lib2/android_common/package-res.apk",
"out/soong/.intermediates/lib/android_common/package-res.apk",
"out/soong/.intermediates/lib3/android_common/package-res.apk",
"foo/res/res/values/strings.xml",
"device/vendor/blah/static_overlay/foo/res/values/strings.xml",
},
- "bar": {"device/vendor/blah/static_overlay/bar/res/values/strings.xml"},
- "lib": {
+ {"bar", "android_common"}: {"device/vendor/blah/static_overlay/bar/res/values/strings.xml"},
+ {"lib", "android_common"}: {
"out/soong/.intermediates/lib2/android_common/package-res.apk",
"lib/res/res/values/strings.xml",
},
},
- rroDirs: map[string][]string{
- "foo": {
+ rroDirs: map[moduleAndVariant][]string{
+ {"foo", "android_common"}: {
"device:device/vendor/blah/overlay/foo/res",
"product:product/vendor/blah/overlay/foo/res",
// Lib dep comes after the direct deps
"device:device/vendor/blah/overlay/lib/res",
},
- "bar": {"device:device/vendor/blah/overlay/bar/res"},
- "lib": {"device:device/vendor/blah/overlay/lib/res"},
+ {"bar", "android_common"}: {"device:device/vendor/blah/overlay/bar/res"},
+ {"lib", "android_common"}: {"device:device/vendor/blah/overlay/lib/res"},
},
},
}
@@ -1621,19 +1634,19 @@
for _, o := range list {
res := module.MaybeOutput(o)
if res.Rule != nil {
- // If the overlay is compiled as part of this module (i.e. a .arsc.flat file),
+ // If the overlay is compiled as part of this moduleAndVariant (i.e. a .arsc.flat file),
// verify the inputs to the .arsc.flat rule.
files = append(files, res.Inputs.Strings()...)
} else {
- // Otherwise, verify the full path to the output of the other module
+ // Otherwise, verify the full path to the output of the other moduleAndVariant
files = append(files, o)
}
}
return files
}
- getResources := func(moduleName string) (resourceFiles, overlayFiles, rroDirs []string) {
- module := result.ModuleForTests(moduleName, "android_common")
+ getResources := func(moduleName, variantName string) (resourceFiles, overlayFiles, rroDirs []string) {
+ module := result.ModuleForTests(moduleName, variantName)
resourceList := module.MaybeOutput("aapt2/res.list")
if resourceList.Rule != nil {
resourceFiles = resourceListToFiles(module, android.PathsRelativeToTop(resourceList.Inputs))
@@ -1658,21 +1671,33 @@
return resourceFiles, overlayFiles, rroDirs
}
- modules := []string{"foo", "bar", "lib", "lib2"}
- for _, module := range modules {
- resourceFiles, overlayFiles, rroDirs := getResources(module)
+ modules := []moduleAndVariant{
+ {"foo", "android_common"},
+ {"foo", "android_common_rro"},
+ {"bar", "android_common"},
+ {"bar", "android_common_rro"},
+ {"lib", "android_common"},
+ {"lib", "android_common_rro"},
+ {"lib2", "android_common"},
+ {"lib2", "android_common_rro"},
+ }
+ for _, moduleAndVariant := range modules {
+ if _, exists := testCase.resourceFiles[moduleAndVariant]; !exists {
+ continue
+ }
+ resourceFiles, overlayFiles, rroDirs := getResources(moduleAndVariant.module, moduleAndVariant.variant)
- if !reflect.DeepEqual(resourceFiles, testCase.resourceFiles[module]) {
+ if !reflect.DeepEqual(resourceFiles, testCase.resourceFiles[moduleAndVariant]) {
t.Errorf("expected %s resource files:\n %#v\n got:\n %#v",
- module, testCase.resourceFiles[module], resourceFiles)
+ moduleAndVariant, testCase.resourceFiles[moduleAndVariant], resourceFiles)
}
- if !reflect.DeepEqual(overlayFiles, testCase.overlayFiles[module]) {
+ if !reflect.DeepEqual(overlayFiles, testCase.overlayFiles[moduleAndVariant]) {
t.Errorf("expected %s overlay files:\n %#v\n got:\n %#v",
- module, testCase.overlayFiles[module], overlayFiles)
+ moduleAndVariant, testCase.overlayFiles[moduleAndVariant], overlayFiles)
}
- if !reflect.DeepEqual(rroDirs, testCase.rroDirs[module]) {
+ if !reflect.DeepEqual(rroDirs, testCase.rroDirs[moduleAndVariant]) {
t.Errorf("expected %s rroDirs: %#v\n got:\n %#v",
- module, testCase.rroDirs[module], rroDirs)
+ moduleAndVariant, testCase.rroDirs[moduleAndVariant], rroDirs)
}
}
})
diff --git a/java/rro_test.go b/java/rro_test.go
index 742c839..4d79130 100644
--- a/java/rro_test.go
+++ b/java/rro_test.go
@@ -282,7 +282,7 @@
enforceRROTargets: []string{"foo"},
rroDirs: map[string][]string{
"foo": {"product/vendor/blah/overlay/lib2/res"},
- "bar": {"product/vendor/blah/overlay/lib2/res"},
+ "bar": nil,
},
},
}