Correct allowlisting for override modules
Prevoiusly, we were partially correcting for override modules in
bp2build/mixed builds in some but not all places. Now we always check
for override modules and ensure that Bazel_module properties are
propagated properly for override modules.
Bug: 279609939
Test: go test soong tests
Change-Id: I5445aa71f4c8013315415a2ca9ab9c6b3be6bce0
diff --git a/android/bazel.go b/android/bazel.go
index 58d9d87..3fe063c 100644
--- a/android/bazel.go
+++ b/android/bazel.go
@@ -431,7 +431,7 @@
}
propValue := b.bazelProperties.Bazel_module.Bp2build_available
- packagePath := ctx.OtherModuleDir(module)
+ packagePath := moduleDirWithPossibleOverride(ctx, module)
// 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
@@ -440,7 +440,7 @@
return true
}
- moduleName := module.Name()
+ moduleName := moduleNameWithPossibleOverride(ctx, module)
allowlist := ctx.Config().Bp2buildPackageConfig
moduleNameAllowed := allowlist.moduleAlwaysConvert[moduleName]
moduleTypeAllowed := allowlist.moduleTypeAlwaysConvert[ctx.OtherModuleType(module)]
diff --git a/android/bazel_paths.go b/android/bazel_paths.go
index bad7baf..ddbdbd4 100644
--- a/android/bazel_paths.go
+++ b/android/bazel_paths.go
@@ -453,8 +453,8 @@
}
func bp2buildModuleLabel(ctx BazelConversionContext, module blueprint.Module) string {
- moduleName := ctx.OtherModuleName(module)
- moduleDir := ctx.OtherModuleDir(module)
+ moduleName := moduleNameWithPossibleOverride(ctx, module)
+ moduleDir := moduleDirWithPossibleOverride(ctx, module)
if moduleDir == Bp2BuildTopLevel {
moduleDir = ""
}
diff --git a/android/override_module.go b/android/override_module.go
index 86f582b..9e95c0f 100644
--- a/android/override_module.go
+++ b/android/override_module.go
@@ -28,7 +28,6 @@
// module based on it.
import (
- "fmt"
"sort"
"sync"
@@ -121,7 +120,7 @@
addOverride(o OverrideModule)
getOverrides() []OverrideModule
- override(ctx BaseModuleContext, o OverrideModule)
+ override(ctx BaseModuleContext, m Module, o OverrideModule)
GetOverriddenBy() string
GetOverriddenByModuleDir() string
@@ -192,7 +191,8 @@
}
// Overrides a base module with the given OverrideModule.
-func (b *OverridableModuleBase) override(ctx BaseModuleContext, o OverrideModule) {
+func (b *OverridableModuleBase) override(ctx BaseModuleContext, m Module, o OverrideModule) {
+
for _, p := range b.overridableProperties {
for _, op := range o.getOverridingProperties() {
if proptools.TypeEqual(p, op) {
@@ -214,6 +214,17 @@
}
b.overridableModuleProperties.OverriddenBy = o.Name()
b.overridableModuleProperties.OverriddenByModuleDir = o.ModuleDir()
+
+ if oBazelable, ok := o.base().module.(Bazelable); ok {
+ if bBazelable, ok := m.(Bazelable); ok {
+ oProps := oBazelable.bazelProps()
+ bProps := bBazelable.bazelProps()
+ bProps.Bazel_module.Bp2build_available = oProps.Bazel_module.Bp2build_available
+ bProps.Bazel_module.Label = oProps.Bazel_module.Label
+ } else {
+ ctx.ModuleErrorf("Override type cannot be Bazelable if original module type is not Bazelable %v %v.", o.Name(), m.Name())
+ }
+ }
}
// GetOverriddenBy returns the name of the override module that has overridden this module.
@@ -302,7 +313,7 @@
// is specified.
ctx.AliasVariation(variants[0])
for i, o := range overrides {
- mods[i+1].(OverridableModule).override(ctx, o)
+ mods[i+1].(OverridableModule).override(ctx, mods[i+1], o)
if o.getOverriddenByPrebuilt() {
// The overriding module itself, too, is overridden by a prebuilt.
// Copy the flag and hide it in make
@@ -340,34 +351,26 @@
// 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 {
- if overridable, ok := ctx.Module().(OverridableModule); ok {
+ return moduleNameWithPossibleOverride(ctx, ctx.Module())
+}
+
+func moduleNameWithPossibleOverride(ctx bazelOtherModuleContext, module blueprint.Module) string {
+ if overridable, ok := module.(OverridableModule); ok {
if o := overridable.GetOverriddenBy(); o != "" {
return o
}
}
- return ctx.OtherModuleName(ctx.Module())
+ return ctx.OtherModuleName(module)
}
-// ModuleDirWithPossibleOverride returns the dir of the OverrideModule that overrides the current
-// variant of this OverridableModule, or ctx.ModuleName() if this module is not an OverridableModule
-// or if this variant is not overridden.
-func moduleDirWithPossibleOverride(ctx BazelConversionContext) string {
- if overridable, ok := ctx.Module().(OverridableModule); ok {
+// 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 {
+ if overridable, ok := module.(OverridableModule); ok {
if o := overridable.GetOverriddenByModuleDir(); o != "" {
return o
}
}
- return ctx.OtherModuleDir(ctx.Module())
-}
-
-// MaybeBp2buildLabelOfOverridingModule returns the bazel label of the
-// overriding module of an OverridableModule (e.g. override_apex label of a base
-// apex), or the module's label itself if not overridden.
-func MaybeBp2buildLabelOfOverridingModule(ctx BazelConversionContext) string {
- moduleName := ModuleNameWithPossibleOverride(ctx)
- moduleDir := moduleDirWithPossibleOverride(ctx)
- if moduleDir == Bp2BuildTopLevel {
- moduleDir = ""
- }
- return fmt.Sprintf("//%s:%s", moduleDir, moduleName)
+ return ctx.OtherModuleDir(module)
}
diff --git a/apex/apex.go b/apex/apex.go
index baf4737..3cd541c 100644
--- a/apex/apex.go
+++ b/apex/apex.go
@@ -1964,9 +1964,6 @@
// overridden by different override_apex modules (e.g. Google or Go variants),
// which is handled by the overrides mutators.
func (a *apexBundle) GetBazelLabel(ctx android.BazelConversionPathContext, module blueprint.Module) string {
- if _, ok := ctx.Module().(android.OverridableModule); ok {
- return android.MaybeBp2buildLabelOfOverridingModule(ctx)
- }
return a.BazelModuleBase.GetBazelLabel(ctx, a)
}
diff --git a/apex/bp2build_test.go b/apex/bp2build_test.go
index 2a0f6e9..b1b6a75 100644
--- a/apex/bp2build_test.go
+++ b/apex/bp2build_test.go
@@ -15,7 +15,10 @@
import (
"android/soong/android"
+ "android/soong/android/allowlists"
"android/soong/bazel/cquery"
+ "fmt"
+ "path/filepath"
"strings"
"testing"
)
@@ -326,7 +329,7 @@
}
func TestOverrideApexImageInMixedBuilds(t *testing.T) {
- bp := `
+ originalBp := `
apex_key{
name: "foo_key",
}
@@ -340,122 +343,203 @@
min_sdk_version: "31",
package_name: "pkg_name",
file_contexts: ":myapex-file_contexts",
- bazel_module: { label: "//:foo" },
-}
+ %s
+}`
+ overrideBp := `
override_apex {
name: "override_foo",
key: "override_foo_key",
package_name: "override_pkg_name",
base: "foo",
- bazel_module: { label: "//:override_foo" },
+ %s
}
`
- outputBaseDir := "out/bazel"
- result := android.GroupFixturePreparers(
- prepareForApexTest,
- android.FixtureModifyConfig(func(config android.Config) {
- config.BazelContext = android.MockBazelContext{
- OutputBaseDir: outputBaseDir,
- LabelToApexInfo: map[string]cquery.ApexInfo{
- "//:foo": cquery.ApexInfo{
- // ApexInfo Starlark provider
- SignedOutput: "signed_out.apex",
- UnsignedOutput: "unsigned_out.apex",
- BundleKeyInfo: []string{"public_key", "private_key"},
- ContainerKeyInfo: []string{"container_cert", "container_private"},
- SymbolsUsedByApex: "foo_using.txt",
- JavaSymbolsUsedByApex: "foo_using.xml",
- BundleFile: "apex_bundle.zip",
- InstalledFiles: "installed-files.txt",
- RequiresLibs: []string{"//path/c:c", "//path/d:d"},
+ originalApexBpDir := "original"
+ originalApexName := "foo"
+ overrideApexBpDir := "override"
+ overrideApexName := "override_foo"
- // unused
- PackageName: "pkg_name",
- ProvidesLibs: []string{"a", "b"},
+ defaultApexLabel := fmt.Sprintf("//%s:%s", originalApexBpDir, originalApexName)
+ defaultOverrideApexLabel := fmt.Sprintf("//%s:%s", overrideApexBpDir, overrideApexName)
- // ApexMkInfo Starlark provider
- MakeModulesToInstall: []string{"c"}, // d deliberately omitted
- },
- "//:override_foo": cquery.ApexInfo{
- // ApexInfo Starlark provider
- SignedOutput: "override_signed_out.apex",
- UnsignedOutput: "override_unsigned_out.apex",
- BundleKeyInfo: []string{"override_public_key", "override_private_key"},
- ContainerKeyInfo: []string{"override_container_cert", "override_container_private"},
- SymbolsUsedByApex: "override_foo_using.txt",
- JavaSymbolsUsedByApex: "override_foo_using.xml",
- BundleFile: "override_apex_bundle.zip",
- InstalledFiles: "override_installed-files.txt",
- RequiresLibs: []string{"//path/c:c", "//path/d:d"},
+ testCases := []struct {
+ desc string
+ bazelModuleProp string
+ apexLabel string
+ overrideBazelModuleProp string
+ overrideApexLabel string
+ bp2buildConfiguration android.Bp2BuildConversionAllowlist
+ }{
+ {
+ desc: "both explicit labels",
+ bazelModuleProp: `bazel_module: { label: "//:foo" },`,
+ apexLabel: "//:foo",
+ overrideBazelModuleProp: `bazel_module: { label: "//:override_foo" },`,
+ overrideApexLabel: "//:override_foo",
+ bp2buildConfiguration: android.NewBp2BuildAllowlist(),
+ },
+ {
+ desc: "both explicitly allowed",
+ bazelModuleProp: `bazel_module: { bp2build_available: true },`,
+ apexLabel: defaultApexLabel,
+ overrideBazelModuleProp: `bazel_module: { bp2build_available: true },`,
+ overrideApexLabel: defaultOverrideApexLabel,
+ bp2buildConfiguration: android.NewBp2BuildAllowlist(),
+ },
+ {
+ desc: "original allowed by dir, override allowed by name",
+ apexLabel: defaultApexLabel,
+ overrideApexLabel: defaultOverrideApexLabel,
+ bp2buildConfiguration: android.NewBp2BuildAllowlist().SetDefaultConfig(
+ map[string]allowlists.BazelConversionConfigEntry{
+ originalApexBpDir: allowlists.Bp2BuildDefaultTrue,
+ }).SetModuleAlwaysConvertList([]string{
+ overrideApexName,
+ }),
+ },
+ {
+ desc: "both allowed by name",
+ apexLabel: defaultApexLabel,
+ overrideApexLabel: defaultOverrideApexLabel,
+ bp2buildConfiguration: android.NewBp2BuildAllowlist().SetModuleAlwaysConvertList([]string{
+ originalApexName,
+ overrideApexName,
+ }),
+ },
+ {
+ desc: "override allowed by name",
+ apexLabel: defaultApexLabel,
+ overrideApexLabel: defaultOverrideApexLabel,
+ bp2buildConfiguration: android.NewBp2BuildAllowlist().SetModuleAlwaysConvertList([]string{
+ overrideApexName,
+ }),
+ },
+ {
+ desc: "override allowed by dir",
+ apexLabel: defaultApexLabel,
+ overrideApexLabel: defaultOverrideApexLabel,
+ bp2buildConfiguration: android.NewBp2BuildAllowlist().SetDefaultConfig(
+ map[string]allowlists.BazelConversionConfigEntry{
+ overrideApexBpDir: allowlists.Bp2BuildDefaultTrue,
+ }).SetModuleAlwaysConvertList([]string{}),
+ },
+ }
- // unused
- PackageName: "override_pkg_name",
- ProvidesLibs: []string{"a", "b"},
+ for _, tc := range testCases {
+ t.Run(tc.desc, func(t *testing.T) {
+ outputBaseDir := "out/bazel"
+ result := android.GroupFixturePreparers(
+ prepareForApexTest,
+ android.FixtureAddTextFile(filepath.Join(originalApexBpDir, "Android.bp"), fmt.Sprintf(originalBp, tc.bazelModuleProp)),
+ android.FixtureAddTextFile(filepath.Join(overrideApexBpDir, "Android.bp"), fmt.Sprintf(overrideBp, tc.overrideBazelModuleProp)),
+ android.FixtureModifyContext(func(ctx *android.TestContext) {
+ ctx.RegisterBp2BuildConfig(tc.bp2buildConfiguration)
+ }),
+ android.FixtureModifyConfig(func(config android.Config) {
+ config.BazelContext = android.MockBazelContext{
+ OutputBaseDir: outputBaseDir,
+ LabelToApexInfo: map[string]cquery.ApexInfo{
+ tc.apexLabel: cquery.ApexInfo{
+ // ApexInfo Starlark provider
+ SignedOutput: "signed_out.apex",
+ UnsignedOutput: "unsigned_out.apex",
+ BundleKeyInfo: []string{"public_key", "private_key"},
+ ContainerKeyInfo: []string{"container_cert", "container_private"},
+ SymbolsUsedByApex: "foo_using.txt",
+ JavaSymbolsUsedByApex: "foo_using.xml",
+ BundleFile: "apex_bundle.zip",
+ InstalledFiles: "installed-files.txt",
+ RequiresLibs: []string{"//path/c:c", "//path/d:d"},
- // ApexMkInfo Starlark provider
- MakeModulesToInstall: []string{"c"}, // d deliberately omitted
- },
- },
+ // unused
+ PackageName: "pkg_name",
+ ProvidesLibs: []string{"a", "b"},
+
+ // ApexMkInfo Starlark provider
+ MakeModulesToInstall: []string{"c"}, // d deliberately omitted
+ },
+ tc.overrideApexLabel: cquery.ApexInfo{
+ // ApexInfo Starlark provider
+ SignedOutput: "override_signed_out.apex",
+ UnsignedOutput: "override_unsigned_out.apex",
+ BundleKeyInfo: []string{"override_public_key", "override_private_key"},
+ ContainerKeyInfo: []string{"override_container_cert", "override_container_private"},
+ SymbolsUsedByApex: "override_foo_using.txt",
+ JavaSymbolsUsedByApex: "override_foo_using.xml",
+ BundleFile: "override_apex_bundle.zip",
+ InstalledFiles: "override_installed-files.txt",
+ RequiresLibs: []string{"//path/c:c", "//path/d:d"},
+
+ // unused
+ PackageName: "override_pkg_name",
+ ProvidesLibs: []string{"a", "b"},
+
+ // ApexMkInfo Starlark provider
+ MakeModulesToInstall: []string{"c"}, // d deliberately omitted
+ },
+ },
+ }
+ }),
+ ).RunTest(t)
+
+ m := result.ModuleForTests("foo", "android_common_override_foo_foo_image").Module()
+ ab, ok := m.(*apexBundle)
+ if !ok {
+ t.Fatalf("Expected module to be an apexBundle, was not")
}
- }),
- ).RunTestWithBp(t, bp)
- m := result.ModuleForTests("foo", "android_common_override_foo_foo_image").Module()
- ab, ok := m.(*apexBundle)
- if !ok {
- t.Fatalf("Expected module to be an apexBundle, was not")
- }
+ if w, g := "out/bazel/execroot/__main__/override_public_key", ab.publicKeyFile.String(); w != g {
+ t.Errorf("Expected public key %q, got %q", w, g)
+ }
- if w, g := "out/bazel/execroot/__main__/override_public_key", ab.publicKeyFile.String(); w != g {
- t.Errorf("Expected public key %q, got %q", w, g)
- }
+ if w, g := "out/bazel/execroot/__main__/override_private_key", ab.privateKeyFile.String(); w != g {
+ t.Errorf("Expected private key %q, got %q", w, g)
+ }
- if w, g := "out/bazel/execroot/__main__/override_private_key", ab.privateKeyFile.String(); w != g {
- t.Errorf("Expected private key %q, got %q", w, g)
- }
+ if w, g := "out/bazel/execroot/__main__/override_container_cert", ab.containerCertificateFile; g != nil && w != g.String() {
+ t.Errorf("Expected public container key %q, got %q", w, g)
+ }
- if w, g := "out/bazel/execroot/__main__/override_container_cert", ab.containerCertificateFile.String(); w != g {
- t.Errorf("Expected public container key %q, got %q", w, g)
- }
+ if w, g := "out/bazel/execroot/__main__/override_container_private", ab.containerPrivateKeyFile; g != nil && w != g.String() {
+ t.Errorf("Expected private container key %q, got %q", w, g)
+ }
- if w, g := "out/bazel/execroot/__main__/override_container_private", ab.containerPrivateKeyFile.String(); w != g {
- t.Errorf("Expected private container key %q, got %q", w, g)
- }
+ if w, g := "out/bazel/execroot/__main__/override_signed_out.apex", ab.outputFile.String(); w != g {
+ t.Errorf("Expected output file %q, got %q", w, g)
+ }
- if w, g := "out/bazel/execroot/__main__/override_signed_out.apex", ab.outputFile.String(); w != g {
- t.Errorf("Expected output file %q, got %q", w, g)
- }
+ if w, g := "out/bazel/execroot/__main__/override_foo_using.txt", ab.nativeApisUsedByModuleFile.String(); w != g {
+ t.Errorf("Expected output file %q, got %q", w, g)
+ }
- if w, g := "out/bazel/execroot/__main__/override_foo_using.txt", ab.nativeApisUsedByModuleFile.String(); w != g {
- t.Errorf("Expected output file %q, got %q", w, g)
- }
+ if w, g := "out/bazel/execroot/__main__/override_foo_using.xml", ab.javaApisUsedByModuleFile.String(); w != g {
+ t.Errorf("Expected output file %q, got %q", w, g)
+ }
- if w, g := "out/bazel/execroot/__main__/override_foo_using.xml", ab.javaApisUsedByModuleFile.String(); w != g {
- t.Errorf("Expected output file %q, got %q", w, g)
- }
+ if w, g := "out/bazel/execroot/__main__/override_installed-files.txt", ab.installedFilesFile.String(); w != g {
+ t.Errorf("Expected installed-files.txt %q, got %q", w, g)
+ }
- if w, g := "out/bazel/execroot/__main__/override_installed-files.txt", ab.installedFilesFile.String(); w != g {
- t.Errorf("Expected installed-files.txt %q, got %q", w, g)
- }
+ mkData := android.AndroidMkDataForTest(t, result.TestContext, m)
+ var builder strings.Builder
+ mkData.Custom(&builder, "override_foo", "BAZEL_TARGET_", "", mkData)
- mkData := android.AndroidMkDataForTest(t, result.TestContext, m)
- var builder strings.Builder
- mkData.Custom(&builder, "override_foo", "BAZEL_TARGET_", "", mkData)
+ data := builder.String()
+ if w := "ALL_MODULES.$(my_register_name).BUNDLE := out/bazel/execroot/__main__/override_apex_bundle.zip"; !strings.Contains(data, w) {
+ t.Errorf("Expected %q in androidmk data, but did not find %q", w, data)
+ }
+ if w := "$(call dist-for-goals,checkbuild,out/bazel/execroot/__main__/override_installed-files.txt:override_foo-installed-files.txt)"; !strings.Contains(data, w) {
+ t.Errorf("Expected %q in androidmk data, but did not find %q", w, data)
+ }
- data := builder.String()
- if w := "ALL_MODULES.$(my_register_name).BUNDLE := out/bazel/execroot/__main__/override_apex_bundle.zip"; !strings.Contains(data, w) {
- t.Errorf("Expected %q in androidmk data, but did not find %q", w, data)
- }
- if w := "$(call dist-for-goals,checkbuild,out/bazel/execroot/__main__/override_installed-files.txt:override_foo-installed-files.txt)"; !strings.Contains(data, w) {
- t.Errorf("Expected %q in androidmk data, but did not find %q", w, data)
- }
-
- // make modules to be installed to system
- if len(ab.makeModulesToInstall) != 1 && ab.makeModulesToInstall[0] != "c" {
- t.Errorf("Expected makeModulestoInstall slice to only contain 'c', got %q", ab.makeModulesToInstall)
- }
- if w := "LOCAL_REQUIRED_MODULES := c"; !strings.Contains(data, w) {
- t.Errorf("Expected %q in androidmk data, but did not find it in %q", w, data)
+ // make modules to be installed to system
+ if len(ab.makeModulesToInstall) != 1 || ab.makeModulesToInstall[0] != "c" {
+ t.Errorf("Expected makeModulestoInstall slice to only contain 'c', got %q", ab.makeModulesToInstall)
+ }
+ if w := "LOCAL_REQUIRED_MODULES := c"; !strings.Contains(data, w) {
+ t.Errorf("Expected %q in androidmk data, but did not find it in %q", w, data)
+ }
+ })
}
}