diff --git a/android/bazel.go b/android/bazel.go index f56c24e04..ef770bf9c 100644 --- a/android/bazel.go +++ b/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. diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index bddc524cd..7a73e18b8 100644 --- a/bp2build/build_conversion.go +++ b/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, } } diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go index 71660a8e1..b1c342c69 100644 --- a/bp2build/build_conversion_test.go +++ b/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, ) } } - } + }) } } diff --git a/bp2build/conversion.go b/bp2build/conversion.go index 101ad3d04..bced4c19f 100644 --- a/bp2build/conversion.go +++ b/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. diff --git a/cmd/soong_build/main.go b/cmd/soong_build/main.go index 70c88563f..7abb67f29 100644 --- a/cmd/soong_build/main.go +++ b/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 - } else { - return bootstrap.CmdlineArgs.OutFile - } + return bp2buildMarker } 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 != "" { - touch(shared.JoinPath(topDir, bp2buildMarker)) - } else { - writeFakeNinjaFile(extraNinjaDeps, codegenContext.Config().BuildDir()) - } + // Create an empty bp2build marker file. + touch(shared.JoinPath(topDir, bp2buildMarker)) + + // 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()) } diff --git a/tests/bootstrap_test.sh b/tests/bootstrap_test.sh index 42363e9c0..8c8dc8208 100755 --- a/tests/bootstrap_test.sh +++ b/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 diff --git a/ui/build/soong.go b/ui/build/soong.go index a41dbe1b5..cd645eb41 100644 --- a/ui/build/soong.go +++ b/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{ - bp2buildInvocation, - mainSoongBuildInvocation, + args.PrimaryBuilderInvocations = []bootstrap.PrimaryBuilderInvocation{bp2buildInvocation} + if config.bazelBuildMode() == mixedBuild { + args.PrimaryBuilderInvocations = append(args.PrimaryBuilderInvocations, mainSoongBuildInvocation) } } else { args.PrimaryBuilderInvocations = []bootstrap.PrimaryBuilderInvocation{mainSoongBuildInvocation}