Provide reason for unconverted bp2build modules
This also changes the expectation of ConvertWithBp2build. Each
implementation must either create one or more Bazel target modules, or
mark the module as unconvertible (with a specific reason).
Manually verified no runtime hit in AOSP
In AOSP, the metrics file size increases from 252K to 1.6M
This changes some effective module counts in bp2build metrics:
- Removes "package" modules from the module count list in
metrics, as these will not be converted like regular modules.
- Counts Handcrafted modules as being "unconverted", as bp2build is not
responsible for them.
Bug: 285631638
Test: Verified generated BUILD.bazel files are bit-for-bit identical
with this change
Test: Manually verified one case of each implemented reasonType
Change-Id: I308dd451d8f28379b15671dae9f931bd0446f5c1
diff --git a/android/Android.bp b/android/Android.bp
index 94d2c04..2ccccf0 100644
--- a/android/Android.bp
+++ b/android/Android.bp
@@ -19,6 +19,7 @@
"soong-shared",
"soong-starlark",
"soong-starlark-format",
+ "soong-ui-bp2build_metrics_proto",
"soong-ui-metrics_proto",
"soong-android-allowlists",
diff --git a/android/bazel.go b/android/bazel.go
index e631ed4..1bfbdec 100644
--- a/android/bazel.go
+++ b/android/bazel.go
@@ -17,8 +17,10 @@
import (
"bufio"
"errors"
+ "fmt"
"strings"
+ "android/soong/ui/metrics/bp2build_metrics_proto"
"github.com/google/blueprint"
"github.com/google/blueprint/proptools"
@@ -73,6 +75,21 @@
// MissingBp2buildDep stores the module names of direct dependency that were not found
MissingDeps []string `blueprint:"mutated"`
+
+ // If non-nil, indicates that the module could not be converted successfully
+ // with bp2build. This will describe the reason the module could not be converted.
+ UnconvertedReason *UnconvertedReason
+}
+
+// The reason a module could not be converted to a BUILD target via bp2build.
+// This should match bp2build_metrics_proto.UnconvertedReason, but omits private
+// proto-related fields that prevent copying this struct.
+type UnconvertedReason struct {
+ // Should correspond to a valid value in bp2build_metrics_proto.UnconvertedReasonType.
+ // A raw int is used here instead, because blueprint logic requires that all transitive
+ // fields of module definitions be primitives.
+ ReasonType int
+ Detail string
}
type BazelModuleProperties struct {
@@ -137,6 +154,12 @@
GetBazelLabel(ctx BazelConversionPathContext, module blueprint.Module) string
ShouldConvertWithBp2build(ctx BazelConversionContext) bool
shouldConvertWithBp2build(ctx bazelOtherModuleContext, module blueprint.Module) bool
+
+ // ConvertWithBp2build either converts the module to a Bazel build target or
+ // declares the module as unconvertible (for logging and metrics).
+ // Modules must implement this function to be bp2build convertible. The function
+ // must either create at least one Bazel target module (using ctx.CreateBazelTargetModule or
+ // its related functions), or declare itself unconvertible using ctx.MarkBp2buildUnconvertible.
ConvertWithBp2build(ctx TopDownMutatorContext)
// namespacedVariableProps is a map from a soong config variable namespace
@@ -232,7 +255,7 @@
if b.ShouldConvertWithBp2build(ctx) {
return bp2buildModuleLabel(ctx, module)
}
- return "" // no label for unconverted module
+ panic(fmt.Errorf("requested non-existent label for module ", module.Name()))
}
type Bp2BuildConversionAllowlist struct {
@@ -533,22 +556,32 @@
}
func registerBp2buildConversionMutator(ctx RegisterMutatorsContext) {
- ctx.TopDown("bp2build_conversion", convertWithBp2build).Parallel()
+ ctx.TopDown("bp2build_conversion", bp2buildConversionMutator).Parallel()
}
-func convertWithBp2build(ctx TopDownMutatorContext) {
+func bp2buildConversionMutator(ctx TopDownMutatorContext) {
if ctx.Config().HasBazelBuildTargetInSource(ctx) {
// Defer to the BUILD target. Generating an additional target would
// cause a BUILD file conflict.
+ ctx.MarkBp2buildUnconvertible(bp2build_metrics_proto.UnconvertedReasonType_DEFINED_IN_BUILD_FILE, "")
return
}
bModule, ok := ctx.Module().(Bazelable)
- if !ok || !bModule.shouldConvertWithBp2build(ctx, ctx.Module()) {
+ if !ok {
+ ctx.MarkBp2buildUnconvertible(bp2build_metrics_proto.UnconvertedReasonType_TYPE_UNSUPPORTED, "")
return
}
-
+ // TODO: b/285631638 - Differentiate between denylisted modules and missing bp2build capabilities.
+ if !bModule.shouldConvertWithBp2build(ctx, ctx.Module()) {
+ ctx.MarkBp2buildUnconvertible(bp2build_metrics_proto.UnconvertedReasonType_UNSUPPORTED, "")
+ return
+ }
bModule.ConvertWithBp2build(ctx)
+
+ if !ctx.Module().base().IsConvertedByBp2build() && ctx.Module().base().GetUnconvertedReason() == nil {
+ panic(fmt.Errorf("illegal bp2build invariant: module '%s' was neither converted nor marked unconvertible", ctx.ModuleName()))
+ }
}
func registerApiBp2buildConversionMutator(ctx RegisterMutatorsContext) {
diff --git a/android/defaults.go b/android/defaults.go
index 31d6014..e0e6e5c 100644
--- a/android/defaults.go
+++ b/android/defaults.go
@@ -17,6 +17,7 @@
import (
"reflect"
+ "android/soong/ui/metrics/bp2build_metrics_proto"
"github.com/google/blueprint"
"github.com/google/blueprint/proptools"
)
@@ -179,7 +180,10 @@
// ConvertWithBp2build to fulfill Bazelable interface; however, at this time defaults module are
// *NOT* converted with bp2build
-func (defaultable *DefaultsModuleBase) ConvertWithBp2build(ctx TopDownMutatorContext) {}
+func (defaultable *DefaultsModuleBase) ConvertWithBp2build(ctx TopDownMutatorContext) {
+ // Defaults types are never convertible.
+ ctx.MarkBp2buildUnconvertible(bp2build_metrics_proto.UnconvertedReasonType_TYPE_UNSUPPORTED, "")
+}
func InitDefaultsModule(module DefaultsModule) {
commonProperties := &commonProperties{}
diff --git a/android/filegroup.go b/android/filegroup.go
index 3522f80..3b86655 100644
--- a/android/filegroup.go
+++ b/android/filegroup.go
@@ -21,6 +21,7 @@
"android/soong/bazel"
"android/soong/bazel/cquery"
+ "android/soong/ui/metrics/bp2build_metrics_proto"
"github.com/google/blueprint"
)
@@ -111,6 +112,7 @@
if len(srcs.Value.Includes) > 1 {
ctx.ModuleErrorf("filegroup '%s' cannot contain a file with the same name", fg.Name())
}
+ ctx.MarkBp2buildUnconvertible(bp2build_metrics_proto.UnconvertedReasonType_SRC_NAME_COLLISION, "")
return
}
}
diff --git a/android/module.go b/android/module.go
index ba32710..73783d8 100644
--- a/android/module.go
+++ b/android/module.go
@@ -30,6 +30,7 @@
"text/scanner"
"android/soong/bazel"
+ "android/soong/ui/metrics/bp2build_metrics_proto"
"github.com/google/blueprint"
"github.com/google/blueprint/proptools"
@@ -565,6 +566,8 @@
// IsConvertedByBp2build returns whether this module was converted via bp2build
IsConvertedByBp2build() bool
+ GetUnconvertedReason() *UnconvertedReason
+
// Bp2buildTargets returns the target(s) generated for Bazel via bp2build for this module
Bp2buildTargets() []bp2buildInfo
GetUnconvertedBp2buildDeps() []string
@@ -1591,14 +1594,31 @@
}
func (m *ModuleBase) addBp2buildInfo(info bp2buildInfo) {
+ if m.commonProperties.BazelConversionStatus.UnconvertedReason != nil {
+ panic(fmt.Errorf("bp2build: module '%s' marked unconvertible and also is converted", m.Name()))
+ }
m.commonProperties.BazelConversionStatus.Bp2buildInfo = append(m.commonProperties.BazelConversionStatus.Bp2buildInfo, info)
}
+func (m *ModuleBase) setBp2buildUnconvertible(reasonType bp2build_metrics_proto.UnconvertedReasonType, detail string) {
+ if len(m.commonProperties.BazelConversionStatus.Bp2buildInfo) > 0 {
+ panic(fmt.Errorf("bp2build: module '%s' marked unconvertible and also is converted", m.Name()))
+ }
+ m.commonProperties.BazelConversionStatus.UnconvertedReason = &UnconvertedReason{
+ ReasonType: int(reasonType),
+ Detail: detail,
+ }
+}
+
// IsConvertedByBp2build returns whether this module was converted via bp2build.
func (m *ModuleBase) IsConvertedByBp2build() bool {
return len(m.commonProperties.BazelConversionStatus.Bp2buildInfo) > 0
}
+func (m *ModuleBase) GetUnconvertedReason() *UnconvertedReason {
+ return m.commonProperties.BazelConversionStatus.UnconvertedReason
+}
+
// Bp2buildTargets returns the Bazel targets bp2build generated for this module.
func (m *ModuleBase) Bp2buildTargets() []bp2buildInfo {
return m.commonProperties.BazelConversionStatus.Bp2buildInfo
diff --git a/android/mutator.go b/android/mutator.go
index 4185315..2ec051e 100644
--- a/android/mutator.go
+++ b/android/mutator.go
@@ -16,6 +16,7 @@
import (
"android/soong/bazel"
+ "android/soong/ui/metrics/bp2build_metrics_proto"
"github.com/google/blueprint"
)
@@ -271,6 +272,10 @@
// any platform for which this bool attribute is false.
CreateBazelTargetModuleWithRestrictions(bazel.BazelTargetModuleProperties, CommonAttributes, interface{}, bazel.BoolAttribute)
+ // MarkBp2buildUnconvertible registers the current module as "unconvertible to bp2build" for the
+ // given reason.
+ MarkBp2buildUnconvertible(reasonType bp2build_metrics_proto.UnconvertedReasonType, detail string)
+
// CreateBazelTargetAliasInDir creates an alias definition in `dir` directory.
// This function can be used to create alias definitions in a directory that is different
// from the directory of the visited Soong module.
@@ -718,6 +723,12 @@
t.createBazelTargetModule(bazelProps, commonAttrs, attrs, enabledProperty)
}
+func (t *topDownMutatorContext) MarkBp2buildUnconvertible(
+ reasonType bp2build_metrics_proto.UnconvertedReasonType, detail string) {
+ mod := t.Module()
+ mod.base().setBp2buildUnconvertible(reasonType, detail)
+}
+
var (
bazelAliasModuleProperties = bazel.BazelTargetModuleProperties{
Rule_class: "alias",