Kaynağa Gözat

Refactor MixedBuildsEnabled and add --ensure-allowlist-integrity.

Currently, there is little verification around allowlisted modules
actually being mixed-built. This flag would allow us to verify
that a module allowlisted is mixed-built for at least one variant.

Bug: 278910100
Test: m nothing --bazel-mode-staging --ensure-allowlist-integrity
Test: m nothing --bazel-mode-staging --ensure-allowlist-integrity
--bazel-force-enabled-modules=com.google.android.neuralnetworks (This
fails, as expected)
Test: build/soong/test/mixed_mode_test.sh

Change-Id: Icd5976f4f44f1a8caca1e5247d986642f7995f97
MarkDacek 1 yıl önce
ebeveyn
işleme
f47e142ffd

+ 40 - 10
android/bazel.go

@@ -32,6 +32,22 @@ const (
 	Bp2BuildTopLevel = "."
 )
 
+type MixedBuildEnabledStatus int
+
+const (
+	// This module can be mixed_built.
+	MixedBuildEnabled = iota
+
+	// There is a technical incompatibility preventing this module from being
+	// bazel-analyzed. Note: the module might also be incompatible.
+	TechnicalIncompatibility
+
+	// This module cannot be mixed_built due to some incompatibility with it
+	// that is not a platform incompatibility. Example: the module-type is not
+	// enabled, or is not bp2build-converted.
+	ModuleIncompatibility
+)
+
 // FileGroupAsLibrary describes a filegroup module that is converted to some library
 // such as aidl_library or proto_library.
 type FileGroupAsLibrary interface {
@@ -346,24 +362,31 @@ func GetBp2BuildAllowList() Bp2BuildConversionAllowlist {
 	}).(Bp2BuildConversionAllowlist)
 }
 
