diff --git a/android/bazel.go b/android/bazel.go index df30ff261..e4fada0fe 100644 --- a/android/bazel.go +++ b/android/bazel.go @@ -21,6 +21,7 @@ import ( "strings" "android/soong/ui/metrics/bp2build_metrics_proto" + "github.com/google/blueprint" "github.com/google/blueprint/bootstrap" "github.com/google/blueprint/proptools" @@ -153,8 +154,8 @@ type Bazelable interface { HasHandcraftedLabel() bool HandcraftedLabel() string GetBazelLabel(ctx BazelConversionPathContext, module blueprint.Module) string - ShouldConvertWithBp2build(ctx BazelConversionContext) bool - shouldConvertWithBp2build(ctx bazelOtherModuleContext, module blueprint.Module) bool + ShouldConvertWithBp2build(ctx ShouldConvertWithBazelContext) bool + shouldConvertWithBp2build(shouldConvertModuleContext, shouldConvertParams) bool // ConvertWithBp2build either converts the module to a Bazel build target or // declares the module as unconvertible (for logging and metrics). @@ -448,12 +449,32 @@ func convertedToBazel(ctx BazelConversionContext, module blueprint.Module) bool if !ok { return false } - return b.shouldConvertWithBp2build(ctx, module) || b.HasHandcraftedLabel() + + return b.HasHandcraftedLabel() || b.shouldConvertWithBp2build(ctx, shouldConvertParams{ + module: module, + moduleDir: ctx.OtherModuleDir(module), + moduleName: ctx.OtherModuleName(module), + moduleType: ctx.OtherModuleType(module), + }) +} + +type ShouldConvertWithBazelContext interface { + ModuleErrorf(format string, args ...interface{}) + Module() Module + Config() Config + ModuleType() string + ModuleName() string + ModuleDir() string } // ShouldConvertWithBp2build returns whether the given BazelModuleBase should be converted with bp2build -func (b *BazelModuleBase) ShouldConvertWithBp2build(ctx BazelConversionContext) bool { - return b.shouldConvertWithBp2build(ctx, ctx.Module()) +func (b *BazelModuleBase) ShouldConvertWithBp2build(ctx ShouldConvertWithBazelContext) bool { + return b.shouldConvertWithBp2build(ctx, shouldConvertParams{ + module: ctx.Module(), + moduleDir: ctx.ModuleDir(), + moduleName: ctx.ModuleName(), + moduleType: ctx.ModuleType(), + }) } type bazelOtherModuleContext interface { @@ -471,11 +492,24 @@ func isPlatformIncompatible(osType OsType, arch ArchType) bool { arch == Riscv64 // TODO(b/262192655) Riscv64 toolchains are not currently supported. } -func (b *BazelModuleBase) shouldConvertWithBp2build(ctx bazelOtherModuleContext, module blueprint.Module) bool { +type shouldConvertModuleContext interface { + ModuleErrorf(format string, args ...interface{}) + Config() Config +} + +type shouldConvertParams struct { + module blueprint.Module + moduleType string + moduleDir string + moduleName string +} + +func (b *BazelModuleBase) shouldConvertWithBp2build(ctx shouldConvertModuleContext, p shouldConvertParams) bool { if !b.bazelProps().Bazel_module.CanConvertToBazel { return false } + module := p.module // In api_bp2build mode, all soong modules that can provide API contributions should be converted // This is irrespective of its presence/absence in bp2build allowlists if ctx.Config().BuildMode == ApiBp2build { @@ -484,7 +518,7 @@ func (b *BazelModuleBase) shouldConvertWithBp2build(ctx bazelOtherModuleContext, } propValue := b.bazelProperties.Bazel_module.Bp2build_available - packagePath := moduleDirWithPossibleOverride(ctx, module) + packagePath := moduleDirWithPossibleOverride(ctx, module, p.moduleDir) // Modules in unit tests which are enabled in the allowlist by type or name // trigger this conditional because unit tests run under the "." package path @@ -493,10 +527,10 @@ func (b *BazelModuleBase) shouldConvertWithBp2build(ctx bazelOtherModuleContext, return true } - moduleName := moduleNameWithPossibleOverride(ctx, module) + moduleName := moduleNameWithPossibleOverride(ctx, module, p.moduleName) allowlist := ctx.Config().Bp2buildPackageConfig moduleNameAllowed := allowlist.moduleAlwaysConvert[moduleName] - moduleTypeAllowed := allowlist.moduleTypeAlwaysConvert[ctx.OtherModuleType(module)] + moduleTypeAllowed := allowlist.moduleTypeAlwaysConvert[p.moduleType] allowlistConvert := moduleNameAllowed || moduleTypeAllowed if moduleNameAllowed && moduleTypeAllowed { ctx.ModuleErrorf("A module cannot be in moduleAlwaysConvert and also be in moduleTypeAlwaysConvert") @@ -588,8 +622,21 @@ func bp2buildConversionMutator(ctx TopDownMutatorContext) { ctx.MarkBp2buildUnconvertible(bp2build_metrics_proto.UnconvertedReasonType_TYPE_UNSUPPORTED, "") return } + // There may be cases where the target is created by a macro rather than in a BUILD file, those + // should be captured as well. + if bModule.HasHandcraftedLabel() { + // 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 + } // TODO: b/285631638 - Differentiate between denylisted modules and missing bp2build capabilities. - if !bModule.shouldConvertWithBp2build(ctx, ctx.Module()) { + if !bModule.shouldConvertWithBp2build(ctx, shouldConvertParams{ + module: ctx.Module(), + moduleDir: ctx.ModuleDir(), + moduleName: ctx.ModuleName(), + moduleType: ctx.ModuleType(), + }) { ctx.MarkBp2buildUnconvertible(bp2build_metrics_proto.UnconvertedReasonType_UNSUPPORTED, "") return } diff --git a/android/bazel_paths.go b/android/bazel_paths.go index b08a4cad7..7992564a6 100644 --- a/android/bazel_paths.go +++ b/android/bazel_paths.go @@ -88,6 +88,8 @@ type BazelConversionPathContext interface { EarlyModulePathContext BazelConversionContext + ModuleName() string + ModuleType() string ModuleErrorf(fmt string, args ...interface{}) PropertyErrorf(property, fmt string, args ...interface{}) GetDirectDep(name string) (blueprint.Module, blueprint.DependencyTag) @@ -459,8 +461,8 @@ func samePackage(label1, label2 string) bool { } func bp2buildModuleLabel(ctx BazelConversionContext, module blueprint.Module) string { - moduleName := moduleNameWithPossibleOverride(ctx, module) - moduleDir := moduleDirWithPossibleOverride(ctx, module) + moduleName := moduleNameWithPossibleOverride(ctx, module, ctx.OtherModuleName(module)) + moduleDir := moduleDirWithPossibleOverride(ctx, module, ctx.OtherModuleDir(module)) if moduleDir == Bp2BuildTopLevel { moduleDir = "" } diff --git a/android/bazel_paths_test.go b/android/bazel_paths_test.go index 60c0a1478..75b77a36c 100644 --- a/android/bazel_paths_test.go +++ b/android/bazel_paths_test.go @@ -19,6 +19,7 @@ import ( "testing" "android/soong/bazel" + "github.com/google/blueprint" "github.com/google/blueprint/pathtools" ) @@ -157,6 +158,14 @@ func (ctx *TestBazelConversionPathContext) ModuleDir() string { return ctx.moduleDir } +func (ctx *TestBazelConversionPathContext) ModuleName() string { + panic("Unimplemented") +} + +func (ctx *TestBazelConversionPathContext) ModuleType() string { + panic("Unimplemented") +} + func TestTransformSubpackagePath(t *testing.T) { cfg := NullConfig("out", "out/soong") cfg.fs = pathtools.MockFs(map[string][]byte{ diff --git a/android/bazel_test.go b/android/bazel_test.go index 13fd40849..194a6b359 100644 --- a/android/bazel_test.go +++ b/android/bazel_test.go @@ -373,7 +373,14 @@ func TestBp2BuildAllowlist(t *testing.T) { allowlist: test.allowlist, } - shouldConvert := test.module.shouldConvertWithBp2build(bcc, test.module.TestModuleInfo) + shouldConvert := test.module.shouldConvertWithBp2build(bcc, + shouldConvertParams{ + module: test.module.TestModuleInfo, + moduleDir: test.module.TestModuleInfo.Dir, + moduleType: test.module.TestModuleInfo.Typ, + moduleName: test.module.TestModuleInfo.ModuleName, + }, + ) if test.shouldConvert != shouldConvert { t.Errorf("Module shouldConvert expected to be: %v, but was: %v", test.shouldConvert, shouldConvert) } diff --git a/android/override_module.go b/android/override_module.go index a4b7431f6..9e0de6fd0 100644 --- a/android/override_module.go +++ b/android/override_module.go @@ -353,26 +353,26 @@ func replaceDepsOnOverridingModuleMutator(ctx BottomUpMutatorContext) { // variant of this OverridableModule, or ctx.ModuleName() if this module is not an OverridableModule // or if this variant is not overridden. func ModuleNameWithPossibleOverride(ctx BazelConversionContext) string { - return moduleNameWithPossibleOverride(ctx, ctx.Module()) + return moduleNameWithPossibleOverride(ctx, ctx.Module(), ctx.OtherModuleName(ctx.Module())) } -func moduleNameWithPossibleOverride(ctx bazelOtherModuleContext, module blueprint.Module) string { +func moduleNameWithPossibleOverride(ctx shouldConvertModuleContext, module blueprint.Module, name string) string { if overridable, ok := module.(OverridableModule); ok { if o := overridable.GetOverriddenBy(); o != "" { return o } } - return ctx.OtherModuleName(module) + return name } // moduleDirWithPossibleOverride returns the dir of the OverrideModule that overrides the current // variant of the given OverridableModule, or ctx.OtherModuleName() if the module is not an // OverridableModule or if the variant is not overridden. -func moduleDirWithPossibleOverride(ctx bazelOtherModuleContext, module blueprint.Module) string { +func moduleDirWithPossibleOverride(ctx shouldConvertModuleContext, module blueprint.Module, dir string) string { if overridable, ok := module.(OverridableModule); ok { if o := overridable.GetOverriddenByModuleDir(); o != "" { return o } } - return ctx.OtherModuleDir(module) + return dir } diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index 6ca4bb441..cd1bc7f19 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -686,6 +686,9 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers // // bp2build converters are used for the majority of modules. if b, ok := m.(android.Bazelable); ok && b.HasHandcraftedLabel() { + if aModule, ok := m.(android.Module); ok && aModule.IsConvertedByBp2build() { + panic(fmt.Errorf("module %q [%s] [%s] was both converted with bp2build and has a handcrafted label", bpCtx.ModuleName(m), moduleType, dir)) + } // Handle modules converted to handcrafted targets. // // Since these modules are associated with some handcrafted