From 1c4d94dccfd5490e98abe5ca95a29f0602c6047f Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Thu, 26 Oct 2023 22:38:34 +0000 Subject: [PATCH] Use `all_apex_contributions` for source/prebuilts selection This flattened singleton module explicitly lists by module name whether source or prebuilt version of a module should be used. If a module appears in this metadata module, it supersedes all other source vs prebuilts selection mechanism Implementation details - Update the module dep chain from --> to --> --> all_apex_contributions - all_apex_contributions sets a provider which is bubbled up to the source module. This requires changing `prebuilt_select` to a bottom up mutator - Update `usePrebuilt` to consult the new provider before falling back to existing source vs selection mechanisms. If (foo|prebuilt_foo) is listed in the selected `apex_contributions` modules, it will be used superseding any other selection mechanisms. - Update this depTag to IsMetaDepTag so that bootclasspath_fragment's validation ignores this new dependency. Test: Added unit tests to assert that this new mechanism supersedes `use_source_config_var` Bug: 308174768 Change-Id: I39a85639642711f3c96b6f18b94d626b55e80c66 --- android/module.go | 2 ++ android/prebuilt.go | 46 +++++++++++++++++++++++-- android/prebuilt_test.go | 72 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 3 deletions(-) diff --git a/android/module.go b/android/module.go index 250161f46..0d405ebd3 100644 --- a/android/module.go +++ b/android/module.go @@ -3231,6 +3231,8 @@ func IsMetaDependencyTag(tag blueprint.DependencyTag) bool { return true } else if tag == licensesTag { return true + } else if tag == acDepTag { + return true } return false } diff --git a/android/prebuilt.go b/android/prebuilt.go index 5acef1de4..edd014a8c 100644 --- a/android/prebuilt.go +++ b/android/prebuilt.go @@ -387,7 +387,7 @@ func RegisterPrebuiltsPreArchMutators(ctx RegisterMutatorsContext) { func RegisterPrebuiltsPostDepsMutators(ctx RegisterMutatorsContext) { ctx.BottomUp("prebuilt_source", PrebuiltSourceDepsMutator).Parallel() - ctx.TopDown("prebuilt_select", PrebuiltSelectModuleMutator).Parallel() + ctx.BottomUp("prebuilt_select", PrebuiltSelectModuleMutator).Parallel() ctx.BottomUp("prebuilt_postdeps", PrebuiltPostDepsMutator).Parallel() } @@ -406,6 +406,8 @@ func PrebuiltRenameMutator(ctx BottomUpMutatorContext) { // PrebuiltSourceDepsMutator adds dependencies to the prebuilt module from the // corresponding source module, if one exists for the same variant. +// Add a dependency from the prebuilt to `all_apex_contributions` +// The metadata will be used for source vs prebuilts selection func PrebuiltSourceDepsMutator(ctx BottomUpMutatorContext) { m := ctx.Module() // If this module is a prebuilt, is enabled and has not been renamed to source then add a @@ -416,6 +418,14 @@ func PrebuiltSourceDepsMutator(ctx BottomUpMutatorContext) { ctx.AddReverseDependency(ctx.Module(), PrebuiltDepTag, name) p.properties.SourceExists = true } + // Add a dependency from the prebuilt to the `all_apex_contributions` + // metadata module + // TODO: When all branches contain this singleton module, make this strict + // TODO: Add this dependency only for mainline prebuilts and not every prebuilt module + if ctx.OtherModuleExists("all_apex_contributions") { + ctx.AddDependency(m, acDepTag, "all_apex_contributions") + } + } } @@ -439,7 +449,7 @@ func checkInvariantsForSourceAndPrebuilt(ctx BaseModuleContext, s, p Module) { // If the visited module is the metadata module `all_apex_contributions`, it sets a // provider containing metadata about whether source or prebuilt of mainline modules should be used. // This logic was added here to prevent the overhead of creating a new mutator. -func PrebuiltSelectModuleMutator(ctx TopDownMutatorContext) { +func PrebuiltSelectModuleMutator(ctx BottomUpMutatorContext) { m := ctx.Module() if p := GetEmbeddedPrebuilt(m); p != nil { if p.srcsSupplier == nil && p.srcsPropertyName == "" { @@ -448,6 +458,17 @@ func PrebuiltSelectModuleMutator(ctx TopDownMutatorContext) { if !p.properties.SourceExists { p.properties.UsePrebuilt = p.usePrebuilt(ctx, nil, m) } + // Propagate the provider received from `all_apex_contributions` + // to the source module + ctx.VisitDirectDepsWithTag(acDepTag, func(am Module) { + if ctx.Config().Bp2buildMode() { + // This provider key is not applicable in bp2build + return + } + psi := ctx.OtherModuleProvider(am, PrebuiltSelectionInfoProvider).(PrebuiltSelectionInfoMap) + ctx.SetProvider(PrebuiltSelectionInfoProvider, psi) + }) + } else if s, ok := ctx.Module().(Module); ok { ctx.VisitDirectDepsWithTag(PrebuiltDepTag, func(prebuiltModule Module) { p := GetEmbeddedPrebuilt(prebuiltModule) @@ -491,7 +512,26 @@ func PrebuiltPostDepsMutator(ctx BottomUpMutatorContext) { // usePrebuilt returns true if a prebuilt should be used instead of the source module. The prebuilt // will be used if it is marked "prefer" or if the source module is disabled. -func (p *Prebuilt) usePrebuilt(ctx TopDownMutatorContext, source Module, prebuilt Module) bool { +func (p *Prebuilt) usePrebuilt(ctx BaseMutatorContext, source Module, prebuilt Module) bool { + // Use `all_apex_contributions` for source vs prebuilt selection. + psi := PrebuiltSelectionInfoMap{} + ctx.VisitDirectDepsWithTag(PrebuiltDepTag, func(am Module) { + if ctx.OtherModuleHasProvider(am, PrebuiltSelectionInfoProvider) { + psi = ctx.OtherModuleProvider(am, PrebuiltSelectionInfoProvider).(PrebuiltSelectionInfoMap) + } + }) + // If the source module is explicitly listed in the metadata module, use that + if source != nil && psi.IsSelected(source.base().BaseModuleName(), source.Name()) { + return false + } + // If the prebuilt module is explicitly listed in the metadata module, use that + if psi.IsSelected(prebuilt.base().BaseModuleName(), prebuilt.Name()) { + return true + } + // If the baseModuleName could not be found in the metadata module, + // fall back to the existing source vs prebuilt selection. + // TODO: Drop the fallback mechanisms + if !ctx.Config().Bp2buildMode() { if p.srcsSupplier != nil && len(p.srcsSupplier(ctx, prebuilt)) == 0 { return false diff --git a/android/prebuilt_test.go b/android/prebuilt_test.go index 915e6fab0..953258edd 100644 --- a/android/prebuilt_test.go +++ b/android/prebuilt_test.go @@ -334,6 +334,78 @@ func TestPrebuilts(t *testing.T) { // be used. prebuilt: []OsType{Android, buildOS}, }, + { + name: "apex_contributions supersedes any source preferred via use_source_config_var", + modules: ` + source { + name: "bar", + } + + prebuilt { + name: "bar", + use_source_config_var: {config_namespace: "acme", var_name: "use_source"}, + srcs: ["prebuilt_file"], + } + apex_contributions { + name: "my_mainline_module_contribution", + api_domain: "apexfoo", + // this metadata module contains prebuilt + contents: ["prebuilt_bar"], + } + all_apex_contributions { + name: "all_apex_contributions", + } + `, + preparer: FixtureModifyProductVariables(func(variables FixtureProductVariables) { + variables.VendorVars = map[string]map[string]string{ + "acme": { + "use_source": "true", + }, + } + variables.BuildFlags = map[string]string{ + "RELEASE_APEX_CONTRIBUTIONS_ADSERVICES": "my_mainline_module_contribution", + } + }), + // use_source_config_var indicates that source should be used + // but this is superseded by `my_mainline_module_contribution` + prebuilt: []OsType{Android, buildOS}, + }, + { + name: "apex_contributions supersedes any prebuilt preferred via use_source_config_var", + modules: ` + source { + name: "bar", + } + + prebuilt { + name: "bar", + use_source_config_var: {config_namespace: "acme", var_name: "use_source"}, + srcs: ["prebuilt_file"], + } + apex_contributions { + name: "my_mainline_module_contribution", + api_domain: "apexfoo", + // this metadata module contains source + contents: ["bar"], + } + all_apex_contributions { + name: "all_apex_contributions", + } + `, + preparer: FixtureModifyProductVariables(func(variables FixtureProductVariables) { + variables.VendorVars = map[string]map[string]string{ + "acme": { + "use_source": "false", + }, + } + variables.BuildFlags = map[string]string{ + "RELEASE_APEX_CONTRIBUTIONS_ADSERVICES": "my_mainline_module_contribution", + } + }), + // use_source_config_var indicates that prebuilt should be used + // but this is superseded by `my_mainline_module_contribution` + prebuilt: nil, + }, { name: "prebuilt use_source_config_var={acme, use_source} - acme_use_source=true", modules: `