-// MixedBuildsEnabled returns true if a module is ready to be replaced by a
-// converted or handcrafted Bazel target. As a side effect, calling this
-// method will also log whether this module is mixed build enabled for
-// metrics reporting.
-func MixedBuildsEnabled(ctx BaseModuleContext) bool {
+// MixedBuildsEnabled returns a MixedBuildEnabledStatus regarding whether
+// a module is ready to be replaced by a converted or handcrafted Bazel target.
+// As a side effect, calling this method will also log whether this module is
+// mixed build enabled for metrics reporting.
+func MixedBuildsEnabled(ctx BaseModuleContext) MixedBuildEnabledStatus {
 	module := ctx.Module()
 	apexInfo := ctx.Provider(ApexInfoProvider).(ApexInfo)
 	withinApex := !apexInfo.IsForPlatform()
+
+	platformIncompatible := isPlatformIncompatible(ctx.Os(), ctx.Arch().ArchType)
+	if platformIncompatible {
+		ctx.Config().LogMixedBuild(ctx, false)
+		return TechnicalIncompatibility
+	}
+
 	mixedBuildEnabled := ctx.Config().IsMixedBuildsEnabled() &&
-		ctx.Os() != Windows && // Windows toolchains are not currently supported.
-		ctx.Os() != LinuxBionic && // Linux Bionic toolchains are not currently supported.
-		ctx.Os() != LinuxMusl && // Linux musl toolchains are not currently supported (b/259266326).
-		ctx.Arch().ArchType != Riscv64 && // TODO(b/262192655) Riscv64 toolchains are not currently supported.
 		module.Enabled() &&
 		convertedToBazel(ctx, module) &&
 		ctx.Config().BazelContext.IsModuleNameAllowed(module.Name(), withinApex)
 	ctx.Config().LogMixedBuild(ctx, mixedBuildEnabled)
-	return mixedBuildEnabled
+
+	if mixedBuildEnabled {
+		return MixedBuildEnabled
+	}
+	return ModuleIncompatibility
 }
 
 // ConvertedToBazel returns whether this module has been converted (with bp2build or manually) to Bazel.
@@ -388,6 +411,13 @@ type bazelOtherModuleContext interface {
 	OtherModuleDir(m blueprint.Module) string
 }
 
+func isPlatformIncompatible(osType OsType, arch ArchType) bool {
+	return osType == Windows || // Windows toolchains are not currently supported.
+		osType == LinuxBionic || // Linux Bionic toolchains are not currently supported.
+		osType == LinuxMusl || // Linux musl toolchains are not currently supported (b/259266326).
+		arch == Riscv64 // TODO(b/262192655) Riscv64 toolchains are not currently supported.
+}
+
 func (b *BazelModuleBase) shouldConvertWithBp2build(ctx bazelOtherModuleContext, module blueprint.Module) bool {
 	if !b.bazelProps().Bazel_module.CanConvertToBazel {
 		return false

+ 2 - 4
android/bazel_handler.go

@@ -86,12 +86,10 @@ func RegisterMixedBuildsMutator(ctx RegistrationContext) {
 func mixedBuildsPrepareMutator(ctx BottomUpMutatorContext) {
 	if m := ctx.Module(); m.Enabled() {
 		if mixedBuildMod, ok := m.(MixedBuildBuildable); ok {
-			queueMixedBuild := mixedBuildMod.IsMixedBuildSupported(ctx) && MixedBuildsEnabled(ctx)
+			mixedBuildEnabled := MixedBuildsEnabled(ctx)
+			queueMixedBuild := mixedBuildMod.IsMixedBuildSupported(ctx) && mixedBuildEnabled == MixedBuildEnabled
 			if queueMixedBuild {
 				mixedBuildMod.QueueBazelCall(ctx)
-			} else if _, ok := ctx.Config().bazelForceEnabledModules[m.Name()]; ok {
-				// TODO(b/273910287) - remove this once --ensure_allowlist_integrity is added
-				ctx.ModuleErrorf("Attempted to force enable an unready module: %s. Did you forget to Bp2BuildDefaultTrue its directory?\n", m.Name())
 			}
 		}
 	}

+ 11 - 0
android/config.go

@@ -102,6 +102,8 @@ type CmdArgs struct {
 	UseBazelProxy bool
 
 	BuildFromTextStub bool
+
+	EnsureAllowlistIntegrity bool
 }
 
 // Build modes that soong_build can run as.
@@ -278,6 +280,11 @@ type config struct {
 	// If buildFromTextStub is true then the Java API stubs are
 	// built from the signature text files, not the source Java files.
 	buildFromTextStub bool
+
+	// If ensureAllowlistIntegrity is true, then the presence of any allowlisted
+	// modules that aren't mixed-built for at least one variant will cause a build
+	// failure
+	ensureAllowlistIntegrity bool
 }
 
 type deviceConfig struct {
@@ -1904,6 +1911,10 @@ func (c *config) UseHostMusl() bool {
 	return Bool(c.productVariables.HostMusl)
 }
 
+func (c *config) GetMixedBuildsEnabledModules() map[string]struct{} {
+	return c.mixedBuildEnabledModules
+}
+
 func (c *config) LogMixedBuild(ctx BaseModuleContext, useBazel bool) {
 	moduleName := ctx.Module().Name()
 	c.mixedBuildsLock.Lock()

+ 1 - 1
android/module.go

@@ -2438,7 +2438,7 @@ func (m *ModuleBase) GenerateBuildActions(blueprintCtx blueprint.ModuleContext)
 
 func (m *ModuleBase) isHandledByBazel(ctx ModuleContext) (MixedBuildBuildable, bool) {
 	if mixedBuildMod, ok := m.module.(MixedBuildBuildable); ok {
-		if mixedBuildMod.IsMixedBuildSupported(ctx) && MixedBuildsEnabled(ctx) {
+		if mixedBuildMod.IsMixedBuildSupported(ctx) && (MixedBuildsEnabled(ctx) == MixedBuildEnabled) {
 			return mixedBuildMod, true
 		}
 	}

+ 1 - 1
cc/cc_test.go

@@ -59,7 +59,7 @@ func registerTestMutators(ctx android.RegistrationContext) {
 func mixedBuildsPrepareMutator(ctx android.BottomUpMutatorContext) {
 	if m := ctx.Module(); m.Enabled() {
 		if mixedBuildMod, ok := m.(android.MixedBuildBuildable); ok {
-			if mixedBuildMod.IsMixedBuildSupported(ctx) && android.MixedBuildsEnabled(ctx) {
+			if mixedBuildMod.IsMixedBuildSupported(ctx) && android.MixedBuildsEnabled(ctx) == android.MixedBuildEnabled {
 				mixedBuildMod.QueueBazelCall(ctx)
 			}
 		}

+ 48 - 2
cmd/soong_build/main.go

@@ -85,7 +85,7 @@ func init() {
 	flag.BoolVar(&cmdlineArgs.BazelModeDev, "bazel-mode-dev", false, "use bazel for analysis of a large number of modules (less stable)")
 	flag.BoolVar(&cmdlineArgs.UseBazelProxy, "use-bazel-proxy", false, "communicate with bazel using unix socket proxy instead of spawning subprocesses")
 	flag.BoolVar(&cmdlineArgs.BuildFromTextStub, "build-from-text-stub", false, "build Java stubs from API text files instead of source files")
-
+	flag.BoolVar(&cmdlineArgs.EnsureAllowlistIntegrity, "ensure-allowlist-integrity", false, "verify that allowlisted modules are mixed-built")
 	// Flags that probably shouldn't be flags of soong_build, but we haven't found
 	// the time to remove them yet
 	flag.BoolVar(&cmdlineArgs.RunGoTests, "t", false, "build and run go tests during bootstrap")
@@ -288,6 +288,46 @@ func writeMetrics(configuration android.Config, eventHandler *metrics.EventHandl
 	maybeQuit(err, "error writing soong_build metrics %s", metricsFile)
 }
 
+// Errors out if any modules expected to be mixed_built were not, unless
+// there is a platform incompatibility.
+func checkForAllowlistIntegrityError(configuration android.Config, isStagingMode bool) error {
+	modules := findModulesNotMixedBuiltForAnyVariant(configuration, isStagingMode)
+	if len(modules) == 0 {
+		return nil
+	}
+
+	return fmt.Errorf("Error: expected the following modules to be mixed_built: %s", modules)
+}
+
+// Returns the list of modules that should have been mixed_built (per the
+// allowlists and cmdline flags) but were not.
+func findModulesNotMixedBuiltForAnyVariant(configuration android.Config, isStagingMode bool) []string {
+	retval := []string{}
+	forceEnabledModules := configuration.BazelModulesForceEnabledByFlag()
+
+	mixedBuildsEnabled := configuration.GetMixedBuildsEnabledModules()
+	for _, module := range allowlists.ProdMixedBuildsEnabledList {
+		if _, ok := mixedBuildsEnabled[module]; !ok && module != "" {
+			retval = append(retval, module)
+		}
+	}
+
+	if isStagingMode {
+		for _, module := range allowlists.StagingMixedBuildsEnabledList {
+			if _, ok := mixedBuildsEnabled[module]; !ok && module != "" {
+				retval = append(retval, module)
+			}
+		}
+	}
+
+	for module, _ := range forceEnabledModules {
+		if _, ok := mixedBuildsEnabled[module]; !ok && module != "" {
+			retval = append(retval, module)
+		}
+	}
+	return retval
+}
+
 func writeJsonModuleGraphAndActions(ctx *android.Context, cmdArgs android.CmdArgs) {
 	graphFile, graphErr := os.Create(shared.JoinPath(topDir, cmdArgs.ModuleGraphFile))
 	maybeQuit(graphErr, "graph err")
@@ -433,8 +473,14 @@ func main() {
 		writeMetrics(configuration, ctx.EventHandler, metricsDir)
 	default:
 		ctx.Register()
-		if configuration.IsMixedBuildsEnabled() {
+		isMixedBuildsEnabled := configuration.IsMixedBuildsEnabled()
+		if isMixedBuildsEnabled {
 			finalOutputFile = runMixedModeBuild(ctx, extraNinjaDeps)
+			if cmdlineArgs.EnsureAllowlistIntegrity {
+				if err := checkForAllowlistIntegrityError(configuration, cmdlineArgs.BazelModeStaging); err != nil {
+					maybeQuit(err, "")
+				}
+			}
 		} else {
 			finalOutputFile = runSoongOnlyBuild(ctx, extraNinjaDeps)
 		}

+ 1 - 1
tests/mixed_mode_test.sh

@@ -88,7 +88,7 @@ EOF
     fail "Bazel actions not found for force-enabled module"
   fi
 
-  unused=`run_soong --bazel-force-enabled-modules=unenabled-touch-file nothing >/dev/null`
+  unused=`run_soong --bazel-force-enabled-modules=unenabled-touch-file --ensure-allowlist-integrity nothing >/dev/null`
 
   if [[ $? -ne 1 ]]; then
     fail "Expected failure due to force-enabling an unenabled module "

+ 29 - 22
ui/build/config.go

@@ -67,28 +67,29 @@ type configImpl struct {
 	logsPrefix    string
 
 	// From the arguments
-	parallel          int
-	keepGoing         int
-	verbose           bool
-	checkbuild        bool
-	dist              bool
-	jsonModuleGraph   bool
-	apiBp2build       bool // Generate BUILD files for Soong modules that contribute APIs
-	bp2build          bool
-	queryview         bool
-	reportMkMetrics   bool // Collect and report mk2bp migration progress metrics.
-	soongDocs         bool
-	multitreeBuild    bool // This is a multitree build.
-	skipConfig        bool
-	skipKati          bool
-	skipKatiNinja     bool
-	skipSoong         bool
-	skipNinja         bool
-	skipSoongTests    bool
-	searchApiDir      bool // Scan the Android.bp files generated in out/api_surfaces
-	skipMetricsUpload bool
-	buildStartedTime  int64 // For metrics-upload-only - manually specify a build-started time
-	buildFromTextStub bool
+	parallel                 int
+	keepGoing                int
+	verbose                  bool
+	checkbuild               bool
+	dist                     bool
+	jsonModuleGraph          bool
+	apiBp2build              bool // Generate BUILD files for Soong modules that contribute APIs
+	bp2build                 bool
+	queryview                bool
+	reportMkMetrics          bool // Collect and report mk2bp migration progress metrics.
+	soongDocs                bool
+	multitreeBuild           bool // This is a multitree build.
+	skipConfig               bool
+	skipKati                 bool
+	skipKatiNinja            bool
+	skipSoong                bool
+	skipNinja                bool
+	skipSoongTests           bool
+	searchApiDir             bool // Scan the Android.bp files generated in out/api_surfaces
+	skipMetricsUpload        bool
+	buildStartedTime         int64 // For metrics-upload-only - manually specify a build-started time
+	buildFromTextStub        bool
+	ensureAllowlistIntegrity bool // For CI builds - make sure modules are mixed-built
 
 	// From the product config
 	katiArgs        []string
@@ -883,6 +884,8 @@ func (c *configImpl) parseArgs(ctx Context, args []string) {
 			} else {
 				ctx.Fatalf("Error parsing build-time-started-unix-millis", err)
 			}
+		} else if arg == "--ensure-allowlist-integrity" {
+			c.ensureAllowlistIntegrity = true
 		} else if len(arg) > 0 && arg[0] == '-' {
 			parseArgNum := func(def int) int {
 				if len(arg) > 2 {
@@ -1710,6 +1713,10 @@ func (c *configImpl) SkipMetricsUpload() bool {
 	return c.skipMetricsUpload
 }
 
+func (c *configImpl) EnsureAllowlistIntegrity() bool {
+	return c.ensureAllowlistIntegrity
+}
+
 // Returns a Time object if one was passed via a command-line flag.
 // Otherwise returns the passed default.
 func (c *configImpl) BuildStartedTimeOrDefault(defaultTime time.Time) time.Time {

+ 3 - 0
ui/build/soong.go

@@ -288,6 +288,9 @@ func bootstrapBlueprint(ctx Context, config Config) {
 	if config.buildFromTextStub {
 		mainSoongBuildExtraArgs = append(mainSoongBuildExtraArgs, "--build-from-text-stub")
 	}
+	if config.ensureAllowlistIntegrity {
+		mainSoongBuildExtraArgs = append(mainSoongBuildExtraArgs, "--ensure-allowlist-integrity")
+	}
 
 	queryviewDir := filepath.Join(config.SoongOutDir(), "queryview")
 	// The BUILD files will be generated in out/soong/.api_bp2build (no symlinks to src files)