Make BUILD file merging slightly smarter.

This change enables checked-in BUILD files like
prebuilts/clang/host/linux-x86/BUILD.bazel to be merged cleanly with the
bp2build generated one into the synthetic workspace.

The checked-in BUILD file contains a package() declaration that bp2build
also generates. To avoid double declaration, the BUILD file writer now
checks if the BazelTargets contain handcrafted targets. If so, it
delegates the package declaration to the handcrafted BUILD file instead.

This change also sorts the bp2build targets before the handcrafted ones,
and adds a section header to demarcate the two sets of targets.

Test: TH
Change-Id: I3ecdeaab3226b895b623daf0791d24a657f7a7c6

android/bazel.go

@@ -138,7 +138,6 @@ var (
 		// e.g. ERROR: Analysis of target '@soong_injection//mixed_builds:buildroot' failed
 		"external/bazelbuild-rules_android":/* recursive = */ true,
 
-		"prebuilts/clang/host/linux-x86":/* recursive = */ false,
 		"prebuilts/sdk":/* recursive = */ false,
 		"prebuilts/sdk/tools":/* recursive = */ false,
 	}
@@ -155,6 +154,7 @@ var (
 		"external/fmtlib":                 Bp2BuildDefaultTrueRecursively,
 		"external/arm-optimized-routines": Bp2BuildDefaultTrueRecursively,
 		"external/scudo":                  Bp2BuildDefaultTrueRecursively,
+		"prebuilts/clang/host/linux-x86":  Bp2BuildDefaultTrueRecursively,
 	}
 
 	// Per-module denylist to always opt modules out of both bp2build and mixed builds.

bp2build/build_conversion.go

