diff --git a/android/fixture.go b/android/fixture.go index 6660afd65..5ad47e8c9 100644 --- a/android/fixture.go +++ b/android/fixture.go @@ -275,6 +275,15 @@ func FixtureModifyContext(mutator func(ctx *TestContext)) FixturePreparer { }) } +// Sync the mock filesystem with the current config, then modify the context, +// This allows context modification that requires filesystem access. +func FixtureModifyContextWithMockFs(mutator func(ctx *TestContext)) FixturePreparer { + return newSimpleFixturePreparer(func(f *fixture) { + f.config.mockFileSystem("", f.mockFS) + mutator(f.ctx) + }) +} + func FixtureRegisterWithContext(registeringFunc func(ctx RegistrationContext)) FixturePreparer { return FixtureModifyContext(func(ctx *TestContext) { registeringFunc(ctx) }) } diff --git a/android/register.go b/android/register.go index df97c75e4..f1c298691 100644 --- a/android/register.go +++ b/android/register.go @@ -15,9 +15,13 @@ package android import ( + "bufio" "fmt" + "path/filepath" "reflect" + "regexp" + "android/soong/shared" "github.com/google/blueprint" ) @@ -197,6 +201,56 @@ func (ctx *Context) RegisterForBazelConversion() { RegisterMutatorsForBazelConversion(ctx, bp2buildPreArchMutators) } +// RegisterExistingBazelTargets reads Bazel BUILD.bazel and BUILD files under +// the workspace, and returns a map containing names of Bazel targets defined in +// these BUILD files. +// For example, maps "//foo/bar" to ["baz", "qux"] if `//foo/bar:{baz,qux}` exist. +func (c *Context) RegisterExistingBazelTargets(topDir string, existingBazelFiles []string) error { + result := map[string][]string{} + + // Search for instances of `name = "$NAME"` (with arbitrary spacing). + targetNameRegex := regexp.MustCompile(`(?m)^\s*name\s*=\s*\"([^\"]+)\"`) + + parseBuildFile := func(path string) error { + fullPath := shared.JoinPath(topDir, path) + sourceDir := filepath.Dir(path) + + fileInfo, err := c.Config().fs.Stat(fullPath) + if err != nil { + return fmt.Errorf("Error accessing Bazel file '%s': %s", path, err) + } + if !fileInfo.IsDir() && + (fileInfo.Name() == "BUILD" || fileInfo.Name() == "BUILD.bazel") { + f, err := c.Config().fs.Open(fullPath) + if err != nil { + return fmt.Errorf("Error reading Bazel file '%s': %s", path, err) + } + defer f.Close() + scanner := bufio.NewScanner(f) + for scanner.Scan() { + line := scanner.Text() + matches := targetNameRegex.FindAllStringSubmatch(line, -1) + for _, match := range matches { + result[sourceDir] = append(result[sourceDir], match[1]) + } + } + } + return nil + } + + for _, path := range existingBazelFiles { + if !c.Config().Bp2buildPackageConfig.ShouldKeepExistingBuildFileForDir(filepath.Dir(path)) { + continue + } + err := parseBuildFile(path) + if err != nil { + return err + } + } + c.Config().SetBazelBuildFileTargets(result) + return nil +} + // Register the pipeline of singletons, module types, and mutators for // generating build.ninja and other files for Kati, from Android.bp files. func (ctx *Context) Register() { diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go index 3887c5da9..3c2673425 100644 --- a/bp2build/build_conversion_test.go +++ b/bp2build/build_conversion_test.go @@ -1926,6 +1926,69 @@ func TestPrettyPrintSelectMapEqualValues(t *testing.T) { android.AssertStringEquals(t, "Print the common value if all keys in an axis have the same value", `[":libfoo.impl"]`, actual) } +func TestAlreadyPresentBuildTarget(t *testing.T) { + bp := ` + custom { + name: "foo", + } + custom { + name: "bar", + } + ` + alreadyPresentBuildFile := + MakeBazelTarget( + "custom", + "foo", + AttrNameToString{}, + ) + expectedBazelTargets := []string{ + MakeBazelTarget( + "custom", + "bar", + AttrNameToString{}, + ), + } + registerCustomModule := func(ctx android.RegistrationContext) { + ctx.RegisterModuleType("custom", customModuleFactoryHostAndDevice) + } + RunBp2BuildTestCase(t, registerCustomModule, Bp2buildTestCase{ + AlreadyExistingBuildContents: alreadyPresentBuildFile, + Blueprint: bp, + ExpectedBazelTargets: expectedBazelTargets, + Description: "Not duplicating work for an already-present BUILD target.", + }) +} + +// Verifies that if a module is defined in pkg1/Android.bp, that a target present +// in pkg2/BUILD.bazel does not result in the module being labeled "already defined +// in a BUILD file". +func TestBuildTargetPresentOtherDirectory(t *testing.T) { + bp := ` + custom { + name: "foo", + } + ` + expectedBazelTargets := []string{ + MakeBazelTarget( + "custom", + "foo", + AttrNameToString{}, + ), + } + registerCustomModule := func(ctx android.RegistrationContext) { + ctx.RegisterModuleType("custom", customModuleFactoryHostAndDevice) + } + RunBp2BuildTestCase(t, registerCustomModule, Bp2buildTestCase{ + KeepBuildFileForDirs: []string{"other_pkg"}, + Filesystem: map[string]string{ + "other_pkg/BUILD.bazel": MakeBazelTarget("custom", "foo", AttrNameToString{}), + }, + Blueprint: bp, + ExpectedBazelTargets: expectedBazelTargets, + Description: "Not treating a BUILD target as the bazel definition for a module in another package", + }) +} + // If CommonAttributes.Dir is set, the bazel target should be created in that dir func TestCreateBazelTargetInDifferentDir(t *testing.T) { t.Parallel() diff --git a/bp2build/cc_library_shared_conversion_test.go b/bp2build/cc_library_shared_conversion_test.go index 44b97227e..9b533f397 100644 --- a/bp2build/cc_library_shared_conversion_test.go +++ b/bp2build/cc_library_shared_conversion_test.go @@ -218,12 +218,12 @@ cc_library_shared { func TestCcLibrarySharedOsSpecificSharedLib(t *testing.T) { runCcLibrarySharedTestCase(t, Bp2buildTestCase{ - Description: "cc_library_shared os-specific shared_libs", - Filesystem: map[string]string{}, + StubbedBuildDefinitions: []string{"shared_dep"}, + Description: "cc_library_shared os-specific shared_libs", + Filesystem: map[string]string{}, Blueprint: soongCcLibrarySharedPreamble + ` cc_library_shared { name: "shared_dep", - bazel_module: { bp2build_available: false }, } cc_library_shared { name: "foo_shared", @@ -243,20 +243,18 @@ cc_library_shared { func TestCcLibrarySharedBaseArchOsSpecificSharedLib(t *testing.T) { runCcLibrarySharedTestCase(t, Bp2buildTestCase{ - Description: "cc_library_shared base, arch, and os-specific shared_libs", - Filesystem: map[string]string{}, + StubbedBuildDefinitions: []string{"shared_dep", "shared_dep2", "shared_dep3"}, + Description: "cc_library_shared base, arch, and os-specific shared_libs", + Filesystem: map[string]string{}, Blueprint: soongCcLibrarySharedPreamble + ` cc_library_shared { name: "shared_dep", - bazel_module: { bp2build_available: false }, } cc_library_shared { name: "shared_dep2", - bazel_module: { bp2build_available: false }, } cc_library_shared { name: "shared_dep3", - bazel_module: { bp2build_available: false }, } cc_library_shared { name: "foo_shared", diff --git a/bp2build/testing.go b/bp2build/testing.go index 997df64a4..7333c0b50 100644 --- a/bp2build/testing.go +++ b/bp2build/testing.go @@ -21,6 +21,7 @@ specific-but-shared functionality among tests in package import ( "fmt" + "path/filepath" "sort" "strings" "testing" @@ -82,7 +83,16 @@ 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 - Filesystem map[string]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. + AlreadyExistingBuildContents string + // StubbedBuildDefinitions, if non-empty, adds stub definitions to an already-present source + // BUILD file in the directory under test, for each target name given. These stub definitions + // are added to the BUILD file given in AlreadyExistingBuildContents, if it is set. + StubbedBuildDefinitions []string + + Filesystem map[string]string // Dir sets the directory which will be compared against the targets in ExpectedBazelTargets. // This should used in conjunction with the Filesystem property to check for targets // generated from a directory that is not the root. @@ -110,11 +120,31 @@ func RunBp2BuildTestCase(t *testing.T, registerModuleTypes func(ctx android.Regi func runBp2BuildTestCaseWithSetup(t *testing.T, extraPreparer android.FixturePreparer, tc Bp2buildTestCase) { t.Helper() - dir := "." + checkDir := "." + if tc.Dir != "" { + checkDir = tc.Dir + } + keepExistingBuildDirs := tc.KeepBuildFileForDirs + buildFilesToParse := []string{} filesystem := make(map[string][]byte) for f, content := range tc.Filesystem { filesystem[f] = []byte(content) } + alreadyExistingBuildContents := tc.AlreadyExistingBuildContents + if len(tc.StubbedBuildDefinitions) > 0 { + for _, targetName := range tc.StubbedBuildDefinitions { + alreadyExistingBuildContents += MakeBazelTarget( + "bp2build_test_stub", + targetName, + AttrNameToString{}) + } + } + if len(alreadyExistingBuildContents) > 0 { + buildFilePath := filepath.Join(checkDir, "BUILD") + filesystem[buildFilePath] = []byte(alreadyExistingBuildContents) + keepExistingBuildDirs = append(keepExistingBuildDirs, checkDir) + buildFilesToParse = append(buildFilesToParse, buildFilePath) + } preparers := []android.FixturePreparer{ extraPreparer, @@ -123,7 +153,7 @@ func runBp2BuildTestCaseWithSetup(t *testing.T, extraPreparer android.FixturePre android.FixtureRegisterWithContext(func(ctx android.RegistrationContext) { ctx.RegisterModuleType(tc.ModuleTypeUnderTest, tc.ModuleTypeUnderTestFactory) }), - android.FixtureModifyContext(func(ctx *android.TestContext) { + android.FixtureModifyContextWithMockFs(func(ctx *android.TestContext) { // A default configuration for tests to not have to specify bp2build_available on top level // targets. bp2buildConfig := android.NewBp2BuildAllowlist().SetDefaultConfig( @@ -131,7 +161,7 @@ func runBp2BuildTestCaseWithSetup(t *testing.T, extraPreparer android.FixturePre android.Bp2BuildTopLevel: allowlists.Bp2BuildDefaultTrueRecursively, }, ) - for _, f := range tc.KeepBuildFileForDirs { + for _, f := range keepExistingBuildDirs { bp2buildConfig.SetKeepExistingBuildFile(map[string]bool{ f: /*recursive=*/ false, }) @@ -141,6 +171,10 @@ func runBp2BuildTestCaseWithSetup(t *testing.T, extraPreparer android.FixturePre // from cloning modules to their original state after mutators run. This // would lose some data intentionally set by these mutators. ctx.SkipCloneModulesAfterMutators = true + err := ctx.RegisterExistingBazelTargets(".", buildFilesToParse) + if err != nil { + t.Errorf("error parsing build files in test setup: %s", err) + } }), android.FixtureModifyEnv(func(env map[string]string) { if tc.UnconvertedDepsMode == errorModulesUnconvertedDeps { @@ -159,10 +193,6 @@ func runBp2BuildTestCaseWithSetup(t *testing.T, extraPreparer android.FixturePre return } - checkDir := dir - if tc.Dir != "" { - checkDir = tc.Dir - } expectedTargets := map[string][]string{ checkDir: tc.ExpectedBazelTargets, } diff --git a/cmd/soong_build/main.go b/cmd/soong_build/main.go index 626f076a3..d20847bb2 100644 --- a/cmd/soong_build/main.go +++ b/cmd/soong_build/main.go @@ -21,7 +21,6 @@ import ( "fmt" "os" "path/filepath" - "regexp" "strings" "time" @@ -617,43 +616,6 @@ func excludedFromSymlinkForest(ctx *android.Context, verbose bool) []string { return excluded } -// buildTargetsByPackage parses Bazel BUILD.bazel and BUILD files under -// the workspace, and returns a map containing names of Bazel targets defined in -// these BUILD files. -// For example, maps "//foo/bar" to ["baz", "qux"] if `//foo/bar:{baz,qux}` exist. -func buildTargetsByPackage(ctx *android.Context) map[string][]string { - existingBazelFiles, err := getExistingBazelRelatedFiles(topDir) - maybeQuit(err, "Error determining existing Bazel-related files") - - result := map[string][]string{} - - // Search for instances of `name = "$NAME"` (with arbitrary spacing). - targetNameRegex := regexp.MustCompile(`(?m)^\s*name\s*=\s*\"([^\"]+)\"`) - - for _, path := range existingBazelFiles { - if !ctx.Config().Bp2buildPackageConfig.ShouldKeepExistingBuildFileForDir(filepath.Dir(path)) { - continue - } - fullPath := shared.JoinPath(topDir, path) - sourceDir := filepath.Dir(path) - fileInfo, err := os.Stat(fullPath) - maybeQuit(err, "Error accessing Bazel file '%s'", fullPath) - - if !fileInfo.IsDir() && - (fileInfo.Name() == "BUILD" || fileInfo.Name() == "BUILD.bazel") { - // Process this BUILD file. - buildFileContent, err := os.ReadFile(fullPath) - maybeQuit(err, "Error reading Bazel file '%s'", fullPath) - - matches := targetNameRegex.FindAllStringSubmatch(string(buildFileContent), -1) - for _, match := range matches { - result[sourceDir] = append(result[sourceDir], match[1]) - } - } - } - return result -} - // Run Soong in the bp2build mode. This creates a standalone context that registers // an alternate pipeline of mutators and singletons specifically for generating // Bazel BUILD files instead of Ninja files. @@ -662,7 +624,11 @@ func runBp2Build(ctx *android.Context, extraNinjaDeps []string, metricsDir strin ctx.EventHandler.Do("bp2build", func() { ctx.EventHandler.Do("read_build", func() { - ctx.Config().SetBazelBuildFileTargets(buildTargetsByPackage(ctx)) + existingBazelFiles, err := getExistingBazelRelatedFiles(topDir) + maybeQuit(err, "Error determining existing Bazel-related files") + + err = ctx.RegisterExistingBazelTargets(topDir, existingBazelFiles) + maybeQuit(err, "Error parsing existing Bazel-related files") }) // Propagate "allow misssing dependencies" bit. This is normally set in