Merge changes I3ecdeaab,I43720641

* changes:
  Make BUILD file merging slightly smarter.
  bp2build/b: exit early in GENERATE_BAZEL_FILES=1.

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)
+		t.Run(testCase.description, func(t *testing.T) {
-		toParse := []string{
+			fs := make(map[string][]byte)
-			"Android.bp",
+			toParse := []string{
-		}
+				"Android.bp",
-		for f, content := range testCase.fs {
-			if strings.HasSuffix(f, "Android.bp") {
-				toParse = append(toParse, f)
 			}
-			fs[f] = []byte(content)
+			for f, content := range testCase.fs {
-		}
+				if strings.HasSuffix(f, "Android.bp") {
-		config := android.TestConfig(buildDir, nil, testCase.bp, fs)
+					toParse = append(toParse, f)
-		ctx := android.NewTestContext(config)
+				}
-		ctx.RegisterModuleType(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestFactory)
+				fs[f] = []byte(content)
-		for _, m := range testCase.depsMutators {
+			}
-			ctx.DepsBp2BuildMutators(m)
+			config := android.TestConfig(buildDir, nil, testCase.bp, fs)
-		}
+			ctx := android.NewTestContext(config)
-		ctx.RegisterBp2BuildMutator(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestBp2BuildMutator)
+			ctx.RegisterModuleType(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestFactory)
-		ctx.RegisterForBazelConversion()
+			for _, m := range testCase.depsMutators {
+				ctx.DepsBp2BuildMutators(m)
+			}
+			ctx.RegisterBp2BuildMutator(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestBp2BuildMutator)
+			ctx.RegisterForBazelConversion()
 
-		_, errs := ctx.ParseFileList(dir, toParse)
+			_, errs := ctx.ParseFileList(dir, toParse)
-		if errored(t, testCase.description, errs) {
+			if errored(t, testCase.description, errs) {
-			continue
+				return
-		}
+			}
-		_, errs = ctx.ResolveDependencies(config)
+			_, errs = ctx.ResolveDependencies(config)
-		if errored(t, testCase.description, errs) {
+			if errored(t, testCase.description, errs) {
-			continue
+				return
-		}
+			}
 
-		checkDir := dir
+			checkDir := dir
-		if testCase.dir != "" {
+			if testCase.dir != "" {
-			checkDir = testCase.dir
+				checkDir = testCase.dir
-		}
+			}
-		bazelTargets := generateBazelTargetsForDir(NewCodegenContext(config, *ctx.Context, Bp2Build), checkDir)
+			bazelTargets := generateBazelTargetsForDir(NewCodegenContext(config, *ctx.Context, Bp2Build), checkDir)
-		if actualCount, expectedCount := len(bazelTargets), len(testCase.expectedBazelTargets); actualCount != expectedCount {
+			bazelTargets.sort()
-			t.Errorf("%s: Expected %d bazel target, got %d\n%s", testCase.description, expectedCount, actualCount, bazelTargets)
+			actualCount := len(bazelTargets)
-		} else {
+			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'",
+						"Expected generated Bazel target to be '%s', got '%s'",
-						testCase.description,
+						expectedContent,
-						w,
+						actualContent,
-						g,
 					)
 				}
 			}
-		}
+		})
 	}
 }
 

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 {
+		targets.sort()
-			// this will cover all bp2build generated targets
+
-			if targets[i].name < targets[j].name {
+		var content string
-				return true
-			}
-			// give a strict ordering to content from hand-crafted targets
-			return targets[i].content < targets[j].content
-		})
-		content := soongModuleLoad
 		if mode == Bp2Build {
-			content = `# This file was automatically generated by bp2build for the Bazel migration project.
+			content = `# READ THIS FIRST:
-# Feel free to edit or test it, but do *not* check it into your version control system.`
+# This file was automatically generated by bp2build for the Bazel migration project.
-			content += "\n\n"
+# Feel free to edit or test it, but do *not* check it into your version control system.
-			content += "package(default_visibility = [\"//visibility:public\"])"
+`
-			content += "\n\n"
+			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.

cmd/soong_build/main.go

@@ -175,6 +175,9 @@ func writeJsonModuleGraph(configuration android.Config, ctx *android.Context, pa
 	writeFakeNinjaFile(extraNinjaDeps, configuration.BuildDir())
 }
 
+// doChosenActivity runs Soong for a specific activity, like bp2build, queryview
+// or the actual Soong build for the build.ninja file. Returns the top level
+// output file of the specific activity.
 func doChosenActivity(configuration android.Config, extraNinjaDeps []string) string {
 	bazelConversionRequested := bp2buildMarker != ""
 	mixedModeBuild := configuration.BazelContext.BazelEnabled()
@@ -187,11 +190,7 @@ func doChosenActivity(configuration android.Config, extraNinjaDeps []string) str
 		// Run the alternate pipeline of bp2build mutators and singleton to convert
 		// Blueprint to BUILD files before everything else.
 		runBp2Build(configuration, extraNinjaDeps)
-		if bp2buildMarker != "" {
+		return bp2buildMarker
-			return bp2buildMarker
-		} else {
-			return bootstrap.CmdlineArgs.OutFile
-		}
 	}
 
 	ctx := newContext(configuration, prepareBuildActions)
@@ -327,13 +326,13 @@ func writeFakeNinjaFile(extraNinjaDeps []string, buildDir string) {
 
 	ninjaFileName := "build.ninja"
 	ninjaFile := shared.JoinPath(topDir, buildDir, ninjaFileName)
-	ninjaFileD := shared.JoinPath(topDir, buildDir, ninjaFileName)
+	ninjaFileD := shared.JoinPath(topDir, buildDir, ninjaFileName+".d")
 	// A workaround to create the 'nothing' ninja target so `m nothing` works,
 	// since bp2build runs without Kati, and the 'nothing' target is declared in
 	// a Makefile.
 	ioutil.WriteFile(ninjaFile, []byte("build nothing: phony\n  phony_output = true\n"), 0666)
 	ioutil.WriteFile(ninjaFileD,
-		[]byte(fmt.Sprintf("%s: \\\n %s\n", ninjaFileName, extraNinjaDepsString)),
+		[]byte(fmt.Sprintf("%s: \\\n %s\n", ninjaFile, extraNinjaDepsString)),
 		0666)
 }
 
@@ -520,9 +519,14 @@ func runBp2Build(configuration android.Config, extraNinjaDeps []string) {
 		os.Exit(1)
 	}
 
-	if bp2buildMarker != "" {
+	// Create an empty bp2build marker file.
-		touch(shared.JoinPath(topDir, bp2buildMarker))
+	touch(shared.JoinPath(topDir, bp2buildMarker))
-	} else {
+
-		writeFakeNinjaFile(extraNinjaDeps, codegenContext.Config().BuildDir())
+	// bp2build *always* writes a fake Ninja file containing just the nothing
-	}
+	// phony target if it ever re-runs. This allows bp2build to exit early with
+	// GENERATE_BAZEL_FILES=1 m nothing.
+	//
+	// If bp2build is invoked as part of an integrated mixed build, the fake
+	// build.ninja file will be rewritten later into the real file anyway.
+	writeFakeNinjaFile(extraNinjaDeps, codegenContext.Config().BuildDir())
 }

tests/bootstrap_test.sh

@@ -493,6 +493,21 @@ function test_bp2build_smoke {
   [[ -e out/soong/workspace ]] || fail "Bazel workspace not created"
 }
 
+function test_bp2build_generates_fake_ninja_file {
+  setup
+  create_mock_bazel
+
+  run_bp2build
+
+  if [[ ! -f "./out/soong/build.ninja" ]]; then
+    fail "./out/soong/build.ninja was not generated"
+  fi
+
+  if ! grep "build nothing: phony" "./out/soong/build.ninja"; then
+    fail "missing phony nothing target in out/soong/build.ninja"
+  fi
+}
+
 function test_bp2build_add_android_bp {
   setup
 
@@ -678,6 +693,7 @@ test_glob_during_bootstrapping
 test_soong_build_rerun_iff_environment_changes
 test_dump_json_module_graph
 test_bp2build_smoke
+test_bp2build_generates_fake_ninja_file
 test_bp2build_null_build
 test_bp2build_add_android_bp
 test_bp2build_add_to_glob

ui/build/soong.go

@@ -155,9 +155,9 @@ func bootstrapBlueprint(ctx Context, config Config, integratedBp2Build bool) {
 			Outputs: []string{bp2BuildMarkerFile},
 			Args:    bp2buildArgs,
 		}
-		args.PrimaryBuilderInvocations = []bootstrap.PrimaryBuilderInvocation{
+		args.PrimaryBuilderInvocations = []bootstrap.PrimaryBuilderInvocation{bp2buildInvocation}
-			bp2buildInvocation,
+		if config.bazelBuildMode() == mixedBuild {
-			mainSoongBuildInvocation,
+			args.PrimaryBuilderInvocations = append(args.PrimaryBuilderInvocations, mainSoongBuildInvocation)
 		}
 	} else {
 		args.PrimaryBuilderInvocations = []bootstrap.PrimaryBuilderInvocation{mainSoongBuildInvocation}