Merge "Revert "Add unit test for parsing build files in bp2build""

android/fixture.go

@@ -275,15 +275,6 @@ 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,13 +15,9 @@
 package android
 
 import (
-	"bufio"
 	"fmt"
-	"path/filepath"
 	"reflect"
-	"regexp"
 
-	"android/soong/shared"
 	"github.com/google/blueprint"
 )
 
@@ -201,49 +197,6 @@ 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() {

bp2build/build_conversion_test.go

@@ -1946,36 +1946,3 @@ 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.",
-	})
-}

bp2build/testing.go

@@ -21,7 +21,6 @@ specific-but-shared functionality among tests in package
 
 import (
 	"fmt"
-	"path/filepath"
 	"sort"
 	"strings"
 	"testing"
@@ -83,12 +82,7 @@ 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
-	// 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
+	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.
@@ -125,22 +119,11 @@ func RunApiBp2BuildTestCase(t *testing.T, registerModuleTypes func(ctx android.R
 
 func runBp2BuildTestCaseWithSetup(t *testing.T, extraPreparer android.FixturePreparer, tc Bp2buildTestCase) {
 	t.Helper()
-	checkDir := "."
-	if tc.Dir != "" {
-		checkDir = tc.Dir
-	}
-	keepExistingBuildDirs := tc.KeepBuildFileForDirs
-	buildFilesToParse := []string{}
+	dir := "."
 	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,
@@ -149,7 +132,7 @@ func runBp2BuildTestCaseWithSetup(t *testing.T, extraPreparer android.FixturePre
 		android.FixtureRegisterWithContext(func(ctx android.RegistrationContext) {
 			ctx.RegisterModuleType(tc.ModuleTypeUnderTest, tc.ModuleTypeUnderTestFactory)
 		}),
-		android.FixtureModifyContextWithMockFs(func(ctx *android.TestContext) {
+		android.FixtureModifyContext(func(ctx *android.TestContext) {
 			// A default configuration for tests to not have to specify bp2build_available on top level
 			// targets.
 			bp2buildConfig := android.NewBp2BuildAllowlist().SetDefaultConfig(
@@ -157,7 +140,7 @@ func runBp2BuildTestCaseWithSetup(t *testing.T, extraPreparer android.FixturePre
 					android.Bp2BuildTopLevel: allowlists.Bp2BuildDefaultTrueRecursively,
 				},
 			)
-			for _, f := range keepExistingBuildDirs {
+			for _, f := range tc.KeepBuildFileForDirs {
 				bp2buildConfig.SetKeepExistingBuildFile(map[string]bool{
 					f: /*recursive=*/ false,
 				})
@@ -167,10 +150,6 @@ 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 {
@@ -189,6 +168,10 @@ 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,6 +21,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"time"
 
@@ -740,6 +741,43 @@ 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.
@@ -748,11 +786,7 @@ func runBp2Build(ctx *android.Context, extraNinjaDeps []string, metricsDir strin
 	ctx.EventHandler.Do("bp2build", func() {
 
 		ctx.EventHandler.Do("read_build", func() {
-			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")
+			ctx.Config().SetBazelBuildFileTargets(buildTargetsByPackage(ctx))
 		})
 
 		// Propagate "allow misssing dependencies" bit. This is normally set in