From ef615e58412530253f7dbac02d4acdafb83203c8 Mon Sep 17 00:00:00 2001 From: Chris Parsons Date: Thu, 18 Aug 2022 22:04:11 -0400 Subject: [PATCH] Add --bazel-mode and --bazel-mode-dev This allows "bazel mixed builds prod mode", in additional to reworking the mechanism in which mixed builds dev mode is enabled. As a followup, CI scripts will be migrated to use the new flags, as USE_BAZEL_ANALYSIS=1 is deprecated. Test: Manually ran --bazel-mode with an allowlist verifying that the module alone was enabled Test: Manually verified --bazel-mode and --bazel-mode-dev cause a build failure Change-Id: If0d34360e60452f428b05828f4ec7596b7cb619a --- android/allowlists/allowlists.go | 5 ++++ android/bazel.go | 10 +++---- android/bazel_handler.go | 41 ++++++++++++++++++---------- android/config.go | 13 +-------- cmd/soong_build/main.go | 6 ++++ ui/build/build.go | 20 ++++++++++++++ ui/build/config.go | 32 ++++++++-------------- ui/build/config_test.go | 47 +++++++++++++++++++++++++++----- ui/build/soong.go | 13 +++++---- 9 files changed, 123 insertions(+), 64 deletions(-) diff --git a/android/allowlists/allowlists.go b/android/allowlists/allowlists.go index 5c79fa2a2..60b3503b2 100644 --- a/android/allowlists/allowlists.go +++ b/android/allowlists/allowlists.go @@ -595,4 +595,9 @@ var ( "prebuilt_platform-robolectric-4.5.1-prebuilt", "prebuilt_currysrc_org.eclipse", } + + ProdMixedBuildsEnabledList = []string{ + // This list left intentionally empty for now. Add specific module names + // to have them built by Bazel in Prod Mixed Builds mode. + } ) diff --git a/android/bazel.go b/android/bazel.go index 71eb036b2..3ae3d6f40 100644 --- a/android/bazel.go +++ b/android/bazel.go @@ -219,16 +219,16 @@ type bp2BuildConversionAllowlist struct { // in the synthetic Bazel workspace. keepExistingBuildFile map[string]bool - // Per-module allowlist to always opt modules in of both bp2build and mixed builds. - // These modules are usually in directories with many other modules that are not ready for - // conversion. + // Per-module allowlist to always opt modules into both bp2build and Bazel Dev Mode mixed + // builds. These modules are usually in directories with many other modules that are not ready + // for conversion. // // A module can either be in this list or its directory allowlisted entirely // in bp2buildDefaultConfig, but not both at the same time. moduleAlwaysConvert map[string]bool - // Per-module-type allowlist to always opt modules in to both bp2build and mixed builds - // when they have the same type as one listed. + // Per-module-type allowlist to always opt modules in to both bp2build and + // Bazel Dev Mode mixed builds when they have the same type as one listed. moduleTypeAlwaysConvert map[string]bool // Per-module denylist to always opt modules out of bp2build conversion. diff --git a/android/bazel_handler.go b/android/bazel_handler.go index d87f98869..93b677930 100644 --- a/android/bazel_handler.go +++ b/android/bazel_handler.go @@ -343,7 +343,30 @@ func (m noopBazelContext) AqueryDepsets() []bazel.AqueryDepset { } func NewBazelContext(c *config) (BazelContext, error) { - if !c.IsMixedBuildsEnabled() { + var modulesDefaultToBazel bool + disabledModules := map[string]bool{} + enabledModules := map[string]bool{} + + switch c.BuildMode { + case BazelProdMode: + modulesDefaultToBazel = false + + for _, enabledProdModule := range allowlists.ProdMixedBuildsEnabledList { + enabledModules[enabledProdModule] = true + } + case BazelDevMode: + modulesDefaultToBazel = true + + // Don't use partially-converted cc_library targets in mixed builds, + // since mixed builds would generally rely on both static and shared + // variants of a cc_library. + for staticOnlyModule, _ := range GetBp2BuildAllowList().ccLibraryStaticOnly { + disabledModules[staticOnlyModule] = true + } + for _, disabledDevModule := range allowlists.MixedBuildsDisabledList { + disabledModules[disabledDevModule] = true + } + default: return noopBazelContext{}, nil } @@ -352,24 +375,12 @@ func NewBazelContext(c *config) (BazelContext, error) { return nil, err } - // TODO(cparsons): Use a different allowlist depending on prod vs. dev - // bazel mode. - disabledModules := map[string]bool{} - // Don't use partially-converted cc_library targets in mixed builds, - // since mixed builds would generally rely on both static and shared - // variants of a cc_library. - for staticOnlyModule, _ := range GetBp2BuildAllowList().ccLibraryStaticOnly { - disabledModules[staticOnlyModule] = true - } - for _, disabledDevModule := range allowlists.MixedBuildsDisabledList { - disabledModules[disabledDevModule] = true - } - return &bazelContext{ bazelRunner: &builtinBazelRunner{}, paths: p, requests: make(map[cqueryKey]bool), - modulesDefaultToBazel: true, + modulesDefaultToBazel: modulesDefaultToBazel, + bazelEnabledModules: enabledModules, bazelDisabledModules: disabledModules, }, nil } diff --git a/android/config.go b/android/config.go index 5ca94209f..222b142e2 100644 --- a/android/config.go +++ b/android/config.go @@ -96,7 +96,6 @@ const ( // Use bazel during analysis of build modules from an allowlist carefully // curated by the build team to be proven stable. - // TODO(cparsons): Implement this mode. BazelProdMode ) @@ -481,14 +480,6 @@ func NewConfig(moduleListFile string, buildMode SoongBuildMode, runGoTests bool, config.AndroidFirstDeviceTarget = FirstTarget(config.Targets[Android], "lib64", "lib32")[0] } - // Checking USE_BAZEL_ANALYSIS must be done here instead of in the caller, so - // that we can invoke IsEnvTrue (which also registers the env var as a - // dependency of the build). - // TODO(cparsons): Remove this hack once USE_BAZEL_ANALYSIS is removed. - if buildMode == AnalysisNoBazel && config.IsEnvTrue("USE_BAZEL_ANALYSIS") { - buildMode = BazelDevMode - } - config.BuildMode = buildMode config.BazelContext, err = NewBazelContext(config) config.bp2buildPackageConfig = GetBp2BuildAllowList() @@ -678,9 +669,7 @@ func (c *config) DeviceName() string { // DeviceProduct returns the current product target. There could be multiple of // these per device type. // -// NOTE: Do not base conditional logic on this value. It may break product -// -// inheritance. +// NOTE: Do not base conditional logic on this value. It may break product inheritance. func (c *config) DeviceProduct() string { return *c.productVariables.DeviceProduct } diff --git a/cmd/soong_build/main.go b/cmd/soong_build/main.go index 693094357..d15dea1c0 100644 --- a/cmd/soong_build/main.go +++ b/cmd/soong_build/main.go @@ -84,6 +84,8 @@ func init() { flag.StringVar(&bp2buildMarker, "bp2build_marker", "", "If set, run bp2build, touch the specified marker file then exit") flag.StringVar(&cmdlineArgs.OutFile, "o", "build.ninja", "the Ninja file to output") flag.BoolVar(&cmdlineArgs.EmptyNinjaFile, "empty-ninja-file", false, "write out a 0-byte ninja file") + flag.BoolVar(&cmdlineArgs.BazelMode, "bazel-mode", false, "use bazel for analysis of certain modules") + flag.BoolVar(&cmdlineArgs.BazelModeDev, "bazel-mode-dev", false, "use bazel for analysis of a large number of modules (less stable)") // Flags that probably shouldn't be flags of soong_build but we haven't found // the time to remove them yet @@ -131,6 +133,10 @@ func newConfig(availableEnv map[string]string) android.Config { buildMode = android.GenerateModuleGraph } else if docFile != "" { buildMode = android.GenerateDocFile + } else if cmdlineArgs.BazelModeDev { + buildMode = android.BazelDevMode + } else if cmdlineArgs.BazelMode { + buildMode = android.BazelProdMode } else { buildMode = android.AnalysisNoBazel } diff --git a/ui/build/build.go b/ui/build/build.go index f7a2d7b18..c61baa135 100644 --- a/ui/build/build.go +++ b/ui/build/build.go @@ -110,6 +110,24 @@ const ( RunAllWithBazel = RunProductConfig | RunSoong | RunKati | RunKatiNinja | RunBazel ) +// checkBazelMode fails the build if there are conflicting arguments for which bazel +// build mode to use. +func checkBazelMode(ctx Context, config Config) { + // TODO(cparsons): Remove USE_BAZEL_ANALYSIS handling. + if config.Environment().IsEnvTrue("USE_BAZEL_ANALYSIS") { + if config.bazelProdMode || config.bazelDevMode { + ctx.Fatalf("USE_BAZEL_ANALYSIS is deprecated.\n" + + "Unset USE_BAZEL_ANALYSIS when using --bazel-mode or --bazel-mode-dev.") + } else { + config.bazelDevMode = true + } + } + if config.bazelProdMode && config.bazelDevMode { + ctx.Fatalf("Conflicting bazel mode.\n" + + "Do not specify both --bazel-mode and --bazel-mode-dev") + } +} + // checkProblematicFiles fails the build if existing Android.mk or CleanSpec.mk files are found at the root of the tree. func checkProblematicFiles(ctx Context) { files := []string{"Android.mk", "CleanSpec.mk"} @@ -221,6 +239,8 @@ func Build(ctx Context, config Config) { defer waitForDist(ctx) + checkBazelMode(ctx, config) + // checkProblematicFiles aborts the build if Android.mk or CleanSpec.mk are found at the root of the tree. checkProblematicFiles(ctx) diff --git a/ui/build/config.go b/ui/build/config.go index e14086712..590aeb9be 100644 --- a/ui/build/config.go +++ b/ui/build/config.go @@ -99,7 +99,10 @@ type configImpl struct { pathReplaced bool - useBazel bool + // TODO(b/243077098): Remove useBazel. + useBazel bool + bazelProdMode bool + bazelDevMode bool // During Bazel execution, Bazel cannot write outside OUT_DIR. // So if DIST_DIR is set to an external dir (outside of OUT_DIR), we need to rig it temporarily and then migrate files at the end of the build. @@ -130,18 +133,6 @@ const ( BUILD_MODULES ) -type bazelBuildMode int - -// Bazel-related build modes. -const ( - // Don't use bazel at all. - noBazel bazelBuildMode = iota - - // Generate synthetic build files and incorporate these files into a build which - // partially uses Bazel. Build metadata may come from Android.bp or BUILD files. - mixedBuild -) - // checkTopDir validates that the current directory is at the root directory of the source tree. func checkTopDir(ctx Context) { if _, err := os.Stat(srcDirFileCheck); err != nil { @@ -496,7 +487,7 @@ func buildConfig(config Config) *smpb.BuildConfig { UseGoma: proto.Bool(config.UseGoma()), UseRbe: proto.Bool(config.UseRBE()), BazelAsNinja: proto.Bool(config.UseBazel()), - BazelMixedBuild: proto.Bool(config.bazelBuildMode() == mixedBuild), + BazelMixedBuild: proto.Bool(config.BazelBuildEnabled()), } c.Targets = append(c.Targets, config.arguments...) @@ -747,6 +738,10 @@ func (c *configImpl) parseArgs(ctx Context, args []string) { c.skipSoongTests = true } else if arg == "--mk-metrics" { c.reportMkMetrics = true + } else if arg == "--bazel-mode" { + c.bazelProdMode = true + } else if arg == "--bazel-mode-dev" { + c.bazelDevMode = true } else if len(arg) > 0 && arg[0] == '-' { parseArgNum := func(def int) int { if len(arg) > 2 { @@ -1134,16 +1129,13 @@ func (c *configImpl) UseRBE() bool { return false } +// TODO(b/243077098): Remove UseBazel. func (c *configImpl) UseBazel() bool { return c.useBazel } -func (c *configImpl) bazelBuildMode() bazelBuildMode { - if c.Environment().IsEnvTrue("USE_BAZEL_ANALYSIS") { - return mixedBuild - } else { - return noBazel - } +func (c *configImpl) BazelBuildEnabled() bool { + return c.bazelProdMode || c.bazelDevMode } func (c *configImpl) StartRBE() bool { diff --git a/ui/build/config_test.go b/ui/build/config_test.go index e29327572..63716b0f7 100644 --- a/ui/build/config_test.go +++ b/ui/build/config_test.go @@ -28,6 +28,7 @@ import ( "android/soong/ui/logger" smpb "android/soong/ui/metrics/metrics_proto" "android/soong/ui/status" + "google.golang.org/protobuf/encoding/prototext" "google.golang.org/protobuf/proto" ) @@ -1005,6 +1006,8 @@ func TestBuildConfig(t *testing.T) { environ Environment arguments []string useBazel bool + bazelDevMode bool + bazelProdMode bool expectedBuildConfig *smpb.BuildConfig }{ { @@ -1064,7 +1067,7 @@ func TestBuildConfig(t *testing.T) { }, }, { - name: "bazel mixed build", + name: "bazel mixed build from env", environ: Environment{"USE_BAZEL_ANALYSIS=1"}, expectedBuildConfig: &smpb.BuildConfig{ ForceUseGoma: proto.Bool(false), @@ -1074,6 +1077,30 @@ func TestBuildConfig(t *testing.T) { BazelMixedBuild: proto.Bool(true), }, }, + { + name: "bazel mixed build from dev mode", + environ: Environment{}, + bazelDevMode: true, + expectedBuildConfig: &smpb.BuildConfig{ + ForceUseGoma: proto.Bool(false), + UseGoma: proto.Bool(false), + UseRbe: proto.Bool(false), + BazelAsNinja: proto.Bool(false), + BazelMixedBuild: proto.Bool(true), + }, + }, + { + name: "bazel mixed build from prod mode", + environ: Environment{}, + bazelProdMode: true, + expectedBuildConfig: &smpb.BuildConfig{ + ForceUseGoma: proto.Bool(false), + UseGoma: proto.Bool(false), + UseRbe: proto.Bool(false), + BazelAsNinja: proto.Bool(false), + BazelMixedBuild: proto.Bool(true), + }, + }, { name: "specified targets", environ: Environment{}, @@ -1094,9 +1121,9 @@ func TestBuildConfig(t *testing.T) { "FORCE_USE_GOMA=1", "USE_GOMA=1", "USE_RBE=1", - "USE_BAZEL_ANALYSIS=1", }, - useBazel: true, + useBazel: true, + bazelDevMode: true, expectedBuildConfig: &smpb.BuildConfig{ ForceUseGoma: proto.Bool(true), UseGoma: proto.Bool(true), @@ -1107,17 +1134,23 @@ func TestBuildConfig(t *testing.T) { }, } + ctx := testContext() for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { c := &configImpl{ - environ: &tc.environ, - useBazel: tc.useBazel, - arguments: tc.arguments, + environ: &tc.environ, + useBazel: tc.useBazel, + bazelDevMode: tc.bazelDevMode, + bazelProdMode: tc.bazelProdMode, + arguments: tc.arguments, } config := Config{c} + checkBazelMode(ctx, config) actualBuildConfig := buildConfig(config) if expected := tc.expectedBuildConfig; !proto.Equal(expected, actualBuildConfig) { - t.Errorf("Expected build config != actual build config: %#v != %#v", *expected, *actualBuildConfig) + t.Errorf("Build config mismatch.\n"+ + "Expected build config: %#v\n"+ + "Actual build config: %#v", prototext.Format(expected), prototext.Format(actualBuildConfig)) } }) } diff --git a/ui/build/soong.go b/ui/build/soong.go index 3920ddddd..2e94a46b1 100644 --- a/ui/build/soong.go +++ b/ui/build/soong.go @@ -254,6 +254,12 @@ func bootstrapBlueprint(ctx Context, config Config) { if config.EmptyNinjaFile() { mainSoongBuildExtraArgs = append(mainSoongBuildExtraArgs, "--empty-ninja-file") } + if config.bazelProdMode { + mainSoongBuildExtraArgs = append(mainSoongBuildExtraArgs, "--bazel-mode") + } + if config.bazelDevMode { + mainSoongBuildExtraArgs = append(mainSoongBuildExtraArgs, "--bazel-mode-dev") + } mainSoongBuildInvocation := primaryBuilderInvocation( config, @@ -263,7 +269,7 @@ func bootstrapBlueprint(ctx Context, config Config) { fmt.Sprintf("analyzing Android.bp files and generating ninja file at %s", config.SoongNinjaFile()), ) - if config.bazelBuildMode() == mixedBuild { + if config.BazelBuildEnabled() { // Mixed builds call Bazel from soong_build and they therefore need the // Bazel workspace to be available. Make that so by adding a dependency on // the bp2build marker file to the action that invokes soong_build . @@ -373,9 +379,6 @@ func runSoong(ctx Context, config Config) { // unused variables were changed? envFile := filepath.Join(config.SoongOutDir(), availableEnvFile) - buildMode := config.bazelBuildMode() - integratedBp2Build := buildMode == mixedBuild - // This is done unconditionally, but does not take a measurable amount of time bootstrapBlueprint(ctx, config) @@ -405,7 +408,7 @@ func runSoong(ctx Context, config Config) { checkEnvironmentFile(soongBuildEnv, config.UsedEnvFile(soongBuildTag)) - if integratedBp2Build || config.Bp2Build() { + if config.BazelBuildEnabled() || config.Bp2Build() { checkEnvironmentFile(soongBuildEnv, config.UsedEnvFile(bp2buildTag)) }