Merge "Handle already existing targets of different name" into main

This commit is contained in:
Christopher Parsons 2023-09-22 18:52:07 +00:00 committed by Gerrit Code Review
commit 68a3d9b33e
9 changed files with 86 additions and 79 deletions

View file

@ -604,13 +604,6 @@ func registerBp2buildConversionMutator(ctx RegisterMutatorsContext) {
} }
func bp2buildConversionMutator(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) bModule, ok := ctx.Module().(Bazelable)
if !ok { if !ok {
ctx.MarkBp2buildUnconvertible(bp2build_metrics_proto.UnconvertedReasonType_TYPE_UNSUPPORTED, "") ctx.MarkBp2buildUnconvertible(bp2build_metrics_proto.UnconvertedReasonType_TYPE_UNSUPPORTED, "")
@ -634,11 +627,24 @@ func bp2buildConversionMutator(ctx TopDownMutatorContext) {
ctx.MarkBp2buildUnconvertible(bp2build_metrics_proto.UnconvertedReasonType_UNSUPPORTED, "") ctx.MarkBp2buildUnconvertible(bp2build_metrics_proto.UnconvertedReasonType_UNSUPPORTED, "")
return return
} }
if ctx.Module().base().GetUnconvertedReason() != nil {
return
}
bModule.ConvertWithBp2build(ctx) bModule.ConvertWithBp2build(ctx)
if !ctx.Module().base().IsConvertedByBp2build() && ctx.Module().base().GetUnconvertedReason() == nil { if len(ctx.Module().base().Bp2buildTargets()) == 0 && ctx.Module().base().GetUnconvertedReason() == nil {
panic(fmt.Errorf("illegal bp2build invariant: module '%s' was neither converted nor marked unconvertible", ctx.ModuleName())) panic(fmt.Errorf("illegal bp2build invariant: module '%s' was neither converted nor marked unconvertible", ctx.ModuleName()))
} }
for _, targetInfo := range ctx.Module().base().Bp2buildTargets() {
if ctx.Config().HasBazelBuildTargetInSource(targetInfo.TargetPackage(), targetInfo.TargetName()) {
// 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, targetInfo.TargetName())
return
}
}
} }
func registerApiBp2buildConversionMutator(ctx RegisterMutatorsContext) { func registerApiBp2buildConversionMutator(ctx RegisterMutatorsContext) {

View file

@ -2022,10 +2022,9 @@ func (c *config) LogMixedBuild(ctx BaseModuleContext, useBazel bool) {
} }
} }
func (c *config) HasBazelBuildTargetInSource(ctx BaseModuleContext) bool { func (c *config) HasBazelBuildTargetInSource(dir string, target string) bool {
moduleName := ctx.Module().Name() for _, existingTarget := range c.bazelTargetsByDir[dir] {
for _, buildTarget := range c.bazelTargetsByDir[ctx.ModuleDir()] { if target == existingTarget {
if moduleName == buildTarget {
return true return true
} }
} }

View file

@ -565,8 +565,8 @@ type Module interface {
AddProperties(props ...interface{}) AddProperties(props ...interface{})
GetProperties() []interface{} GetProperties() []interface{}
// IsConvertedByBp2build returns whether this module was converted via bp2build // If this module should not have bazel BUILD definitions generated by bp2build,
IsConvertedByBp2build() bool // GetUnconvertedReason returns a reason this is the case.
GetUnconvertedReason() *UnconvertedReason GetUnconvertedReason() *UnconvertedReason
// Bp2buildTargets returns the target(s) generated for Bazel via bp2build for this module // Bp2buildTargets returns the target(s) generated for Bazel via bp2build for this module
@ -1639,35 +1639,16 @@ func (b bp2buildInfo) BazelAttributes() []interface{} {
} }
func (m *ModuleBase) addBp2buildInfo(info bp2buildInfo) { func (m *ModuleBase) addBp2buildInfo(info bp2buildInfo) {
reason := m.commonProperties.BazelConversionStatus.UnconvertedReason
if reason != nil {
panic(fmt.Errorf("bp2build: internal error trying to convert module '%s' marked unconvertible. Reason type %d: %s",
m.Name(),
reason.ReasonType,
reason.Detail))
}
m.commonProperties.BazelConversionStatus.Bp2buildInfo = append(m.commonProperties.BazelConversionStatus.Bp2buildInfo, info) m.commonProperties.BazelConversionStatus.Bp2buildInfo = append(m.commonProperties.BazelConversionStatus.Bp2buildInfo, info)
} }
func (m *ModuleBase) setBp2buildUnconvertible(reasonType bp2build_metrics_proto.UnconvertedReasonType, detail string) { func (m *ModuleBase) setBp2buildUnconvertible(reasonType bp2build_metrics_proto.UnconvertedReasonType, detail string) {
if len(m.commonProperties.BazelConversionStatus.Bp2buildInfo) > 0 {
fmt.Println(m.commonProperties.BazelConversionStatus.Bp2buildInfo)
panic(fmt.Errorf("bp2build: internal error trying to mark converted module '%s' as unconvertible. Reason type %d: %s",
m.Name(),
reasonType,
detail))
}
m.commonProperties.BazelConversionStatus.UnconvertedReason = &UnconvertedReason{ m.commonProperties.BazelConversionStatus.UnconvertedReason = &UnconvertedReason{
ReasonType: int(reasonType), ReasonType: int(reasonType),
Detail: detail, 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 { func (m *ModuleBase) GetUnconvertedReason() *UnconvertedReason {
return m.commonProperties.BazelConversionStatus.UnconvertedReason return m.commonProperties.BazelConversionStatus.UnconvertedReason
} }

View file

@ -110,7 +110,7 @@ func TestConvertAndroidLibraryImport(t *testing.T) {
"import.aar": "", "import.aar": "",
"dep.aar": "", "dep.aar": "",
}, },
StubbedBuildDefinitions: []string{"static_lib_dep", "prebuilt_static_import_dep"}, StubbedBuildDefinitions: []string{"static_lib_dep", "static_import_dep", "static_import_dep-neverlink"},
// Bazel's aar_import can only export *_import targets, so we expect // Bazel's aar_import can only export *_import targets, so we expect
// only "static_import_dep" in exports, but both "static_lib_dep" and // only "static_import_dep" in exports, but both "static_lib_dep" and
// "static_import_dep" in deps // "static_import_dep" in deps
@ -125,6 +125,7 @@ android_library_import {
// TODO: b/301007952 - This dep is needed because android_library_import must have aars set. // TODO: b/301007952 - This dep is needed because android_library_import must have aars set.
android_library_import { android_library_import {
name: "static_import_dep", name: "static_import_dep",
aars: ["import.aar"],
} }
`, `,
ExpectedBazelTargets: []string{ ExpectedBazelTargets: []string{

View file

@ -112,7 +112,7 @@ filegroup {
} }
cc_binary { name: "cc_binary_1"} cc_binary { name: "cc_binary_1"}
sh_binary { name: "sh_binary_2"} sh_binary { name: "sh_binary_2", src: "foo.sh"}
apex { apex {
name: "com.android.apogee", name: "com.android.apogee",
@ -609,7 +609,7 @@ filegroup {
} }
cc_binary { name: "cc_binary_1" } cc_binary { name: "cc_binary_1" }
sh_binary { name: "sh_binary_2" } sh_binary { name: "sh_binary_2", src: "foo.sh"}
apex { apex {
name: "com.android.apogee", name: "com.android.apogee",
@ -736,7 +736,7 @@ filegroup {
} }
cc_binary { name: "cc_binary_1"} cc_binary { name: "cc_binary_1"}
sh_binary { name: "sh_binary_2"} sh_binary { name: "sh_binary_2", src: "foo.sh"}
apex_test { apex_test {
name: "com.android.apogee", name: "com.android.apogee",

View file

@ -713,27 +713,32 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers
switch ctx.Mode() { switch ctx.Mode() {
case Bp2Build: case Bp2Build:
// There are two main ways of converting a Soong module to Bazel: if aModule, ok := m.(android.Module); ok {
// 1) Manually handcrafting a Bazel target and associating the module with its label reason := aModule.GetUnconvertedReason()
// 2) Automatically generating with bp2build converters if reason != nil {
// // If this module was force-enabled, cause an error.
// bp2build converters are used for the majority of modules. if _, ok := ctx.Config().BazelModulesForceEnabledByFlag()[m.Name()]; ok && m.Name() != "" {
if b, ok := m.(android.Bazelable); ok && b.HasHandcraftedLabel() { err := fmt.Errorf("Force Enabled Module %s not converted", m.Name())
if aModule, ok := m.(android.Module); ok && aModule.IsConvertedByBp2build() { errs = append(errs, err)
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. // Log the module isn't to be converted by bp2build.
// // TODO: b/291598248 - Log handcrafted modules differently than other unconverted modules.
// Since these modules are associated with some handcrafted metrics.AddUnconvertedModule(m, moduleType, dir, *reason)
// target in a BUILD file, we don't autoconvert them. return
}
if len(aModule.Bp2buildTargets()) == 0 {
panic(fmt.Errorf("illegal bp2build invariant: module '%s' was neither converted nor marked unconvertible", aModule.Name()))
}
// Log the module.
metrics.AddUnconvertedModule(m, moduleType, dir,
android.UnconvertedReason{
ReasonType: int(bp2build_metrics_proto.UnconvertedReasonType_DEFINED_IN_BUILD_FILE),
})
} else if aModule, ok := m.(android.Module); ok && aModule.IsConvertedByBp2build() {
// Handle modules converted to generated targets. // Handle modules converted to generated targets.
targets, targetErrs = generateBazelTargets(bpCtx, aModule)
errs = append(errs, targetErrs...)
for _, t := range targets {
// A module can potentially generate more than 1 Bazel
// target, each of a different rule class.
metrics.IncrementRuleClassCount(t.ruleClass)
}
// Log the module. // Log the module.
metrics.AddConvertedModule(aModule, moduleType, dir) metrics.AddConvertedModule(aModule, moduleType, dir)
@ -761,24 +766,6 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers
return return
} }
} }
targets, targetErrs = generateBazelTargets(bpCtx, aModule)
errs = append(errs, targetErrs...)
for _, t := range targets {
// A module can potentially generate more than 1 Bazel
// target, each of a different rule class.
metrics.IncrementRuleClassCount(t.ruleClass)
}
} else if _, ok := ctx.Config().BazelModulesForceEnabledByFlag()[m.Name()]; ok && m.Name() != "" {
err := fmt.Errorf("Force Enabled Module %s not converted", m.Name())
errs = append(errs, err)
} else if aModule, ok := m.(android.Module); ok {
reason := aModule.GetUnconvertedReason()
if reason == nil {
panic(fmt.Errorf("module '%s' was neither converted nor marked unconvertible with bp2build", aModule.Name()))
} else {
metrics.AddUnconvertedModule(m, moduleType, dir, *reason)
}
return
} else if glib, ok := m.(*bootstrap.GoPackage); ok { } else if glib, ok := m.(*bootstrap.GoPackage); ok {
targets, targetErrs = generateBazelTargetsGoPackage(bpCtx, glib, nameToGoLibMap) targets, targetErrs = generateBazelTargetsGoPackage(bpCtx, glib, nameToGoLibMap)
errs = append(errs, targetErrs...) errs = append(errs, targetErrs...)

View file

@ -1994,6 +1994,41 @@ func TestAlreadyPresentBuildTarget(t *testing.T) {
}) })
} }
func TestAlreadyPresentOneToManyBuildTarget(t *testing.T) {
bp := `
custom {
name: "foo",
one_to_many_prop: true,
}
custom {
name: "bar",
}
`
alreadyPresentBuildFile :=
MakeBazelTarget(
"custom",
// one_to_many_prop ensures that foo generates "foo_proto_library_deps".
"foo_proto_library_deps",
AttrNameToString{},
)
expectedBazelTargets := []string{
MakeBazelTarget(
"custom",
"bar",
AttrNameToString{},
),
}
registerCustomModule := func(ctx android.RegistrationContext) {
ctx.RegisterModuleType("custom", customModuleFactoryHostAndDevice)
}
RunBp2BuildTestCase(t, registerCustomModule, Bp2buildTestCase{
AlreadyExistingBuildContents: alreadyPresentBuildFile,
Blueprint: bp,
ExpectedBazelTargets: expectedBazelTargets,
Description: "Not duplicating work for an already-present BUILD target (different generated name)",
})
}
// Verifies that if a module is defined in pkg1/Android.bp, that a target present // Verifies that if a module is defined in pkg1/Android.bp, that a target present
// in pkg2/BUILD.bazel does not result in the module being labeled "already defined // in pkg2/BUILD.bazel does not result in the module being labeled "already defined
// in a BUILD file". // in a BUILD file".

View file

@ -1595,7 +1595,7 @@ func TestCcLibrarySdkVariantUsesStubs(t *testing.T) {
Description: "cc_library_shared stubs", Description: "cc_library_shared stubs",
ModuleTypeUnderTest: "cc_library_shared", ModuleTypeUnderTest: "cc_library_shared",
ModuleTypeUnderTestFactory: cc.LibrarySharedFactory, ModuleTypeUnderTestFactory: cc.LibrarySharedFactory,
StubbedBuildDefinitions: []string{"libNoStubs", "libHasApexStubs", "libHasApexAndNdkStubs"}, StubbedBuildDefinitions: []string{"libNoStubs", "libHasApexStubs", "libHasApexAndNdkStubs", "libHasApexAndNdkStubs.ndk_stub_libs"},
Blueprint: soongCcLibrarySharedPreamble + ` Blueprint: soongCcLibrarySharedPreamble + `
cc_library_shared { cc_library_shared {
name: "libUsesSdk", name: "libUsesSdk",
@ -1621,9 +1621,7 @@ cc_library_shared {
} }
ndk_library { ndk_library {
name: "libHasApexAndNdkStubs", name: "libHasApexAndNdkStubs",
// TODO: b/301321658 - Stub this once existing-build-file handling can deal with first_version: "28",
// modules that generate targets of a different name.
bazel_module: { bp2build_available: false },
} }
`, `,
ExpectedBazelTargets: []string{ ExpectedBazelTargets: []string{

View file

@ -114,7 +114,7 @@ func TestJavaBinaryHostLibs(t *testing.T) {
runJavaBinaryHostTestCase(t, Bp2buildTestCase{ runJavaBinaryHostTestCase(t, Bp2buildTestCase{
Description: "java_binary_host with srcs, libs.", Description: "java_binary_host with srcs, libs.",
Filesystem: testFs, Filesystem: testFs,
StubbedBuildDefinitions: []string{"prebuilt_java-lib-dep-1"}, StubbedBuildDefinitions: []string{"java-lib-dep-1", "java-lib-dep-1-neverlink"},
Blueprint: `java_binary_host { Blueprint: `java_binary_host {
name: "java-binary-host-libs", name: "java-binary-host-libs",
libs: ["java-lib-dep-1"], libs: ["java-lib-dep-1"],