Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 20f0f780 authored by Liz Kammer's avatar Liz Kammer
Browse files

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
parent 12d170dc

android/bazel.go

+2 −2
Original line number Diff line number Diff line
@@ -431,7 +431,7 @@ func (b *BazelModuleBase) shouldConvertWithBp2build(ctx bazelOtherModuleContext, 
	}



	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 @@ func (b *BazelModuleBase) shouldConvertWithBp2build(ctx bazelOtherModuleContext, 
		return true

	}



	moduleName := module.Name()

	moduleName := moduleNameWithPossibleOverride(ctx, module)

	allowlist := ctx.Config().Bp2buildPackageConfig

	moduleNameAllowed := allowlist.moduleAlwaysConvert[moduleName]

	moduleTypeAllowed := allowlist.moduleTypeAlwaysConvert[ctx.OtherModuleType(module)]

android/bazel_paths.go

+2 −2
Original line number Diff line number Diff line
@@ -453,8 +453,8 @@ func samePackage(label1, label2 string) bool { 
}



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 = ""

	}

android/override_module.go

+27 −24
Original line number Diff line number Diff line
@@ -28,7 +28,6 @@ package android 
// module based on it.



import (

	"fmt"

	"sort"

	"sync"
@@ -121,7 +120,7 @@ type OverridableModule interface { 
	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 @@ func (b *OverridableModuleBase) setOverridesProperty(overridesProperty *[]string 
}



// 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 @@ func (b *OverridableModuleBase) override(ctx BaseModuleContext, o OverrideModule 
	}

	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 @@ func performOverrideMutator(ctx BottomUpMutatorContext) { 
		// 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 @@ 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 {

	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)

}

apex/apex.go

+0 −3
Original line number Diff line number Diff line
@@ -1964,9 +1964,6 @@ func (a *apexBundle) QueueBazelCall(ctx android.BaseModuleContext) { 
// 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)

}

apex/bp2build_test.go

+181 −97
Original line number Diff line number Diff line
@@ -15,7 +15,10 @@ package apex 


import (

	"android/soong/android"

	"android/soong/android/allowlists"

	"android/soong/bazel/cquery"

	"fmt"

	"path/filepath"

	"strings"

	"testing"

)
@@ -326,7 +329,7 @@ apex { 
}



func TestOverrideApexImageInMixedBuilds(t *testing.T) {

	bp := `

	originalBp := `

apex_key{

	name: "foo_key",

}
@@ -340,25 +343,104 @@ apex { 
	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

}

`



	originalApexBpDir := "original"

	originalApexName := "foo"

	overrideApexBpDir := "override"

	overrideApexName := "override_foo"



	defaultApexLabel := fmt.Sprintf("//%s:%s", originalApexBpDir, originalApexName)

	defaultOverrideApexLabel := fmt.Sprintf("//%s:%s", overrideApexBpDir, overrideApexName)



	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{}),

		},

	}



	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{

					"//:foo": cquery.ApexInfo{

							tc.apexLabel: cquery.ApexInfo{

								// ApexInfo Starlark provider

								SignedOutput:          "signed_out.apex",

								UnsignedOutput:        "unsigned_out.apex",
@@ -377,7 +459,7 @@ override_apex { 
								// ApexMkInfo Starlark provider

								MakeModulesToInstall: []string{"c"}, // d deliberately omitted

							},

					"//:override_foo": cquery.ApexInfo{

							tc.overrideApexLabel: cquery.ApexInfo{

								// ApexInfo Starlark provider

								SignedOutput:          "override_signed_out.apex",

								UnsignedOutput:        "override_unsigned_out.apex",
@@ -399,7 +481,7 @@ override_apex { 
						},

					}

				}),

	).RunTestWithBp(t, bp)

			).RunTest(t)



			m := result.ModuleForTests("foo", "android_common_override_foo_foo_image").Module()

			ab, ok := m.(*apexBundle)
@@ -415,11 +497,11 @@ override_apex { 
				t.Errorf("Expected private key %q, got %q", w, g)

			}



	if w, g := "out/bazel/execroot/__main__/override_container_cert", ab.containerCertificateFile.String(); 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_private", ab.containerPrivateKeyFile.String(); 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)

			}
@@ -452,10 +534,12 @@ override_apex { 
			}



			// make modules to be installed to system

	if len(ab.makeModulesToInstall) != 1 && ab.makeModulesToInstall[0] != "c" {

			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)

			}

		})

	}

}