From 2173b5f578f95cc9b6ca05b4a0f481ca1df745b2 Mon Sep 17 00:00:00 2001 From: Chris Parsons Date: Mon, 25 Sep 2023 19:01:40 +0000 Subject: [PATCH] support aidl bp2build changes - Allowlist an aidl module by package, not name (to support a small module name change) - Implement some unit test framework changes which facilitate better aidl bp2build testing - Support a convenience function to add a load hook for registering a module as "has a bazel definition of a given target name" Bug: 301676937 Test: m bp2build, verified the aidl target was generated before/after this CL. Test: Presubmits Test: Ran bp2build progress and ensured that aidl_interface targets under frameworks/ continued to appear converted Change-Id: I62412057d6f61a2ce2bc39488c75af793eb14c94 --- android/allowlists/allowlists.go | 2 +- android/bazel.go | 22 ++++++++++++++++ bp2build/testing.go | 43 +++++++++++++++++++++++++++++--- 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/android/allowlists/allowlists.go b/android/allowlists/allowlists.go index 3a95627e8..caca5b03a 100644 --- a/android/allowlists/allowlists.go +++ b/android/allowlists/allowlists.go @@ -281,6 +281,7 @@ var ( "hardware/interfaces/configstore/1.0": Bp2BuildDefaultTrue, "hardware/interfaces/configstore/1.1": Bp2BuildDefaultTrue, "hardware/interfaces/configstore/utils": Bp2BuildDefaultTrue, + "hardware/interfaces/contexthub/aidl": Bp2BuildDefaultTrue, "hardware/interfaces/graphics/allocator/2.0": Bp2BuildDefaultTrue, "hardware/interfaces/graphics/allocator/3.0": Bp2BuildDefaultTrue, "hardware/interfaces/graphics/allocator/4.0": Bp2BuildDefaultTrue, @@ -905,7 +906,6 @@ var ( "libRSDispatch", // hal_unit_tests and deps - "android.hardware.contexthub_interface", // created implicitly by android.hardware.contexthub "chre_flatbuffers", "event_logger", "hal_unit_tests", diff --git a/android/bazel.go b/android/bazel.go index 451639628..5df12f02e 100644 --- a/android/bazel.go +++ b/android/bazel.go @@ -218,6 +218,28 @@ func InitBazelModule(module BazelModule) { module.bazelProps().Bazel_module.CanConvertToBazel = true } +// BazelHandcraftedHook is a load hook to possibly register the current module as +// a "handcrafted" Bazel target of a given name. If the current module should be +// registered in this way, the hook function should return the target name. If +// it should not be registered in this way, this function should return the empty string. +type BazelHandcraftedHook func(ctx LoadHookContext) string + +// AddBazelHandcraftedHook adds a load hook to (maybe) mark the given module so that +// it is treated by bp2build as if it has a handcrafted Bazel target. +func AddBazelHandcraftedHook(module BazelModule, hook BazelHandcraftedHook) { + AddLoadHook(module, func(ctx LoadHookContext) { + var targetName string = hook(ctx) + if len(targetName) > 0 { + moduleDir := ctx.ModuleDir() + if moduleDir == Bp2BuildTopLevel { + moduleDir = "" + } + label := fmt.Sprintf("//%s:%s", moduleDir, targetName) + module.bazelProps().Bazel_module.Label = &label + } + }) +} + // bazelProps returns the Bazel properties for the given BazelModuleBase. func (b *BazelModuleBase) bazelProps() *properties { return &b.bazelProperties diff --git a/bp2build/testing.go b/bp2build/testing.go index dfd4d573b..d26b346dd 100644 --- a/bp2build/testing.go +++ b/bp2build/testing.go @@ -27,6 +27,7 @@ import ( "strings" "testing" + "android/soong/ui/metrics/bp2build_metrics_proto" "github.com/google/blueprint/proptools" "android/soong/android" @@ -87,6 +88,15 @@ type Bp2buildTestCase struct { // ExpectedBazelTargets compares the BazelTargets generated in `Dir` (if not empty). // Otherwise, it checks the BazelTargets generated by `Blueprint` in the root directory. ExpectedBazelTargets []string + // ExpectedConvertedModules asserts that modules in this list are labeled as "converted + // by bp2build" in the metrics reported by bp2build. + ExpectedConvertedModules []string + // ExpectedHandcraftedModules asserts that modules in this list are labeled as "handcrafted + // in build files" in the metrics reported by bp2build. Such modules are either explicitly + // defined in a BUILD file (by name), or registered as "otherwise implicitly handled" + // by bp2build (for example, by macros owned by other modules). + ExpectedHandcraftedModules []string + // AlreadyExistingBuildContents, if non-empty, simulates an already-present source BUILD file // in the directory under test. The BUILD file has the given contents. This BUILD file // will also be treated as "BUILD file to keep" by the simulated bp2build environment. @@ -116,6 +126,16 @@ type Bp2buildTestCase struct { KeepBuildFileForDirs []string } +func RunBp2BuildTestCaseExtraContext(t *testing.T, registerModuleTypes func(ctx android.RegistrationContext), modifyContext func(ctx *android.TestContext), tc Bp2buildTestCase) { + t.Helper() + bp2buildSetup := android.GroupFixturePreparers( + android.FixtureRegisterWithContext(registerModuleTypes), + android.FixtureModifyContext(modifyContext), + SetBp2BuildTestRunner, + ) + runBp2BuildTestCaseWithSetup(t, bp2buildSetup, tc) +} + func RunBp2BuildTestCase(t *testing.T, registerModuleTypes func(ctx android.RegistrationContext), tc Bp2buildTestCase) { t.Helper() bp2buildSetup := android.GroupFixturePreparers( @@ -223,7 +243,7 @@ func runBp2BuildTestCaseWithSetup(t *testing.T, extraPreparer android.FixturePre checkDir: tc.ExpectedBazelTargets, } - result.CompareAllBazelTargets(t, tc.Description, expectedTargets, true) + result.CompareAllBazelTargets(t, tc, expectedTargets, true) } // SetBp2BuildTestRunner customizes the test fixture mechanism to run tests in Bp2Build mode. @@ -274,7 +294,7 @@ type BazelTestResult struct { // have a corresponding expected BazelTarget. // // If ignoreUnexpected=true then it will ignore directories for which there are no expected targets. -func (b BazelTestResult) CompareAllBazelTargets(t *testing.T, description string, expectedTargets map[string][]string, ignoreUnexpected bool) { +func (b BazelTestResult) CompareAllBazelTargets(t *testing.T, tc Bp2buildTestCase, expectedTargets map[string][]string, ignoreUnexpected bool) { t.Helper() actualTargets := b.buildFileToTargets @@ -301,7 +321,24 @@ func (b BazelTestResult) CompareAllBazelTargets(t *testing.T, description string t.Errorf("expected %d bazel modules in %q but did not find any", expectedCount, dir) } } else { - b.CompareBazelTargets(t, description, expected, actual) + b.CompareBazelTargets(t, tc.Description, expected, actual) + } + } + + for _, module := range tc.ExpectedConvertedModules { + if _, found := b.metrics.convertedModulePathMap[module]; !found { + t.Errorf("expected %s to be generated by bp2build, but was not. Map of converted modules: %s", module, b.metrics.convertedModulePathMap) + } + } + + for _, module := range tc.ExpectedHandcraftedModules { + if reason, found := b.metrics.serialized.UnconvertedModules[module]; !found { + t.Errorf("expected %s to be marked 'unconverted' by bp2build, but was not found. Full list: %s", + module, b.metrics.serialized.UnconvertedModules) + } else { + if reason.Type != bp2build_metrics_proto.UnconvertedReasonType_DEFINED_IN_BUILD_FILE { + t.Errorf("expected %s to be marked 'handcrafted' by bp2build, but was disabled for another reason: %s", module, reason) + } } } }