From 698dd9f00780032e5f196b32e8932ea7891102b0 Mon Sep 17 00:00:00 2001 From: Jooyung Han Date: Wed, 22 Jul 2020 15:17:19 +0900 Subject: [PATCH] apexDepsMutator uses WalkDeps apexDepsMutator marks all dependencies of apex modules. Previously, it was converted from WalkDeps() to Top-down mutator to avoid the pitfall of WalkDeps() bug. (It did't handle multiple visits via different paths.) Because WalkDeps() problem solved in aosp/1277516, apexDepsMutator can be reverted to use WalkDeps(). Even though there's no observable difference between them, I revert this for the up-coming change, which requires different pruning strategies per apexes. Bug: 159195575 Test: m Change-Id: Ib09cbc7a3dfd143dd37b660b1aea6c71392ce2e3 --- android/apex.go | 33 ++++++++++++++------------------- apex/apex.go | 47 ++++++++++++++++++++++------------------------- 2 files changed, 36 insertions(+), 44 deletions(-) diff --git a/android/apex.go b/android/apex.go index 47f07ca96..a7570dc54 100644 --- a/android/apex.go +++ b/android/apex.go @@ -65,9 +65,9 @@ type ApexModule interface { apexModuleBase() *ApexModuleBase - // Marks that this module should be built for the specified APEXes. + // Marks that this module should be built for the specified APEX. // Call this before apex.apexMutator is run. - BuildForApexes(apexes []ApexInfo) + BuildForApex(apex ApexInfo) // Returns the APEXes that this module will be built for ApexVariations() []ApexInfo @@ -96,7 +96,7 @@ type ApexModule interface { IsInstallableToApex() bool // Mutate this module into one or more variants each of which is built - // for an APEX marked via BuildForApexes(). + // for an APEX marked via BuildForApex(). CreateApexVariations(mctx BottomUpMutatorContext) []Module // Tests if this module is available for the specified APEX or ":platform" @@ -178,18 +178,15 @@ func (m *ApexModuleBase) TestFor() []string { return nil } -func (m *ApexModuleBase) BuildForApexes(apexes []ApexInfo) { +func (m *ApexModuleBase) BuildForApex(apex ApexInfo) { m.apexVariationsLock.Lock() defer m.apexVariationsLock.Unlock() -nextApex: - for _, apex := range apexes { - for _, v := range m.apexVariations { - if v.ApexName == apex.ApexName { - continue nextApex - } + for _, v := range m.apexVariations { + if v.ApexName == apex.ApexName { + return } - m.apexVariations = append(m.apexVariations, apex) } + m.apexVariations = append(m.apexVariations, apex) } func (m *ApexModuleBase) ApexVariations() []ApexInfo { @@ -327,17 +324,15 @@ func apexNamesMap() map[string]map[string]bool { // depended on by the specified APEXes. Directly depending means that a module // is explicitly listed in the build definition of the APEX via properties like // native_shared_libs, java_libs, etc. -func UpdateApexDependency(apexes []ApexInfo, moduleName string, directDep bool) { +func UpdateApexDependency(apex ApexInfo, moduleName string, directDep bool) { apexNamesMapMutex.Lock() defer apexNamesMapMutex.Unlock() - for _, apex := range apexes { - apexesForModule, ok := apexNamesMap()[moduleName] - if !ok { - apexesForModule = make(map[string]bool) - apexNamesMap()[moduleName] = apexesForModule - } - apexesForModule[apex.ApexName] = apexesForModule[apex.ApexName] || directDep + apexesForModule, ok := apexNamesMap()[moduleName] + if !ok { + apexesForModule = make(map[string]bool) + apexNamesMap()[moduleName] = apexesForModule } + apexesForModule[apex.ApexName] = apexesForModule[apex.ApexName] || directDep } // TODO(b/146393795): remove this when b/146393795 is fixed diff --git a/apex/apex.go b/apex/apex.go index d0c4e6ec4..de9b3443c 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -669,7 +669,7 @@ func RegisterPreDepsMutators(ctx android.RegisterMutatorsContext) { } func RegisterPostDepsMutators(ctx android.RegisterMutatorsContext) { - ctx.TopDown("apex_deps", apexDepsMutator) + ctx.TopDown("apex_deps", apexDepsMutator).Parallel() ctx.BottomUp("apex", apexMutator).Parallel() ctx.BottomUp("apex_flattened", apexFlattenedMutator).Parallel() ctx.BottomUp("apex_uses", apexUsesMutator).Parallel() @@ -682,33 +682,30 @@ func apexDepsMutator(mctx android.TopDownMutatorContext) { if !mctx.Module().Enabled() { return } - var apexBundles []android.ApexInfo - var directDep bool - if a, ok := mctx.Module().(*apexBundle); ok && !a.vndkApex { - apexBundles = []android.ApexInfo{{ - ApexName: mctx.ModuleName(), - MinSdkVersion: a.minSdkVersion(mctx), - Updatable: a.Updatable(), - }} - directDep = true - } else if am, ok := mctx.Module().(android.ApexModule); ok { - apexBundles = am.ApexVariations() - directDep = false - } - - if len(apexBundles) == 0 { + a, ok := mctx.Module().(*apexBundle) + if !ok || a.vndkApex { return } - - cur := mctx.Module().(android.DepIsInSameApex) - - mctx.VisitDirectDeps(func(child android.Module) { - depName := mctx.OtherModuleName(child) - if am, ok := child.(android.ApexModule); ok && am.CanHaveApexVariants() && - (cur.DepIsInSameApex(mctx, child) || inAnySdk(child)) { - android.UpdateApexDependency(apexBundles, depName, directDep) - am.BuildForApexes(apexBundles) + apexInfo := android.ApexInfo{ + ApexName: mctx.ModuleName(), + MinSdkVersion: a.minSdkVersion(mctx), + Updatable: a.Updatable(), + } + mctx.WalkDeps(func(child, parent android.Module) bool { + am, ok := child.(android.ApexModule) + if !ok || !am.CanHaveApexVariants() { + return false } + if !parent.(android.DepIsInSameApex).DepIsInSameApex(mctx, child) && !inAnySdk(child) { + return false + } + + depName := mctx.OtherModuleName(child) + // If the parent is apexBundle, this child is directly depended. + _, directDep := parent.(*apexBundle) + android.UpdateApexDependency(apexInfo, depName, directDep) + am.BuildForApex(apexInfo) + return true }) }