From c143cc50dd8bfe21d7417a9fc064bef2529f5222 Mon Sep 17 00:00:00 2001 From: Rupert Shuttleworth Date: Tue, 13 Apr 2021 13:08:04 -0400 Subject: [PATCH] Transform paths to headers in include dirs to take package boundaries into account. This allows the following cc_library_static targets to build with bp2build: - libc_freebsd - libc_freebsd_large_stack - libc_openbsd_ndk - libc_gdtoa (* but still fails for mixed builds) - libc_aeabi - libc_static_dispatch - libc_dynamic_dispatch This also allows a number of other cc_library_static targets to progress further in their builds. Test: Added unit test Test: bp2build-sync.py write; bazel build //bionic/... Change-Id: I71742565c16594448a41a6428a5c993171ec4cb4 --- android/bazel.go | 42 ++++----- android/paths.go | 88 +++++++++++++++++++ bp2build/cc_library_static_conversion_test.go | 47 ++++++++++ 3 files changed, 154 insertions(+), 23 deletions(-) diff --git a/android/bazel.go b/android/bazel.go index b3f9d8863..9468891af 100644 --- a/android/bazel.go +++ b/android/bazel.go @@ -129,6 +129,7 @@ var ( // Configure modules in these directories to enable bp2build_available: true or false by default. bp2buildDefaultConfig = Bp2BuildConfig{ "bionic": Bp2BuildDefaultTrueRecursively, + "external/gwp_asan": Bp2BuildDefaultTrueRecursively, "system/core/libcutils": Bp2BuildDefaultTrueRecursively, "system/logging/liblog": Bp2BuildDefaultTrueRecursively, } @@ -138,32 +139,25 @@ var ( "libBionicBenchmarksUtils", // ruperts@, cc_library_static, 'map' file not found "libbionic_spawn_benchmark", // ruperts@, cc_library_static, depends on //system/libbase "libc_jemalloc_wrapper", // ruperts@, cc_library_static, depends on //external/jemalloc_new - "libc_bootstrap", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage - "libc_init_static", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage - "libc_init_dynamic", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage - "libc_tzcode", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage - "libc_freebsd", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage - "libc_freebsd_large_stack", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage - "libc_netbsd", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage - "libc_openbsd_ndk", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage - "libc_openbsd_large_stack", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage - "libc_openbsd", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage - "libc_gdtoa", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage - "libc_fortify", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage - "libc_bionic", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage + "libc_bootstrap", // ruperts@, cc_library_static, 'private/bionic_auxv.h' file not found + "libc_init_static", // ruperts@, cc_library_static, 'private/bionic_elf_tls.h' file not found + "libc_init_dynamic", // ruperts@, cc_library_static, 'private/bionic_defs.h' file not found + "libc_tzcode", // ruperts@, cc_library_static, error: expected expression + "libc_netbsd", // ruperts@, cc_library_static, 'engine.c' file not found + "libc_openbsd_large_stack", // ruperts@, cc_library_static, 'android/log.h' file not found + "libc_openbsd", // ruperts@, cc_library_static, 'android/log.h' file not found + "libc_fortify", // ruperts@, cc_library_static, 'private/bionic_fortify.h' file not found + "libc_bionic", // ruperts@, cc_library_static, 'private/bionic_asm.h' file not found "libc_bionic_ndk", // ruperts@, cc_library_static, depends on //bionic/libc/system_properties - "libc_bionic_systrace", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage - "libc_pthread", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage + "libc_bionic_systrace", // ruperts@, cc_library_static, 'private/bionic_systrace.h' file not found + "libc_pthread", // ruperts@, cc_library_static, 'private/bionic_defs.h' file not found "libc_syscalls", // ruperts@, cc_library_static, mutator panic cannot get direct dep syscalls-arm64.S of libc_syscalls - "libc_aeabi", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage "libc_ndk", // ruperts@, cc_library_static, depends on //bionic/libm:libm "libc_nopthread", // ruperts@, cc_library_static, depends on //external/arm-optimized-routines "libc_common", // ruperts@, cc_library_static, depends on //bionic/libc:libc_nopthread - "libc_static_dispatch", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage - "libc_dynamic_dispatch", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage "libc_common_static", // ruperts@, cc_library_static, depends on //bionic/libc:libc_common "libc_common_shared", // ruperts@, cc_library_static, depends on //bionic/libc:libc_common - "libc_unwind_static", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage + "libc_unwind_static", // ruperts@, cc_library_static, 'private/bionic_elf_tls.h' file not found "libc_nomalloc", // ruperts@, cc_library_static, depends on //bionic/libc:libc_common "libasync_safe", // ruperts@, cc_library_static, 'private/CachedProperty.h' file not found "libc_malloc_debug_backtrace", // ruperts@, cc_library_static, depends on //system/libbase @@ -173,10 +167,7 @@ var ( "liblinker_malloc", // ruperts@, cc_library_static, depends on //system/logging/liblog:liblog "liblinker_debuggerd_stub", // ruperts@, cc_library_static, depends on //system/libbase "libbionic_tests_headers_posix", // ruperts@, cc_library_static, 'complex.h' file not found - "libc_dns", // ruperts@, cc_library_static, 'bionic/libc/async_safe' is a subpackage - - "note_memtag_heap_async", // jingwen@, b/185079815, features.h includes not found - "note_memtag_heap_sync", // jingwen@, b/185079815, features.h includes not found + "libc_dns", // ruperts@, cc_library_static, 'android/log.h' file not found // List of all full_cc_libraries in //bionic, with their immediate failures "libc", // jingwen@, cc_library, depends on //external/gwp_asan @@ -186,6 +177,11 @@ var ( "libm", // jingwen@, cc_library, fatal error: 'freebsd-compat.h' file not found "libseccomp_policy", // jingwen@, cc_library, fatal error: 'seccomp_policy.h' file not found "libstdc++", // jingwen@, cc_library, depends on //external/gwp_asan + + // For mixed builds specifically + "note_memtag_heap_async", // jingwen@, cc_library_static, OK for bp2build but features.h includes not found for mixed builds (b/185079815) + "note_memtag_heap_sync", // jingwen@, cc_library_static, OK for bp2build but features.h includes not found for mixed builds (b/185079815) + "libc_gdtoa", // ruperts@, cc_library_static, OK for bp2build but undefined symbol: __strtorQ for mixed builds } // Used for quicker lookups diff --git a/android/paths.go b/android/paths.go index df1222870..d77082390 100644 --- a/android/paths.go +++ b/android/paths.go @@ -416,6 +416,93 @@ func BazelLabelForModuleDeps(ctx BazelConversionPathContext, modules []string) b return labels } +// Returns true if a prefix + components[:i] + /Android.bp exists +// TODO(b/185358476) Could check for BUILD file instead of checking for Android.bp file, or ensure BUILD is always generated? +func directoryHasBlueprint(fs pathtools.FileSystem, prefix string, components []string, componentIndex int) bool { + blueprintPath := prefix + if blueprintPath != "" { + blueprintPath = blueprintPath + "/" + } + blueprintPath = blueprintPath + strings.Join(components[:componentIndex+1], "/") + blueprintPath = blueprintPath + "/Android.bp" + if exists, _, _ := fs.Exists(blueprintPath); exists { + return true + } else { + return false + } +} + +// Transform a path (if necessary) to acknowledge package boundaries +// +// e.g. something like +// async_safe/include/async_safe/CHECK.h +// might become +// //bionic/libc/async_safe:include/async_safe/CHECK.h +// if the "async_safe" directory is actually a package and not just a directory. +// +// In particular, paths that extend into packages are transformed into absolute labels beginning with //. +func transformSubpackagePath(ctx BazelConversionPathContext, path bazel.Label) bazel.Label { + var newPath bazel.Label + + // Don't transform Bp_text + newPath.Bp_text = path.Bp_text + + if strings.HasPrefix(path.Label, "//") { + // Assume absolute labels are already correct (e.g. //path/to/some/package:foo.h) + newPath.Label = path.Label + return newPath + } + + newLabel := "" + pathComponents := strings.Split(path.Label, "/") + foundBlueprint := false + // Check the deepest subdirectory first and work upwards + for i := len(pathComponents) - 1; i >= 0; i-- { + pathComponent := pathComponents[i] + var sep string + if !foundBlueprint && directoryHasBlueprint(ctx.Config().fs, ctx.ModuleDir(), pathComponents, i) { + sep = ":" + foundBlueprint = true + } else { + sep = "/" + } + if newLabel == "" { + newLabel = pathComponent + } else { + newLabel = pathComponent + sep + newLabel + } + } + if foundBlueprint { + // Ensure paths end up looking like //bionic/... instead of //./bionic/... + moduleDir := ctx.ModuleDir() + if strings.HasPrefix(moduleDir, ".") { + moduleDir = moduleDir[1:] + } + // Make the path into an absolute label (e.g. //bionic/libc/foo:bar.h instead of just foo:bar.h) + if moduleDir == "" { + newLabel = "//" + newLabel + } else { + newLabel = "//" + moduleDir + "/" + newLabel + } + } + newPath.Label = newLabel + + return newPath +} + +// Transform paths to acknowledge package boundaries +// See transformSubpackagePath() for more information +func transformSubpackagePaths(ctx BazelConversionPathContext, paths bazel.LabelList) bazel.LabelList { + var newPaths bazel.LabelList + for _, include := range paths.Includes { + newPaths.Includes = append(newPaths.Includes, transformSubpackagePath(ctx, include)) + } + for _, exclude := range paths.Excludes { + newPaths.Excludes = append(newPaths.Excludes, transformSubpackagePath(ctx, exclude)) + } + return newPaths +} + // BazelLabelForModuleSrc returns bazel.LabelList with paths rooted from the module's local source // directory. It expands globs, and resolves references to modules using the ":name" syntax to // bazel-compatible labels. Properties passed as the paths or excludes argument must have been @@ -445,6 +532,7 @@ func BazelLabelForModuleSrcExcludes(ctx BazelConversionPathContext, paths, exclu } labels := expandSrcsForBazel(ctx, paths, excluded) labels.Excludes = excludeLabels.Includes + labels = transformSubpackagePaths(ctx, labels) return labels } diff --git a/bp2build/cc_library_static_conversion_test.go b/bp2build/cc_library_static_conversion_test.go index 427aed355..7e72a8b22 100644 --- a/bp2build/cc_library_static_conversion_test.go +++ b/bp2build/cc_library_static_conversion_test.go @@ -277,6 +277,53 @@ cc_library_static { "implicit_include_1.h", "implicit_include_2.h", ], +)`}, + }, + { + description: "cc_library_static subpackage test", + moduleTypeUnderTest: "cc_library_static", + moduleTypeUnderTestFactory: cc.LibraryStaticFactory, + moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryStaticBp2Build, + filesystem: map[string]string{ + // subpackage with subdirectory + "subpackage/Android.bp": "", + "subpackage/subpackage_header.h": "", + "subpackage/subdirectory/subdirectory_header.h": "", + // subsubpackage with subdirectory + "subpackage/subsubpackage/Android.bp": "", + "subpackage/subsubpackage/subsubpackage_header.h": "", + "subpackage/subsubpackage/subdirectory/subdirectory_header.h": "", + // subsubsubpackage with subdirectory + "subpackage/subsubpackage/subsubsubpackage/Android.bp": "", + "subpackage/subsubpackage/subsubsubpackage/subsubsubpackage_header.h": "", + "subpackage/subsubpackage/subsubsubpackage/subdirectory/subdirectory_header.h": "", + }, + bp: soongCcLibraryStaticPreamble + ` +cc_library_static { + name: "foo_static", + srcs: [ + ], + include_dirs: [ + "subpackage", + ], + + bazel_module: { bp2build_available: true }, +}`, + expectedBazelTargets: []string{`cc_library_static( + name = "foo_static", + includes = [ + "subpackage", + ".", + ], + linkstatic = True, + srcs = [ + "//subpackage:subpackage_header.h", + "//subpackage:subdirectory/subdirectory_header.h", + "//subpackage/subsubpackage:subsubpackage_header.h", + "//subpackage/subsubpackage:subdirectory/subdirectory_header.h", + "//subpackage/subsubpackage/subsubsubpackage:subsubsubpackage_header.h", + "//subpackage/subsubpackage/subsubsubpackage:subdirectory/subdirectory_header.h", + ], )`}, }, }