Support `enabled` flag in product variable config
Some Android.bp modules have `enabled: false` but only use a product
variable such as `source_build` to enable the module. Currently b2build
does not handle this case at all. This commit adds the functionality
to support this use case.
Also, remove `__enabled` suffix in ProductVariable SelectKey.
Bug: 210546943
Test: go test ./bp2build
Topic: use_enabled_flag_product_variable_config
Change-Id: I459c17a84c172df010666391066bf4d11d19253e
diff --git a/android/module.go b/android/module.go
index c2fa848..5053720 100644
--- a/android/module.go
+++ b/android/module.go
@@ -1139,20 +1139,71 @@
}
}
- data.Append(required)
+ productConfigEnabledLabels := []bazel.Label{}
+ if !proptools.BoolDefault(enabledProperty.Value, true) {
+ // If the module is not enabled by default, then we can check if a
+ // product variable enables it
+ productConfigEnabledLabels = productVariableConfigEnableLabels(ctx)
- var err error
- constraints := constraintAttributes{}
- constraints.Target_compatible_with, err = enabledProperty.ToLabelListAttribute(
+ if len(productConfigEnabledLabels) > 0 {
+ // In this case, an existing product variable configuration overrides any
+ // module-level `enable: false` definition
+ newValue := true
+ enabledProperty.Value = &newValue
+ }
+ }
+
+ productConfigEnabledAttribute := bazel.MakeLabelListAttribute(bazel.LabelList{
+ productConfigEnabledLabels, nil,
+ })
+
+ platformEnabledAttribute, err := enabledProperty.ToLabelListAttribute(
bazel.LabelList{[]bazel.Label{bazel.Label{Label: "@platforms//:incompatible"}}, nil},
bazel.LabelList{[]bazel.Label{}, nil})
-
if err != nil {
- ctx.ModuleErrorf("Error processing enabled attribute: %s", err)
+ ctx.ModuleErrorf("Error processing platform enabled attribute: %s", err)
}
+
+ data.Append(required)
+
+ constraints := constraintAttributes{}
+ moduleEnableConstraints := bazel.LabelListAttribute{}
+ moduleEnableConstraints.Append(platformEnabledAttribute)
+ moduleEnableConstraints.Append(productConfigEnabledAttribute)
+ constraints.Target_compatible_with = moduleEnableConstraints
+
return constraints
}
+// 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)
+ productConfigEnablingTargets := []bazel.Label{}
+ const propName = "Enabled"
+ if productConfigProps, exists := productVariableProps[propName]; exists {
+ for productConfigProp, prop := range productConfigProps {
+ flag, ok := prop.(*bool)
+ if !ok {
+ ctx.ModuleErrorf("Could not convert product variable %s property", proptools.PropertyNameForField(propName))
+ }
+
+ if *flag {
+ axis := productConfigProp.ConfigurationAxis()
+ targetLabel := axis.SelectKey(productConfigProp.SelectKey())
+ productConfigEnablingTargets = append(productConfigEnablingTargets, bazel.Label{
+ Label: targetLabel,
+ })
+ } else {
+ // TODO(b/210546943): handle negative case where `enabled: false`
+ ctx.ModuleErrorf("`enabled: false` is not currently supported for configuration variables. See b/210546943", proptools.PropertyNameForField(propName))
+ }
+ }
+ }
+
+ return productConfigEnablingTargets
+}
+
// A ModuleBase object contains the properties that are common to all Android
// modules. It should be included as an anonymous field in every module
// struct definition. InitAndroidModule should then be called from the module's
diff --git a/android/variable.go b/android/variable.go
index b300267..40dd2d8 100644
--- a/android/variable.go
+++ b/android/variable.go
@@ -601,10 +601,16 @@
value := p.FullConfig
if value == p.Name {
- value = "enabled"
+ value = ""
}
- // e.g. acme__feature1__enabled, android__board__soc_a
- return strings.ToLower(strings.Join([]string{p.Namespace, p.Name, value}, "__"))
+
+ // e.g. acme__feature1, android__board__soc_a
+ selectKey := strings.ToLower(strings.Join([]string{p.Namespace, p.Name}, "__"))
+ if value != "" {
+ selectKey = strings.ToLower(strings.Join([]string{selectKey, value}, "__"))
+ }
+
+ return selectKey
}
// ProductConfigProperties is a map of maps to group property values according
diff --git a/bp2build/soong_config_module_type_conversion_test.go b/bp2build/soong_config_module_type_conversion_test.go
index f1489aa..b1e1fb2 100644
--- a/bp2build/soong_config_module_type_conversion_test.go
+++ b/bp2build/soong_config_module_type_conversion_test.go
@@ -68,7 +68,7 @@
expectedBazelTargets: []string{`cc_library_static(
name = "foo",
copts = select({
- "//build/bazel/product_variables:acme__feature1__enabled": ["-DFEATURE1"],
+ "//build/bazel/product_variables:acme__feature1": ["-DFEATURE1"],
"//conditions:default": ["-DDEFAULT1"],
}),
local_includes = ["."],
@@ -116,7 +116,7 @@
expectedBazelTargets: []string{`cc_library_static(
name = "foo",
copts = select({
- "//build/bazel/product_variables:acme__feature1__enabled": ["-DFEATURE1"],
+ "//build/bazel/product_variables:acme__feature1": ["-DFEATURE1"],
"//conditions:default": ["-DDEFAULT1"],
}),
local_includes = ["."],
@@ -240,10 +240,10 @@
"//build/bazel/product_variables:acme__board__soc_b": ["-DSOC_B"],
"//conditions:default": ["-DSOC_DEFAULT"],
}) + select({
- "//build/bazel/product_variables:acme__feature1__enabled": ["-DFEATURE1"],
+ "//build/bazel/product_variables:acme__feature1": ["-DFEATURE1"],
"//conditions:default": ["-DDEFAULT1"],
}) + select({
- "//build/bazel/product_variables:acme__feature2__enabled": ["-DFEATURE2"],
+ "//build/bazel/product_variables:acme__feature2": ["-DFEATURE2"],
"//conditions:default": ["-DDEFAULT2"],
}),
local_includes = ["."],
@@ -367,7 +367,7 @@
expectedBazelTargets: []string{`cc_library_static(
name = "lib",
copts = select({
- "//build/bazel/product_variables:vendor_foo__feature__enabled": [
+ "//build/bazel/product_variables:vendor_foo__feature": [
"-cflag_feature_2",
"-cflag_feature_1",
],
@@ -446,11 +446,11 @@
expectedBazelTargets: []string{`cc_library_static(
name = "lib",
asflags = select({
- "//build/bazel/product_variables:acme__feature__enabled": ["-asflag_bar"],
+ "//build/bazel/product_variables:acme__feature": ["-asflag_bar"],
"//conditions:default": ["-asflag_default_bar"],
}),
copts = select({
- "//build/bazel/product_variables:acme__feature__enabled": [
+ "//build/bazel/product_variables:acme__feature": [
"-cflag_foo",
"-cflag_bar",
],
@@ -465,11 +465,11 @@
`cc_library_static(
name = "lib2",
asflags = select({
- "//build/bazel/product_variables:acme__feature__enabled": ["-asflag_bar"],
+ "//build/bazel/product_variables:acme__feature": ["-asflag_bar"],
"//conditions:default": ["-asflag_default_bar"],
}),
copts = select({
- "//build/bazel/product_variables:acme__feature__enabled": [
+ "//build/bazel/product_variables:acme__feature": [
"-cflag_bar",
"-cflag_foo",
],
@@ -561,13 +561,13 @@
expectedBazelTargets: []string{`cc_library_static(
name = "lib",
copts = select({
- "//build/bazel/product_variables:vendor_bar__feature__enabled": ["-DVENDOR_BAR_FEATURE"],
+ "//build/bazel/product_variables:vendor_bar__feature": ["-DVENDOR_BAR_FEATURE"],
"//conditions:default": ["-DVENDOR_BAR_DEFAULT"],
}) + select({
- "//build/bazel/product_variables:vendor_foo__feature__enabled": ["-DVENDOR_FOO_FEATURE"],
+ "//build/bazel/product_variables:vendor_foo__feature": ["-DVENDOR_FOO_FEATURE"],
"//conditions:default": ["-DVENDOR_FOO_DEFAULT"],
}) + select({
- "//build/bazel/product_variables:vendor_qux__feature__enabled": ["-DVENDOR_QUX_FEATURE"],
+ "//build/bazel/product_variables:vendor_qux__feature": ["-DVENDOR_QUX_FEATURE"],
"//conditions:default": ["-DVENDOR_QUX_DEFAULT"],
}),
local_includes = ["."],
@@ -834,3 +834,152 @@
srcs = ["main.cc"],
)`}})
}
+
+func TestSoongConfigModuleType_ProductVariableConfigWithPlatformConfig(t *testing.T) {
+ bp := `
+soong_config_bool_variable {
+ name: "special_build",
+}
+
+soong_config_module_type {
+ name: "alphabet_cc_defaults",
+ module_type: "cc_defaults",
+ config_namespace: "alphabet_module",
+ bool_variables: ["special_build"],
+ properties: ["enabled"],
+}
+
+alphabet_cc_defaults {
+ name: "alphabet_sample_cc_defaults",
+ soong_config_variables: {
+ special_build: {
+ enabled: true,
+ },
+ },
+}
+
+cc_binary {
+ name: "alphabet_binary",
+ srcs: ["main.cc"],
+ defaults: ["alphabet_sample_cc_defaults"],
+ enabled: false,
+ arch: {
+ x86_64: {
+ enabled: false,
+ },
+ },
+ target: {
+ darwin: {
+ enabled: false,
+ },
+ },
+}`
+
+ runSoongConfigModuleTypeTest(t, bp2buildTestCase{
+ description: "soong config variables - generates selects for library_linking_strategy",
+ moduleTypeUnderTest: "cc_binary",
+ moduleTypeUnderTestFactory: cc.BinaryFactory,
+ blueprint: bp,
+ filesystem: map[string]string{},
+ expectedBazelTargets: []string{`cc_binary(
+ name = "alphabet_binary",
+ local_includes = ["."],
+ srcs = ["main.cc"],
+ target_compatible_with = ["//build/bazel/product_variables:alphabet_module__special_build"] + select({
+ "//build/bazel/platforms/os_arch:android_x86_64": ["@platforms//:incompatible"],
+ "//build/bazel/platforms/os_arch:darwin_arm64": ["@platforms//:incompatible"],
+ "//build/bazel/platforms/os_arch:darwin_x86_64": ["@platforms//:incompatible"],
+ "//build/bazel/platforms/os_arch:linux_bionic_x86_64": ["@platforms//:incompatible"],
+ "//build/bazel/platforms/os_arch:linux_glibc_x86_64": ["@platforms//:incompatible"],
+ "//build/bazel/platforms/os_arch:linux_musl_x86_64": ["@platforms//:incompatible"],
+ "//build/bazel/platforms/os_arch:windows_x86_64": ["@platforms//:incompatible"],
+ "//conditions:default": [],
+ }),
+)`}})
+}
+
+func TestSoongConfigModuleType_ProductVariableConfigOverridesEnable(t *testing.T) {
+ bp := `
+soong_config_bool_variable {
+ name: "special_build",
+}
+
+soong_config_module_type {
+ name: "alphabet_cc_defaults",
+ module_type: "cc_defaults",
+ config_namespace: "alphabet_module",
+ bool_variables: ["special_build"],
+ properties: ["enabled"],
+}
+
+alphabet_cc_defaults {
+ name: "alphabet_sample_cc_defaults",
+ soong_config_variables: {
+ special_build: {
+ enabled: true,
+ },
+ },
+}
+
+cc_binary {
+ name: "alphabet_binary",
+ srcs: ["main.cc"],
+ defaults: ["alphabet_sample_cc_defaults"],
+ enabled: false,
+}`
+
+ runSoongConfigModuleTypeTest(t, bp2buildTestCase{
+ description: "soong config variables - generates selects for library_linking_strategy",
+ moduleTypeUnderTest: "cc_binary",
+ moduleTypeUnderTestFactory: cc.BinaryFactory,
+ blueprint: bp,
+ filesystem: map[string]string{},
+ expectedBazelTargets: []string{`cc_binary(
+ name = "alphabet_binary",
+ local_includes = ["."],
+ srcs = ["main.cc"],
+ target_compatible_with = ["//build/bazel/product_variables:alphabet_module__special_build"],
+)`}})
+}
+
+func TestSoongConfigModuleType_ProductVariableIgnoredIfEnabledByDefault(t *testing.T) {
+ bp := `
+soong_config_bool_variable {
+ name: "special_build",
+}
+
+soong_config_module_type {
+ name: "alphabet_cc_defaults",
+ module_type: "cc_defaults",
+ config_namespace: "alphabet_module",
+ bool_variables: ["special_build"],
+ properties: ["enabled"],
+}
+
+alphabet_cc_defaults {
+ name: "alphabet_sample_cc_defaults",
+ soong_config_variables: {
+ special_build: {
+ enabled: true,
+ },
+ },
+}
+
+cc_binary {
+ name: "alphabet_binary",
+ srcs: ["main.cc"],
+ defaults: ["alphabet_sample_cc_defaults"],
+}`
+
+ runSoongConfigModuleTypeTest(t, bp2buildTestCase{
+ description: "soong config variables - generates selects for library_linking_strategy",
+ moduleTypeUnderTest: "cc_binary",
+ moduleTypeUnderTestFactory: cc.BinaryFactory,
+ blueprint: bp,
+ filesystem: map[string]string{},
+ expectedBazelTargets: []string{`cc_binary(
+ name = "alphabet_binary",
+ local_includes = ["."],
+ srcs = ["main.cc"],
+)`}})
+}