@@ -19,6 +19,7 @@ import (
 	"android/soong/bazel"
 	"fmt"
 	"reflect"
+	"sort"
 	"strings"
 
 	"github.com/google/blueprint"
@@ -34,6 +35,7 @@ type BazelTarget struct {
 	content         string
 	ruleClass       string
 	bzlLoadLocation string
+	handcrafted     bool
 }
 
 // IsLoadedFromStarlark determines if the BazelTarget's rule class is loaded from a .bzl file,
@@ -45,12 +47,47 @@ func (t BazelTarget) IsLoadedFromStarlark() bool {
 // BazelTargets is a typedef for a slice of BazelTarget objects.
 type BazelTargets []BazelTarget
 
+// HasHandcraftedTargetsreturns true if a set of bazel targets contain
+// handcrafted ones.
+func (targets BazelTargets) hasHandcraftedTargets() bool {
+	for _, target := range targets {
+		if target.handcrafted {
+			return true
+		}
+	}
+	return false
+}
+
+// sort a list of BazelTargets in-place, by name, and by generated/handcrafted types.
+func (targets BazelTargets) sort() {
+	sort.Slice(targets, func(i, j int) bool {
+		if targets[i].handcrafted != targets[j].handcrafted {
+			// Handcrafted targets will be generated after the bp2build generated targets.
+			return targets[j].handcrafted
+		}
+		// This will cover all bp2build generated targets.
+		return targets[i].name < targets[j].name
+	})
+}
+
 // String returns the string representation of BazelTargets, without load
 // statements (use LoadStatements for that), since the targets are usually not
 // adjacent to the load statements at the top of the BUILD file.
 func (targets BazelTargets) String() string {
 	var res string
 	for i, target := range targets {
+		// There is only at most 1 handcrafted "target", because its contents
+		// represent the entire BUILD file content from the tree. See
+		// build_conversion.go#getHandcraftedBuildContent for more information.
+		//
+		// Add a header to make it easy to debug where the handcrafted targets
+		// are in a generated BUILD file.
+		if target.handcrafted {
+			res += "# -----------------------------\n"
+			res += "# Section: Handcrafted targets. \n"
+			res += "# -----------------------------\n\n"
+		}
+
 		res += target.content
 		if i != len(targets)-1 {
 			res += "\n\n"
@@ -267,7 +304,8 @@ func getHandcraftedBuildContent(ctx *CodegenContext, b android.Bazelable, pathTo
 	}
 	// TODO(b/181575318): once this is more targeted, we need to include name, rule class, etc
 	return BazelTarget{
-		content: c,
+		content:     c,
+		handcrafted: true,
 	}, nil
 }
 
@@ -294,6 +332,7 @@ func generateBazelTarget(ctx bpToBuildContext, m blueprint.Module, btm android.B
 			targetName,
 			attributes,
 		),
+		handcrafted: false,
 	}
 }
 

bp2build/build_conversion_test.go

@@ -1452,53 +1452,61 @@ filegroup {
 
 	dir := "."
 	for _, testCase := range testCases {
-		fs := make(map[string][]byte)
-		toParse := []string{
-			"Android.bp",
-		}
-		for f, content := range testCase.fs {
-			if strings.HasSuffix(f, "Android.bp") {
-				toParse = append(toParse, f)
+		t.Run(testCase.description, func(t *testing.T) {
+			fs := make(map[string][]byte)
+			toParse := []string{
+				"Android.bp",
 			}
-			fs[f] = []byte(content)
-		}
-		config := android.TestConfig(buildDir, nil, testCase.bp, fs)
-		ctx := android.NewTestContext(config)
-		ctx.RegisterModuleType(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestFactory)
-		for _, m := range testCase.depsMutators {
-			ctx.DepsBp2BuildMutators(m)
-		}
-		ctx.RegisterBp2BuildMutator(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestBp2BuildMutator)
-		ctx.RegisterForBazelConversion()
+			for f, content := range testCase.fs {
+				if strings.HasSuffix(f, "Android.bp") {
+					toParse = append(toParse, f)
+				}
+				fs[f] = []byte(content)
+			}
+			config := android.TestConfig(buildDir, nil, testCase.bp, fs)
+			ctx := android.NewTestContext(config)
+			ctx.RegisterModuleType(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestFactory)
+			for _, m := range testCase.depsMutators {
+				ctx.DepsBp2BuildMutators(m)
+			}
+			ctx.RegisterBp2BuildMutator(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestBp2BuildMutator)
+			ctx.RegisterForBazelConversion()
 
-		_, errs := ctx.ParseFileList(dir, toParse)
-		if errored(t, testCase.description, errs) {
-			continue
-		}
-		_, errs = ctx.ResolveDependencies(config)
-		if errored(t, testCase.description, errs) {
-			continue
-		}
+			_, errs := ctx.ParseFileList(dir, toParse)
+			if errored(t, testCase.description, errs) {
+				return
+			}
+			_, errs = ctx.ResolveDependencies(config)
+			if errored(t, testCase.description, errs) {
+				return
+			}
 
-		checkDir := dir
-		if testCase.dir != "" {
-			checkDir = testCase.dir
-		}
-		bazelTargets := generateBazelTargetsForDir(NewCodegenContext(config, *ctx.Context, Bp2Build), checkDir)
-		if actualCount, expectedCount := len(bazelTargets), len(testCase.expectedBazelTargets); actualCount != expectedCount {
-			t.Errorf("%s: Expected %d bazel target, got %d\n%s", testCase.description, expectedCount, actualCount, bazelTargets)
-		} else {
+			checkDir := dir
+			if testCase.dir != "" {
+				checkDir = testCase.dir
+			}
+			bazelTargets := generateBazelTargetsForDir(NewCodegenContext(config, *ctx.Context, Bp2Build), checkDir)
+			bazelTargets.sort()
+			actualCount := len(bazelTargets)
+			expectedCount := len(testCase.expectedBazelTargets)
+			if actualCount != expectedCount {
+				t.Errorf("Expected %d bazel target, got %d\n%s", expectedCount, actualCount, bazelTargets)
+			}
+			if !strings.Contains(bazelTargets.String(), "Section: Handcrafted targets. ") {
+				t.Errorf("Expected string representation of bazelTargets to contain handcrafted section header.")
+			}
 			for i, target := range bazelTargets {
-				if w, g := testCase.expectedBazelTargets[i], target.content; w != g {
+				actualContent := target.content
+				expectedContent := testCase.expectedBazelTargets[i]
+				if expectedContent != actualContent {
 					t.Errorf(
-						"%s: Expected generated Bazel target to be '%s', got '%s'",
-						testCase.description,
-						w,
-						g,
+						"Expected generated Bazel target to be '%s', got '%s'",
+						expectedContent,
+						actualContent,
 					)
 				}
 			}
-		}
+		})
 	}
 }
 

bp2build/conversion.go

@@ -5,7 +5,6 @@ import (
 	"android/soong/cc/config"
 	"fmt"
 	"reflect"
-	"sort"
 	"strings"
 
 	"github.com/google/blueprint/proptools"
@@ -64,22 +63,28 @@ func createBuildFiles(buildToTargets map[string]BazelTargets, mode CodegenMode)
 			continue
 		}
 		targets := buildToTargets[dir]
-		sort.Slice(targets, func(i, j int) bool {
-			// this will cover all bp2build generated targets
-			if targets[i].name < targets[j].name {
-				return true
-			}
-			// give a strict ordering to content from hand-crafted targets
-			return targets[i].content < targets[j].content
-		})
-		content := soongModuleLoad
+		targets.sort()
+
+		var content string
 		if mode == Bp2Build {
-			content = `# This file was automatically generated by bp2build for the Bazel migration project.
-# Feel free to edit or test it, but do *not* check it into your version control system.`
-			content += "\n\n"
-			content += "package(default_visibility = [\"//visibility:public\"])"
-			content += "\n\n"
+			content = `# READ THIS FIRST:
+# This file was automatically generated by bp2build for the Bazel migration project.
+# Feel free to edit or test it, but do *not* check it into your version control system.
+`
+			if targets.hasHandcraftedTargets() {
+				// For BUILD files with both handcrafted and generated targets,
+				// don't hardcode actual content, like package() declarations.
+				// Leave that responsibility to the checked-in BUILD file
+				// instead.
+				content += `# This file contains generated targets and handcrafted targets that are manually managed in the source tree.`
+			} else {
+				// For fully-generated BUILD files, hardcode the default visibility.
+				content += "package(default_visibility = [\"//visibility:public\"])"
+			}
+			content += "\n"
 			content += targets.LoadStatements()
+		} else if mode == QueryView {
+			content = soongModuleLoad
 		}
 		if content != "" {
 			// If there are load statements, add a couple of newlines.