From 2b50ce669ce41c81839cabab3f6d109ea4c6f16b Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Mon, 26 Apr 2021 15:47:28 -0400 Subject: [PATCH] Make GetDirectDep implementation match comment The comment on the interface of GetDirectDep states if there are multiple deps with different tags, the first will be returned; however, the current behavior is to panic if there are multiple deps. The behavior now: * a single dep, return the module and tag * a single module with different tags: return module with first tag * multiple modules: panic * no module: return nil, nil Bug: 186488405 Test: ~/aosp/build/bazel/ci/bp2build.sh Test: m nothing Change-Id: Id1e7315e7874b4a683ad7357ed2793822315821f --- android/bazel.go | 3 +- android/module.go | 42 +++++++++++++++---- bp2build/cc_library_static_conversion_test.go | 25 +++++++++++ 3 files changed, 62 insertions(+), 8 deletions(-) diff --git a/android/bazel.go b/android/bazel.go index 49fc0adb0..76201de10 100644 --- a/android/bazel.go +++ b/android/bazel.go @@ -181,7 +181,8 @@ var ( "libc_nopthread", // ruperts@, cc_library_static, depends on //external/arm-optimized-routine // Things that transitively depend on //system/libbase. libbase doesn't work because: - // "Multiple dependencies having same BaseModuleName() "fmtlib" found from "libbase"" + // fmtlib: fatal error: 'cassert' file not found + // libbase: no such target '//build/bazel/platforms/os:darwin': target 'darwin' not declared "libbionic_spawn_benchmark", // ruperts@, cc_library_static, depends on libbase, libgoogle-benchmark "libc_malloc_debug", // ruperts@, cc_library_static, depends on libbase "libc_malloc_debug_backtrace", // ruperts@, cc_library_static, depends on libbase diff --git a/android/module.go b/android/module.go index 9f923e2d0..fdb529083 100644 --- a/android/module.go +++ b/android/module.go @@ -213,7 +213,7 @@ type BaseModuleContext interface { // GetDirectDep returns the Module and DependencyTag for the direct dependency with the specified // name, or nil if none exists. If there are multiple dependencies on the same module it returns - // the first DependencyTag. It skips any dependencies that are not an android.Module. + // the first DependencyTag. GetDirectDep(name string) (blueprint.Module, blueprint.DependencyTag) // VisitDirectDepsBlueprint calls visit for each direct dependency. If there are multiple @@ -2244,11 +2244,12 @@ func (b *baseModuleContext) validateAndroidModule(module blueprint.Module, stric return aModule } -func (b *baseModuleContext) getDirectDepInternal(name string, tag blueprint.DependencyTag) (blueprint.Module, blueprint.DependencyTag) { - type dep struct { - mod blueprint.Module - tag blueprint.DependencyTag - } +type dep struct { + mod blueprint.Module + tag blueprint.DependencyTag +} + +func (b *baseModuleContext) getDirectDepsInternal(name string, tag blueprint.DependencyTag) []dep { var deps []dep b.VisitDirectDepsBlueprint(func(module blueprint.Module) { if aModule, _ := module.(Module); aModule != nil { @@ -2265,6 +2266,11 @@ func (b *baseModuleContext) getDirectDepInternal(name string, tag blueprint.Depe } } }) + return deps +} + +func (b *baseModuleContext) getDirectDepInternal(name string, tag blueprint.DependencyTag) (blueprint.Module, blueprint.DependencyTag) { + deps := b.getDirectDepsInternal(name, tag) if len(deps) == 1 { return deps[0].mod, deps[0].tag } else if len(deps) >= 2 { @@ -2275,6 +2281,25 @@ func (b *baseModuleContext) getDirectDepInternal(name string, tag blueprint.Depe } } +func (b *baseModuleContext) getDirectDepFirstTag(name string) (blueprint.Module, blueprint.DependencyTag) { + foundDeps := b.getDirectDepsInternal(name, nil) + deps := map[blueprint.Module]bool{} + for _, dep := range foundDeps { + deps[dep.mod] = true + } + if len(deps) == 1 { + return foundDeps[0].mod, foundDeps[0].tag + } else if len(deps) >= 2 { + // this could happen if two dependencies have the same name in different namespaces + // TODO(b/186554727): this should not occur if namespaces are handled within + // getDirectDepsInternal. + panic(fmt.Errorf("Multiple dependencies having same BaseModuleName() %q found from %q", + name, b.ModuleName())) + } else { + return nil, nil + } +} + func (b *baseModuleContext) GetDirectDepsWithTag(tag blueprint.DependencyTag) []Module { var deps []Module b.VisitDirectDepsBlueprint(func(module blueprint.Module) { @@ -2292,8 +2317,11 @@ func (m *moduleContext) GetDirectDepWithTag(name string, tag blueprint.Dependenc return module } +// GetDirectDep returns the Module and DependencyTag for the direct dependency with the specified +// name, or nil if none exists. If there are multiple dependencies on the same module it returns the +// first DependencyTag. func (b *baseModuleContext) GetDirectDep(name string) (blueprint.Module, blueprint.DependencyTag) { - return b.getDirectDepInternal(name, nil) + return b.getDirectDepFirstTag(name) } func (b *baseModuleContext) VisitDirectDepsBlueprint(visit func(blueprint.Module)) { diff --git a/bp2build/cc_library_static_conversion_test.go b/bp2build/cc_library_static_conversion_test.go index 9d23cc599..e8235d588 100644 --- a/bp2build/cc_library_static_conversion_test.go +++ b/bp2build/cc_library_static_conversion_test.go @@ -719,6 +719,31 @@ cc_library_static { "not-for-x86_64.c", ], }), +)`}, + }, + { + description: "cc_library_static multiple dep same name panic", + moduleTypeUnderTest: "cc_library_static", + moduleTypeUnderTestFactory: cc.LibraryStaticFactory, + moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryStaticBp2Build, + depsMutators: []android.RegisterMutatorFunc{cc.RegisterDepsBp2Build}, + filesystem: map[string]string{}, + bp: soongCcLibraryStaticPreamble + ` +cc_library_static { name: "static_dep" } +cc_library_static { + name: "foo_static", + static_libs: ["static_dep"], + whole_static_libs: ["static_dep"], +}`, + expectedBazelTargets: []string{`cc_library_static( + name = "foo_static", + copts = ["-I."], + deps = [":static_dep"], + linkstatic = True, +)`, `cc_library_static( + name = "static_dep", + copts = ["-I."], + linkstatic = True, )`}, }, }