From d6358775c85d764d65e6ae819e23889545b9633e Mon Sep 17 00:00:00 2001 From: Chris Parsons Date: Tue, 18 May 2021 18:35:24 -0400 Subject: [PATCH] Propagate unexported deps via implementation_deps Test: bp2build and mixed_libc CI Test: Manually verified that libc_bionic_ndk compilation gets the appropriate headers (and no extra headers) from downstream Change-Id: I79eb6e8ec1d415bd50d12105da4cf97101f95474 --- android/bazel.go | 7 +-- bp2build/cc_library_conversion_test.go | 6 +-- .../cc_library_headers_conversion_test.go | 13 ++--- bp2build/cc_library_static_conversion_test.go | 16 +++---- cc/bp2build.go | 12 +++-- cc/library.go | 48 +++++++++++-------- cc/library_headers.go | 16 ++++--- 7 files changed, 65 insertions(+), 53 deletions(-) diff --git a/android/bazel.go b/android/bazel.go index 2d4755fb5..f56c24e04 100644 --- a/android/bazel.go +++ b/android/bazel.go @@ -220,19 +220,16 @@ var ( // Per-module denylist to opt modules out of mixed builds. Such modules will // still be generated via bp2build. mixedBuildsDisabledList = []string{ - "libc_bionic_ndk", // cparsons@ cc_library_static, depends on //bionic/libc:libsystemproperties "libc_common", // cparsons@ cc_library_static, depends on //bionic/libc:libc_nopthread "libc_common_static", // cparsons@ cc_library_static, depends on //bionic/libc:libc_common "libc_common_shared", // cparsons@ cc_library_static, depends on //bionic/libc:libc_common "libc_netbsd", // lberki@, cc_library_static, version script assignment of 'LIBC_PRIVATE' to symbol 'SHA1Final' failed: symbol not defined - "libc_nopthread", // cparsons@ cc_library_static, depends on //bionic/libc:libc_bionic_ndk + "libc_nopthread", // cparsons@ cc_library_static, version script assignment of 'LIBC' to symbol 'memcmp' failed: symbol not defined "libc_openbsd", // ruperts@, cc_library_static, OK for bp2build but error: duplicate symbol: strcpy for mixed builds - "libsystemproperties", // cparsons@, cc_library_static, wrong include paths - "libpropertyinfoparser", // cparsons@, cc_library_static, wrong include paths "libarm-optimized-routines-string", // jingwen@, cc_library_static, OK for bp2build but b/186615213 (asflags not handled in bp2build), version script assignment of 'LIBC' to symbol 'memcmp' failed: symbol not defined (also for memrchr, strnlen) "fmtlib_ndk", // http://b/187040371, cc_library_static, OK for bp2build but format-inl.h:11:10: fatal error: 'cassert' file not found for mixed builds "libc_nomalloc", // cc_library_static, OK for bp2build but ld.lld: error: undefined symbol: pthread_mutex_lock (and others) - } + } // Used for quicker lookups bp2buildModuleDoNotConvert = map[string]bool{} diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index a1ffabce9..bc484ba98 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/bp2build/cc_library_conversion_test.go @@ -115,7 +115,7 @@ cc_library { "-I.", "-I$(BINDIR)/.", ], - deps = [":some-headers"], + implementation_deps = [":some-headers"], includes = ["foo-dir"], linkopts = ["-Wl,--exclude-libs=bar.a"] + select({ "//build/bazel/platforms/arch:x86": ["-Wl,--exclude-libs=baz.a"], @@ -186,7 +186,7 @@ cc_library { "-I.", "-I$(BINDIR)/.", ], - deps = [":libc_headers"], + implementation_deps = [":libc_headers"], linkopts = [ "-Wl,--exclude-libs=libgcc.a", "-Wl,--exclude-libs=libgcc_stripped.a", @@ -317,10 +317,10 @@ cc_library { name: "shared_dep_for_both" } "-Ifoo/bar", "-I$(BINDIR)/foo/bar", ], - deps = [":static_dep_for_both"], dynamic_deps = [":shared_dep_for_both"], dynamic_deps_for_shared = [":shared_dep_for_shared"], dynamic_deps_for_static = [":shared_dep_for_static"], + implementation_deps = [":static_dep_for_both"], shared_copts = ["sharedflag"], shared_srcs = ["sharedonly.cpp"], srcs = ["both.cpp"], diff --git a/bp2build/cc_library_headers_conversion_test.go b/bp2build/cc_library_headers_conversion_test.go index a3965ed9d..11124d518 100644 --- a/bp2build/cc_library_headers_conversion_test.go +++ b/bp2build/cc_library_headers_conversion_test.go @@ -135,7 +135,7 @@ cc_library_headers { "-I.", "-I$(BINDIR)/.", ], - deps = [ + implementation_deps = [ ":lib-1", ":lib-2", ], @@ -216,7 +216,7 @@ cc_library_headers { "-I.", "-I$(BINDIR)/.", ], - deps = [":base-lib"] + select({ + implementation_deps = [":base-lib"] + select({ "//build/bazel/platforms/os:android": [":android-lib"], "//build/bazel/platforms/os:darwin": [":darwin-lib"], "//build/bazel/platforms/os:fuchsia": [":fuchsia-lib"], @@ -286,10 +286,11 @@ cc_library_headers { "-I$(BINDIR)/.", ], deps = select({ - "//build/bazel/platforms/os:android": [ - ":android-lib", - ":exported-lib", - ], + "//build/bazel/platforms/os:android": [":exported-lib"], + "//conditions:default": [], + }), + implementation_deps = select({ + "//build/bazel/platforms/os:android": [":android-lib"], "//conditions:default": [], }), )`}, diff --git a/bp2build/cc_library_static_conversion_test.go b/bp2build/cc_library_static_conversion_test.go index 4fcf5e474..dc868c249 100644 --- a/bp2build/cc_library_static_conversion_test.go +++ b/bp2build/cc_library_static_conversion_test.go @@ -83,7 +83,7 @@ func TestCcLibraryStaticBp2Build(t *testing.T) { dir string }{ { - description: "cc_library_static test", + description: "cc_library_static simple test", moduleTypeUnderTest: "cc_library_static", moduleTypeUnderTestFactory: cc.LibraryStaticFactory, moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryStaticBp2Build, @@ -165,8 +165,8 @@ cc_library_static { "local_include_dir_2", ], export_include_dirs: [ - "export_include_dir_1", - "export_include_dir_2" + "export_include_dir_1", + "export_include_dir_2" ], header_libs: [ "header_lib_1", @@ -191,7 +191,7 @@ cc_library_static { "-I.", "-I$(BINDIR)/.", ], - deps = [ + implementation_deps = [ ":header_lib_1", ":header_lib_2", ":static_lib_1", @@ -464,7 +464,7 @@ cc_library_static { "-I.", "-I$(BINDIR)/.", ], - deps = select({ + implementation_deps = select({ "//build/bazel/platforms/arch:arm64": [":static_dep"], "//conditions:default": [], }), @@ -509,7 +509,7 @@ cc_library_static { "-I.", "-I$(BINDIR)/.", ], - deps = select({ + implementation_deps = select({ "//build/bazel/platforms/os:android": [":static_dep"], "//conditions:default": [], }), @@ -559,7 +559,7 @@ cc_library_static { "-I.", "-I$(BINDIR)/.", ], - deps = [":static_dep"] + select({ + implementation_deps = [":static_dep"] + select({ "//build/bazel/platforms/arch:arm64": [":static_dep4"], "//conditions:default": [], }) + select({ @@ -827,7 +827,7 @@ cc_library_static { "-I.", "-I$(BINDIR)/.", ], - deps = [":static_dep"], + implementation_deps = [":static_dep"], linkstatic = True, )`, `cc_library_static( name = "static_dep", diff --git a/cc/bp2build.go b/cc/bp2build.go index 75543acf2..95a3fe157 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -329,6 +329,7 @@ type linkerAttributes struct { deps bazel.LabelListAttribute dynamicDeps bazel.LabelListAttribute wholeArchiveDeps bazel.LabelListAttribute + exportedDeps bazel.LabelListAttribute linkopts bazel.StringListAttribute versionScript bazel.LabelAttribute } @@ -346,6 +347,7 @@ func getBp2BuildLinkerFlags(linkerProperties *BaseLinkerProperties) []string { // configurable attribute values. func bp2BuildParseLinkerProps(ctx android.TopDownMutatorContext, module *Module) linkerAttributes { var deps bazel.LabelListAttribute + var exportedDeps bazel.LabelListAttribute var dynamicDeps bazel.LabelListAttribute var wholeArchiveDeps bazel.LabelListAttribute var linkopts bazel.StringListAttribute @@ -354,11 +356,12 @@ func bp2BuildParseLinkerProps(ctx android.TopDownMutatorContext, module *Module) for _, linkerProps := range module.linker.linkerProps() { if baseLinkerProps, ok := linkerProps.(*BaseLinkerProperties); ok { libs := baseLinkerProps.Header_libs - libs = append(libs, baseLinkerProps.Export_header_lib_headers...) libs = append(libs, baseLinkerProps.Static_libs...) + exportedLibs := baseLinkerProps.Export_header_lib_headers wholeArchiveLibs := baseLinkerProps.Whole_static_libs libs = android.SortedUniqueStrings(libs) deps = bazel.MakeLabelListAttribute(android.BazelLabelForModuleDeps(ctx, libs)) + exportedDeps = bazel.MakeLabelListAttribute(android.BazelLabelForModuleDeps(ctx, exportedLibs)) linkopts.Value = getBp2BuildLinkerFlags(baseLinkerProps) wholeArchiveDeps = bazel.MakeLabelListAttribute(android.BazelLabelForModuleDeps(ctx, wholeArchiveLibs)) @@ -376,11 +379,12 @@ func bp2BuildParseLinkerProps(ctx android.TopDownMutatorContext, module *Module) for arch, p := range module.GetArchProperties(ctx, &BaseLinkerProperties{}) { if baseLinkerProps, ok := p.(*BaseLinkerProperties); ok { libs := baseLinkerProps.Header_libs - libs = append(libs, baseLinkerProps.Export_header_lib_headers...) libs = append(libs, baseLinkerProps.Static_libs...) + exportedLibs := baseLinkerProps.Export_header_lib_headers wholeArchiveLibs := baseLinkerProps.Whole_static_libs libs = android.SortedUniqueStrings(libs) deps.SetValueForArch(arch.Name, android.BazelLabelForModuleDeps(ctx, libs)) + exportedDeps.SetValueForArch(arch.Name, android.BazelLabelForModuleDeps(ctx, exportedLibs)) linkopts.SetValueForArch(arch.Name, getBp2BuildLinkerFlags(baseLinkerProps)) wholeArchiveDeps.SetValueForArch(arch.Name, android.BazelLabelForModuleDeps(ctx, wholeArchiveLibs)) @@ -397,12 +401,13 @@ func bp2BuildParseLinkerProps(ctx android.TopDownMutatorContext, module *Module) for os, p := range module.GetTargetProperties(ctx, &BaseLinkerProperties{}) { if baseLinkerProps, ok := p.(*BaseLinkerProperties); ok { libs := baseLinkerProps.Header_libs - libs = append(libs, baseLinkerProps.Export_header_lib_headers...) libs = append(libs, baseLinkerProps.Static_libs...) + exportedLibs := baseLinkerProps.Export_header_lib_headers wholeArchiveLibs := baseLinkerProps.Whole_static_libs libs = android.SortedUniqueStrings(libs) wholeArchiveDeps.SetValueForOS(os.Name, android.BazelLabelForModuleDeps(ctx, wholeArchiveLibs)) deps.SetValueForOS(os.Name, android.BazelLabelForModuleDeps(ctx, libs)) + exportedDeps.SetValueForOS(os.Name, android.BazelLabelForModuleDeps(ctx, exportedLibs)) linkopts.SetValueForOS(os.Name, getBp2BuildLinkerFlags(baseLinkerProps)) @@ -413,6 +418,7 @@ func bp2BuildParseLinkerProps(ctx android.TopDownMutatorContext, module *Module) return linkerAttributes{ deps: deps, + exportedDeps: exportedDeps, dynamicDeps: dynamicDeps, wholeArchiveDeps: wholeArchiveDeps, linkopts: linkopts, diff --git a/cc/library.go b/cc/library.go index 1ba359758..c918b96ba 100644 --- a/cc/library.go +++ b/cc/library.go @@ -221,17 +221,19 @@ func RegisterLibraryBuildComponents(ctx android.RegistrationContext) { // For bp2build conversion. type bazelCcLibraryAttributes struct { // Attributes pertaining to both static and shared variants. - Srcs bazel.LabelListAttribute - Hdrs bazel.LabelListAttribute - Deps bazel.LabelListAttribute - Dynamic_deps bazel.LabelListAttribute - Whole_archive_deps bazel.LabelListAttribute - Copts bazel.StringListAttribute - Includes bazel.StringListAttribute - Linkopts bazel.StringListAttribute + Srcs bazel.LabelListAttribute + Hdrs bazel.LabelListAttribute + Deps bazel.LabelListAttribute + Implementation_deps bazel.LabelListAttribute + Dynamic_deps bazel.LabelListAttribute + Whole_archive_deps bazel.LabelListAttribute + Copts bazel.StringListAttribute + Includes bazel.StringListAttribute + Linkopts bazel.StringListAttribute // Attributes pertaining to shared variant. Shared_copts bazel.StringListAttribute Shared_srcs bazel.LabelListAttribute + Exported_deps_for_shared bazel.LabelListAttribute Static_deps_for_shared bazel.LabelListAttribute Dynamic_deps_for_shared bazel.LabelListAttribute Whole_archive_deps_for_shared bazel.LabelListAttribute @@ -240,6 +242,7 @@ type bazelCcLibraryAttributes struct { // Attributes pertaining to static variant. Static_copts bazel.StringListAttribute Static_srcs bazel.LabelListAttribute + Exported_deps_for_static bazel.LabelListAttribute Static_deps_for_static bazel.LabelListAttribute Dynamic_deps_for_static bazel.LabelListAttribute Whole_archive_deps_for_static bazel.LabelListAttribute @@ -292,7 +295,8 @@ func CcLibraryBp2Build(ctx android.TopDownMutatorContext) { attrs := &bazelCcLibraryAttributes{ Srcs: srcs, - Deps: linkerAttrs.deps, + Implementation_deps: linkerAttrs.deps, + Deps: linkerAttrs.exportedDeps, Dynamic_deps: linkerAttrs.dynamicDeps, Whole_archive_deps: linkerAttrs.wholeArchiveDeps, Copts: compilerAttrs.copts, @@ -2217,14 +2221,15 @@ func maybeInjectBoringSSLHash(ctx android.ModuleContext, outputFile android.Modu } type bazelCcLibraryStaticAttributes struct { - Copts bazel.StringListAttribute - Srcs bazel.LabelListAttribute - Deps bazel.LabelListAttribute - Whole_archive_deps bazel.LabelListAttribute - Linkopts bazel.StringListAttribute - Linkstatic bool - Includes bazel.StringListAttribute - Hdrs bazel.LabelListAttribute + Copts bazel.StringListAttribute + Srcs bazel.LabelListAttribute + Implementation_deps bazel.LabelListAttribute + Deps bazel.LabelListAttribute + Whole_archive_deps bazel.LabelListAttribute + Linkopts bazel.StringListAttribute + Linkstatic bool + Includes bazel.StringListAttribute + Hdrs bazel.LabelListAttribute } type bazelCcLibraryStatic struct { @@ -2245,10 +2250,11 @@ func ccLibraryStaticBp2BuildInternal(ctx android.TopDownMutatorContext, module * exportedIncludes := bp2BuildParseExportedIncludes(ctx, module) attrs := &bazelCcLibraryStaticAttributes{ - Copts: compilerAttrs.copts, - Srcs: compilerAttrs.srcs, - Deps: linkerAttrs.deps, - Whole_archive_deps: linkerAttrs.wholeArchiveDeps, + Copts: compilerAttrs.copts, + Srcs: compilerAttrs.srcs, + Implementation_deps: linkerAttrs.deps, + Deps: linkerAttrs.exportedDeps, + Whole_archive_deps: linkerAttrs.wholeArchiveDeps, Linkopts: linkerAttrs.linkopts, Linkstatic: true, diff --git a/cc/library_headers.go b/cc/library_headers.go index 0aba8dea8..20659292d 100644 --- a/cc/library_headers.go +++ b/cc/library_headers.go @@ -109,10 +109,11 @@ func prebuiltLibraryHeaderFactory() android.Module { } type bazelCcLibraryHeadersAttributes struct { - Copts bazel.StringListAttribute - Hdrs bazel.LabelListAttribute - Includes bazel.StringListAttribute - Deps bazel.LabelListAttribute + Copts bazel.StringListAttribute + Hdrs bazel.LabelListAttribute + Includes bazel.StringListAttribute + Deps bazel.LabelListAttribute + Implementation_deps bazel.LabelListAttribute } type bazelCcLibraryHeaders struct { @@ -147,9 +148,10 @@ func CcLibraryHeadersBp2Build(ctx android.TopDownMutatorContext) { linkerAttrs := bp2BuildParseLinkerProps(ctx, module) attrs := &bazelCcLibraryHeadersAttributes{ - Copts: compilerAttrs.copts, - Includes: exportedIncludes, - Deps: linkerAttrs.deps, + Copts: compilerAttrs.copts, + Includes: exportedIncludes, + Implementation_deps: linkerAttrs.deps, + Deps: linkerAttrs.exportedDeps, } props := bazel.BazelTargetModuleProperties{