Merge "Add unit test for parsing build files in bp2build" into main

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) })
 }

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() {

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()

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",
+		StubbedBuildDefinitions: []string{"shared_dep"},
-		Filesystem:  map[string]string{},
+		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",
+		StubbedBuildDefinitions: []string{"shared_dep", "shared_dep2", "shared_dep3"},
-		Filesystem:  map[string]string{},
+		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",

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,
 	}

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