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
diff --git a/android/module.go b/android/module.go
index 58c5464..773d77b 100644
--- a/android/module.go
+++ b/android/module.go
@@ -1334,7 +1334,7 @@
// 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 {
diff --git a/android/variable.go b/android/variable.go
index f7ac7d6..1b5d558 100644
--- a/android/variable.go
+++ b/android/variable.go
@@ -663,9 +663,8 @@
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{}
diff --git a/apex/apex.go b/apex/apex.go
index 88eb72f..f506876 100644
--- a/apex/apex.go
+++ b/apex/apex.go
@@ -50,7 +50,7 @@
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 @@
}
// 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 @@
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
diff --git a/bp2build/apex_conversion_test.go b/bp2build/apex_conversion_test.go
index 1c0e563..73c889f 100644
--- a/bp2build/apex_conversion_test.go
+++ b/bp2build/apex_conversion_test.go
@@ -61,7 +61,10 @@
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 @@
}),
}})
}
+
+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"`,
+ }),
+ }})
+}
diff --git a/bp2build/testing.go b/bp2build/testing.go
index 43baf98..bac5383 100644
--- a/bp2build/testing.go
+++ b/bp2build/testing.go
@@ -454,7 +454,7 @@
}
}
}
- 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 {
diff --git a/cc/bp2build.go b/cc/bp2build.go
index 1ea8bda..8644bf6 100644
--- a/cc/bp2build.go
+++ b/cc/bp2build.go
@@ -766,7 +766,7 @@
nativeCoverage = BoolPtr(false)
}
- productVariableProps := android.ProductVariableProperties(ctx)
+ productVariableProps := android.ProductVariableProperties(ctx, ctx.Module())
(&compilerAttrs).convertProductVariables(ctx, productVariableProps)
(&linkerAttrs).convertProductVariables(ctx, productVariableProps)
diff --git a/etc/prebuilt_etc.go b/etc/prebuilt_etc.go
index b0660df..dcd7fdc 100644
--- a/etc/prebuilt_etc.go
+++ b/etc/prebuilt_etc.go
@@ -717,7 +717,7 @@
}
}
- for propName, productConfigProps := range android.ProductVariableProperties(ctx) {
+ for propName, productConfigProps := range android.ProductVariableProperties(ctx, ctx.Module()) {
for configProp, propVal := range productConfigProps {
if propName == "Src" {
props, ok := propVal.(*string)