diff --git a/android/fixture.go b/android/fixture.go index dbc3bc5e0..03abb7f26 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 64b0207e7..5d277e1ac 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,49 @@ func (ctx *Context) RegisterForBazelConversion() { RegisterMutatorsForBazelConversion(ctx, bp2buildPreArchMutators) } +func (c *Context) ParseBuildFiles(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 { + err := parseBuildFile(path) + if err != nil { + return err + } + } + c.Config().SetBazelBuildFileTargets(result) + return nil +} + // RegisterForApiBazelConversion is similar to RegisterForBazelConversion except that // it only generates API targets in the generated workspace func (ctx *Context) RegisterForApiBazelConversion() { diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go index e127fd542..1a357438a 100644 --- a/bp2build/build_conversion_test.go +++ b/bp2build/build_conversion_test.go @@ -1946,3 +1946,36 @@ func TestPrettyPrintSelectMapEqualValues(t *testing.T) { actual, _ := prettyPrintAttribute(lla, 0) 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.", + }) +} diff --git a/bp2build/testing.go b/bp2build/testing.go index 140b214cf..89ef07bf8 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,12 @@ 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 + + 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. @@ -119,11 +125,22 @@ func RunApiBp2BuildTestCase(t *testing.T, registerModuleTypes func(ctx android.R 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) } + if len(tc.AlreadyExistingBuildContents) > 0 { + buildFilePath := filepath.Join(checkDir, "BUILD") + filesystem[buildFilePath] = []byte(tc.AlreadyExistingBuildContents) + keepExistingBuildDirs = append(keepExistingBuildDirs, checkDir) + buildFilesToParse = append(buildFilesToParse, buildFilePath) + } preparers := []android.FixturePreparer{ extraPreparer, @@ -132,7 +149,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( @@ -140,7 +157,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, }) @@ -150,6 +167,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.ParseBuildFiles(".", 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 { @@ -168,10 +189,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 989dd7fcd..69756778c 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" @@ -737,43 +736,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. @@ -782,7 +744,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.ParseBuildFiles(topDir, existingBazelFiles) + maybeQuit(err, "Error parsing existing Bazel-related files") }) // Propagate "allow misssing dependencies" bit. This is normally set in