apexDepsMutator is a top-down mutator

apex { name: ["myapex"], native_shared_libs: ["libX", "libY"] }
cc_library { name: "libX", shared_libs: ["libY"] }
cc_library { name: "libY", shared_libs: ["libZ"], stubs: {...} }

apexDepsMutator was a bottom up mutator and it uses WalkDeps to traverse
the dependency tree rooted at myapex in a depth-first order. While
traversing the tree, if calls BuildForApex for a module that will be
part of the APEX.

libY is visited twice. Once via libX and once via myapex. If the visit
from libX was before the visit from myapex (since this is a depth-first
traversing), BuildForApex is not called for libY and its dependency
libZ, because libY provides a stub. And then when libY is again visited
via myapex, BuildForApex is correctly called for the module, but not for
its dependencies libZ because the paths from libY to libZ was already
visited.

As a result, the apex variant of libY has a dependency to the non-apex
variant of libZ.

Fixing the problem by changing the mutator a top-down one.

Bug: 148645937
Test: m
Change-Id: Ib2cb28852087c63a568b3fd036504e9261cf0782
This commit is contained in:
Jiyong Park 2020-02-12 07:53:12 +09:00
parent ab872e0295
commit f760cae41b
2 changed files with 46 additions and 30 deletions

View file

@ -38,9 +38,12 @@ type ApexModule interface {
Module
apexModuleBase() *ApexModuleBase
// Marks that this module should be built for the APEX of the specified name.
// Marks that this module should be built for the APEXes of the specified names.
// Call this before apex.apexMutator is run.
BuildForApex(apexName string)
BuildForApexes(apexNames []string)
// Returns the name of the APEXes that this modoule will be built for
ApexVariations() []string
// Returns the name of APEX that this module will be built for. Empty string
// is returned when 'IsForPlatform() == true'. Note that a module can be
@ -66,7 +69,7 @@ type ApexModule interface {
IsInstallableToApex() bool
// Mutate this module into one or more variants each of which is built
// for an APEX marked via BuildForApex().
// for an APEX marked via BuildForApexes().
CreateApexVariations(mctx BottomUpMutatorContext) []Module
// Sets the name of the apex variant of this module. Called inside
@ -111,13 +114,19 @@ func (m *ApexModuleBase) apexModuleBase() *ApexModuleBase {
return m
}
func (m *ApexModuleBase) BuildForApex(apexName string) {
func (m *ApexModuleBase) BuildForApexes(apexNames []string) {
m.apexVariationsLock.Lock()
defer m.apexVariationsLock.Unlock()
for _, apexName := range apexNames {
if !InList(apexName, m.apexVariations) {
m.apexVariations = append(m.apexVariations, apexName)
}
}
}
func (m *ApexModuleBase) ApexVariations() []string {
return m.apexVariations
}
func (m *ApexModuleBase) ApexName() string {
return m.ApexProperties.ApexName
@ -219,18 +228,20 @@ func apexNamesMap() map[string]map[string]bool {
}
// Update the map to mark that a module named moduleName is directly or indirectly
// depended on by an APEX named apexName. Directly depending means that a module
// 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(apexName string, moduleName string, directDep bool) {
func UpdateApexDependency(apexNames []string, moduleName string, directDep bool) {
apexNamesMapMutex.Lock()
defer apexNamesMapMutex.Unlock()
apexNames, ok := apexNamesMap()[moduleName]
for _, apexName := range apexNames {
apexesForModule, ok := apexNamesMap()[moduleName]
if !ok {
apexNames = make(map[string]bool)
apexNamesMap()[moduleName] = apexNames
apexesForModule = make(map[string]bool)
apexNamesMap()[moduleName] = apexesForModule
}
apexesForModule[apexName] = apexesForModule[apexName] || directDep
}
apexNames[apexName] = apexNames[apexName] || directDep
}
// TODO(b/146393795): remove this when b/146393795 is fixed

View file

@ -1027,7 +1027,7 @@ func RegisterPreDepsMutators(ctx android.RegisterMutatorsContext) {
}
func RegisterPostDepsMutators(ctx android.RegisterMutatorsContext) {
ctx.BottomUp("apex_deps", apexDepsMutator)
ctx.TopDown("apex_deps", apexDepsMutator)
ctx.BottomUp("apex", apexMutator).Parallel()
ctx.BottomUp("apex_flattened", apexFlattenedMutator).Parallel()
ctx.BottomUp("apex_uses", apexUsesMutator).Parallel()
@ -1035,25 +1035,30 @@ func RegisterPostDepsMutators(ctx android.RegisterMutatorsContext) {
// Mark the direct and transitive dependencies of apex bundles so that they
// can be built for the apex bundles.
func apexDepsMutator(mctx android.BottomUpMutatorContext) {
func apexDepsMutator(mctx android.TopDownMutatorContext) {
var apexBundleNames []string
var directDep bool
if a, ok := mctx.Module().(*apexBundle); ok && !a.vndkApex {
apexBundleName := mctx.ModuleName()
mctx.WalkDeps(func(child, parent android.Module) bool {
depName := mctx.OtherModuleName(child)
// If the parent is apexBundle, this child is directly depended.
_, directDep := parent.(*apexBundle)
android.UpdateApexDependency(apexBundleName, depName, directDep)
apexBundleNames = []string{mctx.ModuleName()}
directDep = true
} else if am, ok := mctx.Module().(android.ApexModule); ok {
apexBundleNames = am.ApexVariations()
directDep = false
}
if len(apexBundleNames) == 0 {
return
}
mctx.VisitDirectDeps(func(child android.Module) {
depName := mctx.OtherModuleName(child)
if am, ok := child.(android.ApexModule); ok && am.CanHaveApexVariants() &&
(directDep || am.DepIsInSameApex(mctx, child)) {
am.BuildForApex(apexBundleName)
return true
} else {
return false
android.UpdateApexDependency(apexBundleNames, depName, directDep)
am.BuildForApexes(apexBundleNames)
}
})
}
}
// Create apex variations if a module is included in APEX(s).
func apexMutator(mctx android.BottomUpMutatorContext) {