From daa09efb5e00a316460faec498dc65fbca41bae0 Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Wed, 15 Dec 2021 15:35:38 -0500 Subject: [PATCH] Start unblocking com.android.runtime deps Adds support to warn rather than error about missing deps in order to suport modules in build/soong with deps outside of build/sooong in bp2build integration tests. Test: build/bazel/ci/bp2build.sh Change-Id: I1282bccd37a3fc9f33555d34e68d7f0873d8272c --- android/bazel.go | 32 +++++++++++++++++++++++++----- android/bazel_paths.go | 33 ++++++++++++++++++++++--------- android/module.go | 18 +++++++++++++++++ bp2build/build_conversion.go | 9 +++++++++ bp2build/build_conversion_test.go | 13 ++++++++++++ bp2build/metrics.go | 16 +++++++++++++-- 6 files changed, 105 insertions(+), 16 deletions(-) diff --git a/android/bazel.go b/android/bazel.go index 970ad0d71..2eb541780 100644 --- a/android/bazel.go +++ b/android/bazel.go @@ -236,6 +236,8 @@ var ( // Configure modules in these directories to enable bp2build_available: true or false by default. bp2buildDefaultConfig = Bp2BuildConfig{ "art/libdexfile": Bp2BuildDefaultTrueRecursively, + "art/runtime": Bp2BuildDefaultTrueRecursively, + "art/tools": Bp2BuildDefaultTrue, "bionic": Bp2BuildDefaultTrueRecursively, "bootable/recovery/tools/recovery_l10n": Bp2BuildDefaultTrue, "build/bazel/examples/soong_config_variables": Bp2BuildDefaultTrueRecursively, @@ -245,6 +247,7 @@ var ( "build/soong/cc/libbuildversion": Bp2BuildDefaultTrue, // Skip tests subdir "build/soong/cc/ndkstubgen": Bp2BuildDefaultTrue, "build/soong/cc/symbolfile": Bp2BuildDefaultTrue, + "build/soong/linkerconfig": Bp2BuildDefaultTrueRecursively, "build/soong/scripts": Bp2BuildDefaultTrueRecursively, "cts/common/device-side/nativetesthelper/jni": Bp2BuildDefaultTrueRecursively, "development/apps/DevelopmentSettings": Bp2BuildDefaultTrue, @@ -323,6 +326,7 @@ var ( "packages/apps/DevCamera": Bp2BuildDefaultTrue, "packages/apps/HTMLViewer": Bp2BuildDefaultTrue, "packages/apps/Protips": Bp2BuildDefaultTrue, + "packages/modules/StatsD/lib/libstatssocket": Bp2BuildDefaultTrueRecursively, "packages/modules/adb": Bp2BuildDefaultTrue, "packages/modules/adb/apex": Bp2BuildDefaultTrue, "packages/modules/adb/crypto": Bp2BuildDefaultTrueRecursively, @@ -336,6 +340,8 @@ var ( "packages/services/Car/tests/SampleRearViewCamera": Bp2BuildDefaultTrue, "prebuilts/clang/host/linux-x86": Bp2BuildDefaultTrueRecursively, "system/apex": Bp2BuildDefaultFalse, // TODO(b/207466993): flaky failures + "system/apex/proto": Bp2BuildDefaultTrueRecursively, + "system/apex/libs": Bp2BuildDefaultTrueRecursively, "system/core/debuggerd": Bp2BuildDefaultTrueRecursively, "system/core/diagnose_usb": Bp2BuildDefaultTrueRecursively, "system/core/libasyncio": Bp2BuildDefaultTrue, @@ -364,6 +370,16 @@ var ( // Per-module denylist to always opt modules out of both bp2build and mixed builds. bp2buildModuleDoNotConvertList = []string{ + "libnativehelper_compat_libc", // Broken compile: implicit declaration of function 'strerror_r' is invalid in C99 + + "libart", // depends on unconverted modules: art_operator_srcs, libodrstatslog, libelffile, art_cmdlineparser_headers, cpp-define-generator-definitions, libcpu_features, libdexfile, libartpalette, libbacktrace, libnativebridge, libnativeloader, libsigchain, libunwindstack, libartbase, libprofile, cpp-define-generator-asm-support, apex-info-list-tinyxml, libtinyxml2, libnativeloader-headers, libstatssocket, heapprofd_client_api + "libart-runtime-gtest", // depends on unconverted modules: libgtest_isolated, libart-compiler, libdexfile, libprofile, libartbase, libbacktrace, libartbase-art-gtest + "libart_headers", // depends on unconverted modules: art_libartbase_headers + "libartd", // depends on unconverted modules: apex-info-list-tinyxml, libtinyxml2, libnativeloader-headers, libstatssocket, heapprofd_client_api, art_operator_srcs, libodrstatslog, libelffiled, art_cmdlineparser_headers, cpp-define-generator-definitions, libcpu_features, libdexfiled, libartpalette, libbacktrace, libnativebridge, libnativeloader, libsigchain, libunwindstack, libartbased, libprofiled, cpp-define-generator-asm-support + "libartd-runtime-gtest", // depends on unconverted modules: libgtest_isolated, libartd-compiler, libdexfiled, libprofiled, libartbased, libbacktrace, libartbased-art-gtest + "libstatslog_art", // depends on unconverted modules: statslog_art.cpp, statslog_art.h + "statslog_art.h", "statslog_art.cpp", // depends on unconverted modules: stats-log-api-gen + "libandroid_runtime_lazy", // depends on unconverted modules: libbinder_headers "libcmd", // depends on unconverted modules: libbinder @@ -403,22 +419,23 @@ var ( "libdebuggerd", // depends on unconverted modules libdexfile_support, libunwindstack, gwp_asan_crash_handler, libtombstone_proto, libprotobuf-cpp-lite "libdexfile_static", // depends on libartpalette, libartbase, libdexfile, which are of unsupported type: art_cc_library. - "host_bionic_linker_asm", // depends on extract_linker, a go binary. - "host_bionic_linker_script", // depends on extract_linker, a go binary. - "static_crasher", // depends on unconverted modules: libdebuggerd_handler + "static_crasher", // depends on unconverted modules: libdebuggerd_handler "pbtombstone", "crash_dump", // depends on libdebuggerd, libunwindstack "libbase_ndk", // http://b/186826477, fails to link libctscamera2_jni for device (required for CtsCameraTestCases) - "libprotobuf-python", // contains .proto sources "libprotobuf-internal-protos", // b/210751803, we don't handle path property for filegroups "libprotobuf-internal-python-srcs", // b/210751803, we don't handle path property for filegroups "libprotobuf-java-full", // b/210751803, we don't handle path property for filegroups "libprotobuf-java-util-full", // b/210751803, we don't handle path property for filegroups "conscrypt", // b/210751803, we don't handle path property for filegroups - "conv_linker_config", // depends on linker_config_proto, a python lib with proto sources + // python protos + "libprotobuf-python", // contains .proto sources + "conv_linker_config", // depends on linker_config_proto, a python lib with proto sources + "apex_build_info_proto", "apex_manifest_proto", // a python lib with proto sources + "linker_config_proto", // contains .proto sources "brotli-fuzzer-corpus", // b/202015218: outputs are in location incompatible with bazel genrule handling. @@ -446,6 +463,11 @@ var ( "libdexfile", // depends on unconverted modules: dexfile_operator_srcs, libartbase, libartpalette, "libdexfiled", // depends on unconverted modules: dexfile_operator_srcs, libartbased, libartpalette + + // go deps: + "apex-protos", // depends on unconverted modules: soong_zip + "host_bionic_linker_asm", // depends on extract_linker, a go binary. + "host_bionic_linker_script", // depends on extract_linker, a go binary. } // Per-module denylist of cc_library modules to only generate the static diff --git a/android/bazel_paths.go b/android/bazel_paths.go index 62e6156a4..f353a9dc8 100644 --- a/android/bazel_paths.go +++ b/android/bazel_paths.go @@ -92,6 +92,7 @@ type BazelConversionPathContext interface { GetDirectDep(name string) (blueprint.Module, blueprint.DependencyTag) ModuleFromName(name string) (blueprint.Module, bool) AddUnconvertedBp2buildDep(string) + AddMissingBp2buildDep(dep string) } // BazelLabelForModuleDeps expects a list of reference to other modules, ("" @@ -129,8 +130,10 @@ func BazelLabelForModuleDepsWithFn(ctx BazelConversionPathContext, modules []str } if m, t := SrcIsModuleWithTag(module); m != "" { l := getOtherModuleLabel(ctx, m, t, moduleToLabelFn) - l.OriginalModuleName = bpText - labels.Includes = append(labels.Includes, l) + if l != nil { + l.OriginalModuleName = bpText + labels.Includes = append(labels.Includes, *l) + } } else { ctx.ModuleErrorf("%q, is not a module reference", module) } @@ -157,11 +160,17 @@ func BazelLabelForModuleDepsExcludesWithFn(ctx BazelConversionPathContext, modul } func BazelLabelForModuleSrcSingle(ctx BazelConversionPathContext, path string) bazel.Label { - return BazelLabelForModuleSrcExcludes(ctx, []string{path}, []string(nil)).Includes[0] + if srcs := BazelLabelForModuleSrcExcludes(ctx, []string{path}, []string(nil)).Includes; len(srcs) > 0 { + return srcs[0] + } + return bazel.Label{} } func BazelLabelForModuleDepSingle(ctx BazelConversionPathContext, path string) bazel.Label { - return BazelLabelForModuleDepsExcludes(ctx, []string{path}, []string(nil)).Includes[0] + if srcs := BazelLabelForModuleDepsExcludes(ctx, []string{path}, []string(nil)).Includes; len(srcs) > 0 { + return srcs[0] + } + return bazel.Label{} } // BazelLabelForModuleSrc expects a list of path (relative to local module directory) and module @@ -328,9 +337,9 @@ func expandSrcsForBazel(ctx BazelConversionPathContext, paths, expandedExcludes for _, p := range paths { if m, tag := SrcIsModuleWithTag(p); m != "" { l := getOtherModuleLabel(ctx, m, tag, BazelModuleLabel) - if !InList(l.Label, expandedExcludes) { + if l != nil && !InList(l.Label, expandedExcludes) { l.OriginalModuleName = fmt.Sprintf(":%s", m) - labels.Includes = append(labels.Includes, l) + labels.Includes = append(labels.Includes, *l) } } else { var expandedPaths []bazel.Label @@ -354,10 +363,16 @@ func expandSrcsForBazel(ctx BazelConversionPathContext, paths, expandedExcludes // module. The label will be relative to the current directory if appropriate. The dependency must // already be resolved by either deps mutator or path deps mutator. func getOtherModuleLabel(ctx BazelConversionPathContext, dep, tag string, - labelFromModule func(BazelConversionPathContext, blueprint.Module) string) bazel.Label { + labelFromModule func(BazelConversionPathContext, blueprint.Module) string) *bazel.Label { m, _ := ctx.ModuleFromName(dep) + // The module was not found in an Android.bp file, this is often due to: + // * a limited manifest + // * a required module not being converted from Android.mk if m == nil { - panic(fmt.Errorf("No module named %q found, but was a direct dep of %q", dep, ctx.Module().Name())) + ctx.AddMissingBp2buildDep(dep) + return &bazel.Label{ + Label: ":" + dep + "__BP2BUILD__MISSING__DEP", + } } if !convertedToBazel(ctx, m) { ctx.AddUnconvertedBp2buildDep(dep) @@ -371,7 +386,7 @@ func getOtherModuleLabel(ctx BazelConversionPathContext, dep, tag string, otherLabel = bazelShortLabel(otherLabel) } - return bazel.Label{ + return &bazel.Label{ Label: otherLabel, } } diff --git a/android/module.go b/android/module.go index 2750131d8..9a31bf8c3 100644 --- a/android/module.go +++ b/android/module.go @@ -320,6 +320,9 @@ type BaseModuleContext interface { // AddUnconvertedBp2buildDep stores module name of a direct dependency that was not converted via bp2build AddUnconvertedBp2buildDep(dep string) + // AddMissingBp2buildDep stores the module name of a direct dependency that was not found. + AddMissingBp2buildDep(dep string) + Target() Target TargetPrimary() bool @@ -516,6 +519,7 @@ type Module interface { // Bp2buildTargets returns the target(s) generated for Bazel via bp2build for this module Bp2buildTargets() []bp2buildInfo GetUnconvertedBp2buildDeps() []string + GetMissingBp2buildDeps() []string BuildParamsForTests() []BuildParams RuleParamsForTests() map[blueprint.Rule]blueprint.RuleParams @@ -857,6 +861,9 @@ type commonProperties struct { // UnconvertedBp2buildDep stores the module names of direct dependency that were not converted to // Bazel UnconvertedBp2buildDeps []string `blueprint:"mutated"` + + // MissingBp2buildDep stores the module names of direct dependency that were not found + MissingBp2buildDeps []string `blueprint:"mutated"` } // CommonAttributes represents the common Bazel attributes from which properties @@ -1325,12 +1332,23 @@ func (b *baseModuleContext) AddUnconvertedBp2buildDep(dep string) { *unconvertedDeps = append(*unconvertedDeps, dep) } +// AddMissingBp2buildDep stores module name of a dependency that was not found in a Android.bp file. +func (b *baseModuleContext) AddMissingBp2buildDep(dep string) { + missingDeps := &b.Module().base().commonProperties.MissingBp2buildDeps + *missingDeps = append(*missingDeps, dep) +} + // GetUnconvertedBp2buildDeps returns the list of module names of this module's direct dependencies that // were not converted to Bazel. func (m *ModuleBase) GetUnconvertedBp2buildDeps() []string { return FirstUniqueStrings(m.commonProperties.UnconvertedBp2buildDeps) } +// GetMissingBp2buildDeps eturns the list of module names that were not found in Android.bp files. +func (m *ModuleBase) GetMissingBp2buildDeps() []string { + return FirstUniqueStrings(m.commonProperties.MissingBp2buildDeps) +} + func (m *ModuleBase) AddJSONData(d *map[string]interface{}) { (*d)["Android"] = map[string]interface{}{} } diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index 54b59af3a..5887d06c2 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -324,6 +324,15 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers return } } + if unconvertedDeps := aModule.GetMissingBp2buildDeps(); len(unconvertedDeps) > 0 { + msg := fmt.Sprintf("%q depends on missing modules: %s", m.Name(), strings.Join(unconvertedDeps, ", ")) + if ctx.unconvertedDepMode == warnUnconvertedDeps { + metrics.moduleWithMissingDepsMsgs = append(metrics.moduleWithMissingDepsMsgs, msg) + } else if ctx.unconvertedDepMode == errorModulesUnconvertedDeps { + errs = append(errs, fmt.Errorf(msg)) + return + } + } targets = generateBazelTargets(bpCtx, aModule) for _, t := range targets { // A module can potentially generate more than 1 Bazel diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go index 1440b6fce..b21a47713 100644 --- a/bp2build/build_conversion_test.go +++ b/bp2build/build_conversion_test.go @@ -300,6 +300,19 @@ custom { }), }, }, + { + description: "non-existent dep", + blueprint: `custom { + name: "has_dep", + arch_paths: [":dep"], + bazel_module: { bp2build_available: true }, +}`, + expectedBazelTargets: []string{ + makeBazelTarget("custom", "has_dep", attrNameToString{ + "arch_paths": `[":dep__BP2BUILD__MISSING__DEP"]`, + }), + }, + }, { description: "arch-variant srcs", blueprint: `custom { diff --git a/bp2build/metrics.go b/bp2build/metrics.go index 68ac54435..557ea994c 100644 --- a/bp2build/metrics.go +++ b/bp2build/metrics.go @@ -30,6 +30,10 @@ type CodegenMetrics struct { // NOTE: NOT in the .proto moduleWithUnconvertedDepsMsgs []string + // List of modules with missing deps + // NOTE: NOT in the .proto + moduleWithMissingDepsMsgs []string + // List of converted modules convertedModules []string } @@ -54,13 +58,21 @@ func (metrics *CodegenMetrics) Print() { generatedTargetCount += count } fmt.Printf( - "[bp2build] Converted %d Android.bp modules to %d total generated BUILD targets. Included %d handcrafted BUILD targets. There are %d total Android.bp modules.\n%d converted modules have unconverted deps: \n\t%s", + `[bp2build] Converted %d Android.bp modules to %d total generated BUILD targets. Included %d handcrafted BUILD targets. There are %d total Android.bp modules. +%d converted modules have unconverted deps: + %s +%d converted modules have missing deps: + %s +`, metrics.generatedModuleCount, generatedTargetCount, metrics.handCraftedModuleCount, metrics.TotalModuleCount(), len(metrics.moduleWithUnconvertedDepsMsgs), - strings.Join(metrics.moduleWithUnconvertedDepsMsgs, "\n\t")) + strings.Join(metrics.moduleWithUnconvertedDepsMsgs, "\n\t"), + len(metrics.moduleWithMissingDepsMsgs), + strings.Join(metrics.moduleWithMissingDepsMsgs, "\n\t"), + ) } const bp2buildMetricsFilename = "bp2build_metrics.pb"