Read BUILD files in bp2build

The parsed BUILD files will be scanned for obvious definitions of BUILD
targets which have Android.bp counterparts. In such cases, bp2build will
automatically omit conversion of these defined modules (to prevent
collisions). With this change, we no longer need one-off denylisting of
modules which have BUILD file definitions.

This has a 0.03s to 0.2s slowdown for bp2build with current state. This
impact is identical on a heavier test branch, as well. I also ran an
experiment that applied BUILD scanning to all source BUILD files
(regardless of allowlisting), and this had a 2 second slowdown.

We may want to look into parallelizing or improving the performance of
this evaluation, but it's probably not worth the effort at this time,
since the current performance hit is small.

Test: New integration test
Test: Removed libprotobuf-python from denylist and tested building the
package
Test: Treehugger

Change-Id: Ibde3bab12cd4a8fed642ad46e5344a56953bec91

android/allowlists/allowlists.go

@@ -461,6 +461,9 @@ var (
 
 		// TODO(b/266459895): remove this and the placeholder BUILD file after re-enabling libunwindstack
 		"external/rust/crates/rustc-demangle-capi":/* recursive = */ false,
+
+		// Used for testing purposes only. Should not actually exist in the real source tree.
+		"testpkg/keep_build_file":/* recursive = */ false,
 	}
 
 	Bp2buildModuleAlwaysConvertList = []string{
@@ -845,9 +848,6 @@ var (
 		"libbase_ndk",  // TODO(b/186826477): fails to link libctscamera2_jni for device (required for CtsCameraTestCases)
 		"bouncycastle", // TODO(b/274474005): Need support for custom system_modules.
 
-		// python protos
-		"libprotobuf-python", // Has a handcrafted alternative
-
 		// genrule incompatibilities
 		"brotli-fuzzer-corpus",                                       // TODO(b/202015218): outputs are in location incompatible with bazel genrule handling.
 		"platform_tools_properties", "build_tools_source_properties", // TODO(b/203369847): multiple genrules in the same package creating the same file

android/bazel.go

@@ -537,6 +537,12 @@ func registerBp2buildConversionMutator(ctx RegisterMutatorsContext) {
 }
 
 func convertWithBp2build(ctx TopDownMutatorContext) {
+	if ctx.Config().HasBazelBuildTargetInSource(ctx) {
+		// Defer to the BUILD target. Generating an additional target would
+		// cause a BUILD file conflict.
+		return
+	}
+
 	bModule, ok := ctx.Module().(Bazelable)
 	if !ok || !bModule.shouldConvertWithBp2build(ctx, ctx.Module()) {
 		return

android/config.go

@@ -291,6 +291,10 @@ type config struct {
 	// "--bazel-force-enabled-modules"
 	bazelForceEnabledModules map[string]struct{}
 
+	// Names of Bazel targets as defined by BUILD files in the source tree,
+	// keyed by the directory in which they are defined.
+	bazelTargetsByDir map[string][]string
+
 	// If true, for any requests to Bazel, communicate with a Bazel proxy using
 	// unix sockets, instead of spawning Bazel as a subprocess.
 	UseBazelProxy bool
@@ -2002,6 +2006,20 @@ func (c *config) LogMixedBuild(ctx BaseModuleContext, useBazel bool) {
 	}
 }
 
+func (c *config) HasBazelBuildTargetInSource(ctx BaseModuleContext) bool {
+	moduleName := ctx.Module().Name()
+	for _, buildTarget := range c.bazelTargetsByDir[ctx.ModuleDir()] {
+		if moduleName == buildTarget {
+			return true
+		}
+	}
+	return false
+}
+
+func (c *config) SetBazelBuildFileTargets(bazelTargetsByDir map[string][]string) {
+	c.bazelTargetsByDir = bazelTargetsByDir
+}
+
 // ApiSurfaces directory returns the source path inside the api_surfaces repo
 // (relative to workspace root).
 func (c *config) ApiSurfacesDir(s ApiSurface, version string) string {

cmd/soong_build/main.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"time"
 
@@ -762,6 +763,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.
@@ -769,6 +807,10 @@ func runBp2Build(ctx *android.Context, extraNinjaDeps []string, metricsDir strin
 	var codegenMetrics *bp2build.CodegenMetrics
 	ctx.EventHandler.Do("bp2build", func() {
 
+		ctx.EventHandler.Do("read_build", func() {
+			ctx.Config().SetBazelBuildFileTargets(buildTargetsByPackage(ctx))
+		})
+
 		// Propagate "allow misssing dependencies" bit. This is normally set in
 		// newContext(), but we create ctx without calling that method.
 		ctx.SetAllowMissingDependencies(ctx.Config().AllowMissingDependencies())

tests/bp2build_bazel_test.sh

@@ -232,6 +232,63 @@ function test_bp2build_generates_all_buildfiles {
   eval "${_save_trap}"
 }
 
+function test_build_files_take_precedence {
+  _save_trap=$(trap -p EXIT)
+  trap '[[ $? -ne 0 ]] && echo Are you running this locally? Try changing --sandbox_tmpfs_path to something other than /tmp/ in build/bazel/linux.bazelrc.' EXIT
+  _build_files_take_precedence
+  eval "${_save_trap}"
+}
+
+function _build_files_take_precedence {
+  setup
+
+  # This specific directory is hardcoded in bp2build as being one
+  # where the BUILD file should be intentionally kept.
+  mkdir -p testpkg/keep_build_file
+  cat > testpkg/keep_build_file/Android.bp <<'EOF'
+genrule {
+    name: "print_origin",
+    cmd: "echo 'from_soong' > $(out)",
+    out: [
+        "origin.txt",
+    ],
+    bazel_module: {
+        bp2build_available: true,
+    },
+  }
+EOF
+
+  run_soong bp2build
+  run_bazel build --config=android --config=bp2build --config=ci //testpkg/keep_build_file:print_origin
+
+  local -r output_file="$(find -L bazel-out -name origin.txt)"
+  if [[ ! -f "${output_file}" ]]; then
+    fail "Expected origin.txt to be generated, but was missing"
+  fi
+  if ! grep from_soong "${output_file}"; then
+    fail "Expected to find 'from_soong' in '${output_file}'"
+  fi
+
+  cat > testpkg/keep_build_file/BUILD.bazel <<'EOF'
+genrule(
+    name = "print_origin",
+    outs = ["origin.txt"],
+    cmd = "echo 'from_bazel' > $@",
+)
+EOF
+
+  # Clean the workspace. There is a test infrastructure bug where run_bazel
+  # will symlink Android.bp files in the source directory again and thus
+  # pollute the workspace.
+  # TODO: b/286059878 - Remove this clean after the underlying bug is fixed.
+  run_soong clean
+  run_soong bp2build
+  run_bazel build --config=android --config=bp2build --config=ci //testpkg/keep_build_file:print_origin
+  if ! grep from_bazel "${output_file}"; then
+    fail "Expected to find 'from_bazel' in '${output_file}'"
+  fi
+}
+
 function test_bp2build_symlinks_files {
   setup
   mkdir -p foo