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

Commit 912bc886 authored by Cole Faust's avatar Cole Faust
Browse files

Use product variables from the overridden apex 

override_apex's bp2build converter had a bug where it was looking at
the product variables for the override_apex module itself instead of
for the base module it is overriding.

Fixes: 271424349
Test: go test
Change-Id: If1e2653d3751fa908faf0ab97dfa2e943ebe98ec
parent 8eb8ae64

android/module.go

+1 −1
Original line number Diff line number Diff line
@@ -1334,7 +1334,7 @@ func (attrs *CommonAttributes) fillCommonBp2BuildModuleAttrs(ctx *topDownMutator 
// Check product variables for `enabled: true` flag override.

// Returns a list of the constraint_value targets who enable this override.

func productVariableConfigEnableLabels(ctx *topDownMutatorContext) []bazel.Label {

	productVariableProps := ProductVariableProperties(ctx)

	productVariableProps := ProductVariableProperties(ctx, ctx.Module())

	productConfigEnablingTargets := []bazel.Label{}

	const propName = "Enabled"

	if productConfigProps, exists := productVariableProps[propName]; exists {

android/variable.go

+2 −3
Original line number Diff line number Diff line
@@ -663,9 +663,8 @@ func (p *ProductConfigProperty) SelectKey() string { 
type ProductConfigProperties map[string]map[ProductConfigProperty]interface{}



// ProductVariableProperties returns a ProductConfigProperties containing only the properties which

// have been set for the module in the given context.

func ProductVariableProperties(ctx BazelConversionPathContext) ProductConfigProperties {

	module := ctx.Module()

// have been set for the given module.

func ProductVariableProperties(ctx ArchVariantContext, module Module) ProductConfigProperties {

	moduleBase := module.base()



	productConfigProperties := ProductConfigProperties{}

apex/apex.go

+3 −8
Original line number Diff line number Diff line
@@ -50,7 +50,7 @@ func registerApexBuildComponents(ctx android.RegistrationContext) { 
	ctx.RegisterModuleType("apex", BundleFactory)

	ctx.RegisterModuleType("apex_test", TestApexBundleFactory)

	ctx.RegisterModuleType("apex_vndk", vndkApexBundleFactory)

	ctx.RegisterModuleType("apex_defaults", defaultsFactory)

	ctx.RegisterModuleType("apex_defaults", DefaultsFactory)

	ctx.RegisterModuleType("prebuilt_apex", PrebuiltFactory)

	ctx.RegisterModuleType("override_apex", OverrideApexFactory)

	ctx.RegisterModuleType("apex_set", apexSetFactory)
@@ -2728,14 +2728,9 @@ type Defaults struct { 
}



// apex_defaults provides defaultable properties to other apex modules.

func defaultsFactory() android.Module {

	return DefaultsFactory()

}



func DefaultsFactory(props ...interface{}) android.Module {

func DefaultsFactory() android.Module {

	module := &Defaults{}



	module.AddProperties(props...)

	module.AddProperties(

		&apexBundleProperties{},

		&apexTargetBundleProperties{},
@@ -3538,7 +3533,7 @@ func convertWithBp2build(a *apexBundle, ctx android.TopDownMutatorContext) (baze 
		fileContextsLabelAttribute.SetValue(android.BazelLabelForModuleSrcSingle(ctx, *a.properties.File_contexts))

	}



	productVariableProps := android.ProductVariableProperties(ctx)

	productVariableProps := android.ProductVariableProperties(ctx, a)

	// TODO(b/219503907) this would need to be set to a.MinSdkVersionValue(ctx) but

	// given it's coming via config, we probably don't want to put it in here.

	var minSdkVersion bazel.StringAttribute

bp2build/apex_conversion_test.go

+87 −0
Original line number Diff line number Diff line
@@ -61,7 +61,10 @@ func registerOverrideApexModuleTypes(ctx android.RegistrationContext) { 
	ctx.RegisterModuleType("android_app_certificate", java.AndroidAppCertificateFactory)

	ctx.RegisterModuleType("filegroup", android.FileGroupFactory)

	ctx.RegisterModuleType("apex", apex.BundleFactory)

	ctx.RegisterModuleType("apex_defaults", apex.DefaultsFactory)

	ctx.RegisterModuleType("prebuilt_etc", etc.PrebuiltEtcFactory)

	ctx.RegisterModuleType("soong_config_module_type", android.SoongConfigModuleTypeFactory)

	ctx.RegisterModuleType("soong_config_string_variable", android.SoongConfigStringVariableDummyFactory)

}



func TestApexBundleSimple(t *testing.T) {
@@ -1359,3 +1362,87 @@ apex_test { 
			}),

		}})

}



func TestApexBundle_overridePlusProductVars(t *testing.T) {

	// Reproduction of b/271424349

	// Tests that overriding an apex that uses product variables correctly copies the product var

	// selects over to the override.

	runOverrideApexTestCase(t, Bp2buildTestCase{

		Description:                "apex - overriding a module that uses product vars",

		ModuleTypeUnderTest:        "override_apex",

		ModuleTypeUnderTestFactory: apex.OverrideApexFactory,

		Blueprint: `

soong_config_string_variable {

    name: "library_linking_strategy",

    values: [

        "prefer_static",

    ],

}



soong_config_module_type {

    name: "library_linking_strategy_apex_defaults",

    module_type: "apex_defaults",

    config_namespace: "ANDROID",

    variables: ["library_linking_strategy"],

    properties: [

        "manifest",

        "min_sdk_version",

    ],

}



library_linking_strategy_apex_defaults {

    name: "higher_min_sdk_when_prefer_static",

    soong_config_variables: {

        library_linking_strategy: {

            // Use the R min_sdk_version

            prefer_static: {},

            // Override the R min_sdk_version to min_sdk_version that supports dcla

            conditions_default: {

                min_sdk_version: "31",

            },

        },

    },

}



filegroup {

	name: "foo-file_contexts",

	srcs: [

		"com.android.apogee-file_contexts",

	],

	bazel_module: { bp2build_available: false },

}



apex {

	name: "foo",

	defaults: ["higher_min_sdk_when_prefer_static"],

	min_sdk_version: "30",

	package_name: "pkg_name",

	file_contexts: ":foo-file_contexts",

}

override_apex {

	name: "override_foo",

	base: ":foo",

	package_name: "override_pkg_name",

}

`,

		ExpectedBazelTargets: []string{

			MakeBazelTarget("apex", "foo", AttrNameToString{

				"file_contexts": `":foo-file_contexts"`,

				"manifest":      `"apex_manifest.json"`,

				"min_sdk_version": `select({

        "//build/bazel/product_variables:android__library_linking_strategy__prefer_static": "30",

        "//conditions:default": "31",

    })`,

				"package_name": `"pkg_name"`,

			}), MakeBazelTarget("apex", "override_foo", AttrNameToString{

				"base_apex_name": `"foo"`,

				"file_contexts":  `":foo-file_contexts"`,

				"manifest":       `"apex_manifest.json"`,

				"min_sdk_version": `select({

        "//build/bazel/product_variables:android__library_linking_strategy__prefer_static": "30",

        "//conditions:default": "31",

    })`,

				"package_name": `"override_pkg_name"`,

			}),

		}})

}

bp2build/testing.go

+1 −1
Original line number Diff line number Diff line
@@ -454,7 +454,7 @@ func (m *customModule) ConvertWithBp2build(ctx android.TopDownMutatorContext) { 
			}

		}

	}

	productVariableProps := android.ProductVariableProperties(ctx)

	productVariableProps := android.ProductVariableProperties(ctx, ctx.Module())

	if props, ok := productVariableProps["String_literal_prop"]; ok {

		for c, p := range props {

			if val, ok := p.(*string); ok {