From c04fb9e6a26ada83fc113a6c2a322efa71367676 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Mon, 1 Mar 2021 12:25:10 +0000 Subject: [PATCH] Remove deapexer and prebuilt apex select mutators Originally, when the prebuilt_apex was first created, it selected the source to use in its DepsMutator. It did that because that was a convenient place for it to perform that work which had to be: * After the arch mutator had run so MultiTargets() was available. * Before the prebuilt_select mutator runs as that relied on the Source property to have been set. Change 064b70c9 then duplicated the call from the DepsMutator of the deapexer module type that was added as part of the work to make dex files available for hiddenapi processing. Change 356f7d45 moved it out of the the DepsMutator methods into its their own mutators, presumably because it interfered with the Soong -> Bazel conversion work. This change improves the existing PrebuiltSrcsSupplier mechanism to support reporting errors so that the logic for selecting the source can be done on demand rather than in separate mutators. The main complication was that PrebuiltSrcsSupplier is called with a BaseModuleContext for both source and prebuilt modules so it cannot use any methods on it that are related to the current module. That necessitated adding MultiTargets() to android.Module. Bug: 181267622 Test: m droid Change-Id: I106c78fd21016f051a315b82b470d8f12b1f820b --- android/module.go | 1 + android/prebuilt.go | 19 +++++++++++-------- apex/apex.go | 2 -- apex/deapexer.go | 12 +----------- apex/prebuilt.go | 45 +++++++++++++++++++++------------------------ cc/prebuilt.go | 2 +- 6 files changed, 35 insertions(+), 46 deletions(-) diff --git a/android/module.go b/android/module.go index 53422465a..8b23b0bd1 100644 --- a/android/module.go +++ b/android/module.go @@ -443,6 +443,7 @@ type Module interface { Disable() Enabled() bool Target() Target + MultiTargets() []Target Owner() string InstallInData() bool InstallInTestcases() bool diff --git a/android/prebuilt.go b/android/prebuilt.go index 39d30c5ec..04864a1d0 100644 --- a/android/prebuilt.go +++ b/android/prebuilt.go @@ -100,7 +100,7 @@ func (p *Prebuilt) Prefer() bool { // more modules like this. func (p *Prebuilt) SingleSourcePath(ctx ModuleContext) Path { if p.srcsSupplier != nil { - srcs := p.srcsSupplier(ctx) + srcs := p.srcsSupplier(ctx, ctx.Module()) if len(srcs) == 0 { ctx.PropertyErrorf(p.srcsPropertyName, "missing prebuilt source file") @@ -128,8 +128,11 @@ func (p *Prebuilt) UsePrebuilt() bool { // Called to provide the srcs value for the prebuilt module. // +// This can be called with a context for any module not just the prebuilt one itself. It can also be +// called concurrently. +// // Return the src value or nil if it is not available. -type PrebuiltSrcsSupplier func(ctx BaseModuleContext) []string +type PrebuiltSrcsSupplier func(ctx BaseModuleContext, prebuilt Module) []string // Initialize the module as a prebuilt module that uses the provided supplier to access the // prebuilt sources of the module. @@ -163,7 +166,7 @@ func InitPrebuiltModule(module PrebuiltInterface, srcs *[]string) { panic(fmt.Errorf("srcs must not be nil")) } - srcsSupplier := func(ctx BaseModuleContext) []string { + srcsSupplier := func(ctx BaseModuleContext, _ Module) []string { return *srcs } @@ -184,7 +187,7 @@ func InitSingleSourcePrebuiltModule(module PrebuiltInterface, srcProps interface srcFieldIndex := srcStructField.Index srcPropertyName := proptools.PropertyNameForField(srcField) - srcsSupplier := func(ctx BaseModuleContext) []string { + srcsSupplier := func(ctx BaseModuleContext, _ Module) []string { if !module.Enabled() { return nil } @@ -256,12 +259,12 @@ func PrebuiltSelectModuleMutator(ctx TopDownMutatorContext) { panic(fmt.Errorf("prebuilt module did not have InitPrebuiltModule called on it")) } if !p.properties.SourceExists { - p.properties.UsePrebuilt = p.usePrebuilt(ctx, nil) + p.properties.UsePrebuilt = p.usePrebuilt(ctx, nil, m) } } else if s, ok := ctx.Module().(Module); ok { ctx.VisitDirectDepsWithTag(PrebuiltDepTag, func(m Module) { p := m.(PrebuiltInterface).Prebuilt() - if p.usePrebuilt(ctx, s) { + if p.usePrebuilt(ctx, s, m) { p.properties.UsePrebuilt = true s.ReplacedByPrebuilt() } @@ -296,8 +299,8 @@ 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) bool { - if p.srcsSupplier != nil && len(p.srcsSupplier(ctx)) == 0 { +func (p *Prebuilt) usePrebuilt(ctx TopDownMutatorContext, source Module, prebuilt Module) bool { + if p.srcsSupplier != nil && len(p.srcsSupplier(ctx, prebuilt)) == 0 { return false } diff --git a/apex/apex.go b/apex/apex.go index 662bbbd0b..a4bdc639b 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -54,8 +54,6 @@ func init() { func RegisterPreDepsMutators(ctx android.RegisterMutatorsContext) { ctx.TopDown("apex_vndk", apexVndkMutator).Parallel() ctx.BottomUp("apex_vndk_deps", apexVndkDepsMutator).Parallel() - ctx.BottomUp("prebuilt_apex_select_source", prebuiltSelectSourceMutator).Parallel() - ctx.BottomUp("deapexer_select_source", deapexerSelectSourceMutator).Parallel() } func RegisterPostDepsMutators(ctx android.RegisterMutatorsContext) { diff --git a/apex/deapexer.go b/apex/deapexer.go index 8f4a28569..46ce41f4d 100644 --- a/apex/deapexer.go +++ b/apex/deapexer.go @@ -65,7 +65,7 @@ func privateDeapexerFactory() android.Module { &module.properties, &module.apexFileProperties, ) - android.InitSingleSourcePrebuiltModule(module, &module.apexFileProperties, "Source") + android.InitPrebuiltModuleWithSrcSupplier(module, module.apexFileProperties.prebuiltApexSelector, "src") android.InitAndroidMultiTargetsArchModule(module, android.DeviceSupported, android.MultilibCommon) return module } @@ -78,16 +78,6 @@ func (p *Deapexer) Name() string { return p.prebuilt.Name(p.ModuleBase.Name()) } -func deapexerSelectSourceMutator(ctx android.BottomUpMutatorContext) { - p, ok := ctx.Module().(*Deapexer) - if !ok { - return - } - if err := p.apexFileProperties.selectSource(ctx); err != nil { - ctx.ModuleErrorf("%s", err) - } -} - func (p *Deapexer) DepsMutator(ctx android.BottomUpMutatorContext) { // Add dependencies from the java modules to which this exports files from the `.apex` file onto // this module so that they can access the `DeapexerInfo` object that this provides. diff --git a/apex/prebuilt.go b/apex/prebuilt.go index ec7f25336..3280cd8e5 100644 --- a/apex/prebuilt.go +++ b/apex/prebuilt.go @@ -109,8 +109,10 @@ type Prebuilt struct { type ApexFileProperties struct { // the path to the prebuilt .apex file to import. - Source string `blueprint:"mutated"` - + // + // This cannot be marked as `android:"arch_variant"` because the `prebuilt_apex` is only mutated + // for android_common. That is so that it will have the same arch variant as, and so be compatible + // with, the source `apex` module type that it replaces. Src *string Arch struct { Arm struct { @@ -128,15 +130,20 @@ type ApexFileProperties struct { } } -func (p *ApexFileProperties) selectSource(ctx android.BottomUpMutatorContext) error { - // This is called before prebuilt_select and prebuilt_postdeps mutators - // The mutators requires that src to be set correctly for each arch so that - // arch variants are disabled when src is not provided for the arch. - if len(ctx.MultiTargets()) != 1 { - return fmt.Errorf("compile_multilib shouldn't be \"both\" for prebuilt_apex") +// prebuiltApexSelector selects the correct prebuilt APEX file for the build target. +// +// The ctx parameter can be for any module not just the prebuilt module so care must be taken not +// to use methods on it that are specific to the current module. +// +// See the ApexFileProperties.Src property. +func (p *ApexFileProperties) prebuiltApexSelector(ctx android.BaseModuleContext, prebuilt android.Module) []string { + multiTargets := prebuilt.MultiTargets() + if len(multiTargets) != 1 { + ctx.OtherModuleErrorf(prebuilt, "compile_multilib shouldn't be \"both\" for prebuilt_apex") + return nil } var src string - switch ctx.MultiTargets()[0].Arch.ArchType { + switch multiTargets[0].Arch.ArchType { case android.Arm: src = String(p.Arch.Arm.Src) case android.Arm64: @@ -146,14 +153,14 @@ func (p *ApexFileProperties) selectSource(ctx android.BottomUpMutatorContext) er case android.X86_64: src = String(p.Arch.X86_64.Src) default: - return fmt.Errorf("prebuilt_apex does not support %q", ctx.MultiTargets()[0].Arch.String()) + ctx.OtherModuleErrorf(prebuilt, "prebuilt_apex does not support %q", multiTargets[0].Arch.String()) + return nil } if src == "" { src = String(p.Src) } - p.Source = src - return nil + return []string{src} } type PrebuiltProperties struct { @@ -217,7 +224,7 @@ func (p *Prebuilt) Name() string { func PrebuiltFactory() android.Module { module := &Prebuilt{} module.AddProperties(&module.properties) - android.InitSingleSourcePrebuiltModule(module, &module.properties, "Source") + android.InitPrebuiltModuleWithSrcSupplier(module, module.properties.prebuiltApexSelector, "src") android.InitAndroidMultiTargetsArchModule(module, android.DeviceSupported, android.MultilibCommon) android.AddLoadHook(module, func(ctx android.LoadHookContext) { @@ -250,16 +257,6 @@ func prebuiltApexExportedModuleName(ctx android.BottomUpMutatorContext, name str return name } -func prebuiltSelectSourceMutator(ctx android.BottomUpMutatorContext) { - p, ok := ctx.Module().(*Prebuilt) - if !ok { - return - } - if err := p.properties.selectSource(ctx); err != nil { - ctx.ModuleErrorf("%s", err) - } -} - type exportedDependencyTag struct { blueprint.BaseDependencyTag name string @@ -535,7 +532,7 @@ func apexSetFactory() android.Module { module := &ApexSet{} module.AddProperties(&module.properties) - srcsSupplier := func(ctx android.BaseModuleContext) []string { + srcsSupplier := func(ctx android.BaseModuleContext, _ android.Module) []string { return module.prebuiltSrcs(ctx) } diff --git a/cc/prebuilt.go b/cc/prebuilt.go index 2cd18cb99..6b9a3d529 100644 --- a/cc/prebuilt.go +++ b/cc/prebuilt.go @@ -246,7 +246,7 @@ func NewPrebuiltLibrary(hod android.HostOrDeviceSupported) (*Module, *libraryDec module.AddProperties(&prebuilt.properties) - srcsSupplier := func(ctx android.BaseModuleContext) []string { + srcsSupplier := func(ctx android.BaseModuleContext, _ android.Module) []string { return prebuilt.prebuiltSrcs(ctx) }