Handle already existing targets of different name
In other words, if, in bp2build, module "foo" would generate "foo",
and "foo_two", and "foo_two" already exists in a build file,
bp2build should label "foo" as being unconvertible.
Fixes: 301321658
Fixes: 301312582
Bug: 285631638
Test: Unit tests
Test: Verified that `m bp2build` results in bit-for-bit identical
contents for out/soong/bp2build before and after this change.
Change-Id: Icbbdd69fce83579ec9b172d04b2bf1f294698f70
diff --git a/bp2build/aar_conversion_test.go b/bp2build/aar_conversion_test.go
index 59aacbb..40356a1 100644
--- a/bp2build/aar_conversion_test.go
+++ b/bp2build/aar_conversion_test.go
@@ -110,7 +110,7 @@
"import.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
// only "static_import_dep" in exports, but both "static_lib_dep" and
// "static_import_dep" in deps
@@ -125,6 +125,7 @@
// TODO: b/301007952 - This dep is needed because android_library_import must have aars set.
android_library_import {
name: "static_import_dep",
+ aars: ["import.aar"],
}
`,
ExpectedBazelTargets: []string{
diff --git a/bp2build/apex_conversion_test.go b/bp2build/apex_conversion_test.go
index 60de28c..5871d59 100644
--- a/bp2build/apex_conversion_test.go
+++ b/bp2build/apex_conversion_test.go
@@ -112,7 +112,7 @@
}
cc_binary { name: "cc_binary_1"}
-sh_binary { name: "sh_binary_2"}
+sh_binary { name: "sh_binary_2", src: "foo.sh"}
apex {
name: "com.android.apogee",
@@ -609,7 +609,7 @@
}
cc_binary { name: "cc_binary_1" }
-sh_binary { name: "sh_binary_2" }
+sh_binary { name: "sh_binary_2", src: "foo.sh"}
apex {
name: "com.android.apogee",
@@ -736,7 +736,7 @@
}
cc_binary { name: "cc_binary_1"}
-sh_binary { name: "sh_binary_2"}
+sh_binary { name: "sh_binary_2", src: "foo.sh"}
apex_test {
name: "com.android.apogee",
diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go
index 15b7766..e6941df 100644
--- a/bp2build/build_conversion.go
+++ b/bp2build/build_conversion.go
@@ -713,27 +713,32 @@
switch ctx.Mode() {
case Bp2Build:
- // There are two main ways of converting a Soong module to Bazel:
- // 1) Manually handcrafting a Bazel target and associating the module with its label
- // 2) Automatically generating with bp2build converters
- //
- // 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
- // target in a BUILD file, we don't autoconvert them.
+ if aModule, ok := m.(android.Module); ok {
+ reason := aModule.GetUnconvertedReason()
+ if reason != nil {
+ // If this module was force-enabled, cause an error.
+ 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)
+ }
- // 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() {
+ // Log the module isn't to be converted by bp2build.
+ // TODO: b/291598248 - Log handcrafted modules differently than other unconverted modules.
+ metrics.AddUnconvertedModule(m, moduleType, dir, *reason)
+ return
+ }
+ if len(aModule.Bp2buildTargets()) == 0 {
+ panic(fmt.Errorf("illegal bp2build invariant: module '%s' was neither converted nor marked unconvertible", aModule.Name()))
+ }
+
// 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.
metrics.AddConvertedModule(aModule, moduleType, dir)
@@ -761,24 +766,6 @@
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 {
targets, targetErrs = generateBazelTargetsGoPackage(bpCtx, glib, nameToGoLibMap)
errs = append(errs, targetErrs...)
diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go
index 329c907..afbfffa 100644
--- a/bp2build/build_conversion_test.go
+++ b/bp2build/build_conversion_test.go
@@ -1994,6 +1994,41 @@
})
}
+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
// in pkg2/BUILD.bazel does not result in the module being labeled "already defined
// in a BUILD file".
diff --git a/bp2build/cc_library_shared_conversion_test.go b/bp2build/cc_library_shared_conversion_test.go
index bdde996..6fa14e3 100644
--- a/bp2build/cc_library_shared_conversion_test.go
+++ b/bp2build/cc_library_shared_conversion_test.go
@@ -1595,7 +1595,7 @@
Description: "cc_library_shared stubs",
ModuleTypeUnderTest: "cc_library_shared",
ModuleTypeUnderTestFactory: cc.LibrarySharedFactory,
- StubbedBuildDefinitions: []string{"libNoStubs", "libHasApexStubs", "libHasApexAndNdkStubs"},
+ StubbedBuildDefinitions: []string{"libNoStubs", "libHasApexStubs", "libHasApexAndNdkStubs", "libHasApexAndNdkStubs.ndk_stub_libs"},
Blueprint: soongCcLibrarySharedPreamble + `
cc_library_shared {
name: "libUsesSdk",
@@ -1621,9 +1621,7 @@
}
ndk_library {
name: "libHasApexAndNdkStubs",
- // TODO: b/301321658 - Stub this once existing-build-file handling can deal with
- // modules that generate targets of a different name.
- bazel_module: { bp2build_available: false },
+ first_version: "28",
}
`,
ExpectedBazelTargets: []string{
diff --git a/bp2build/java_binary_host_conversion_test.go b/bp2build/java_binary_host_conversion_test.go
index 7d8ab63..4271f76 100644
--- a/bp2build/java_binary_host_conversion_test.go
+++ b/bp2build/java_binary_host_conversion_test.go
@@ -114,7 +114,7 @@
runJavaBinaryHostTestCase(t, Bp2buildTestCase{
Description: "java_binary_host with srcs, libs.",
Filesystem: testFs,
- StubbedBuildDefinitions: []string{"prebuilt_java-lib-dep-1"},
+ StubbedBuildDefinitions: []string{"java-lib-dep-1", "java-lib-dep-1-neverlink"},
Blueprint: `java_binary_host {
name: "java-binary-host-libs",
libs: ["java-lib-dep-1"],