From 479e39f8fb705ab1fbd363b72cc337584ba777a7 Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Mon, 7 Aug 2023 22:43:58 +0000 Subject: [PATCH 1/2] Handle nil enabled values If enabled does not appear inside `soong_config_vars`, we can ignore it. Bug: 210546943 Test: go test ./bp2build Change-Id: I9e4d51c3b683f262921449634f827915ce87dc8d --- android/module.go | 5 +- ...oong_config_module_type_conversion_test.go | 55 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/android/module.go b/android/module.go index 0120a544e..a662130a7 100644 --- a/android/module.go +++ b/android/module.go @@ -1434,7 +1434,10 @@ func productVariableConfigEnableAttribute(ctx *topDownMutatorContext) bazel.Labe ctx.ModuleErrorf("Could not convert product variable enabled property") } - if *flag { + if flag == nil { + // soong config var is not used to set `enabled`. nothing to do. + continue + } else if *flag { axis := productConfigProp.ConfigurationAxis() result.SetSelectValue(axis, bazel.ConditionsDefaultConfigKey, bazel.MakeLabelList([]bazel.Label{{Label: "@platforms//:incompatible"}})) result.SetSelectValue(axis, productConfigProp.SelectKey(), bazel.LabelList{Includes: []bazel.Label{}}) diff --git a/bp2build/soong_config_module_type_conversion_test.go b/bp2build/soong_config_module_type_conversion_test.go index 1ff47f2f0..b91cf6d53 100644 --- a/bp2build/soong_config_module_type_conversion_test.go +++ b/bp2build/soong_config_module_type_conversion_test.go @@ -1520,3 +1520,58 @@ special_cc_defaults { runSoongConfigModuleTypeTest(t, bp2buildTestCase) } } + +func TestNoPanicIfEnabledIsNotUsed(t *testing.T) { + bp := ` +soong_config_string_variable { + name: "my_string_variable", + values: ["val1", "val2"], +} +soong_config_module_type { + name: "special_cc_defaults", + module_type: "cc_defaults", + config_namespace: "my_namespace", + variables: ["my_string_variable"], + properties: [ + "cflags", + "enabled", + ], +} +special_cc_defaults { + name: "my_special_cc_defaults", + soong_config_variables: { + my_string_variable: { + val1: { + cflags: ["-DFOO"], + }, + val2: { + cflags: ["-DBAR"], + }, + }, + }, +} +cc_binary { + name: "my_binary", + enabled: false, + defaults: ["my_special_cc_defaults"], +} +` + tc := Bp2buildTestCase{ + Description: "Soong config vars is not used to set `enabled` property", + ModuleTypeUnderTest: "cc_binary", + ModuleTypeUnderTestFactory: cc.BinaryFactory, + Blueprint: bp, + ExpectedBazelTargets: []string{ + MakeBazelTarget("cc_binary", "my_binary", AttrNameToString{ + "copts": `select({ + "//build/bazel/product_config/config_settings:my_namespace__my_string_variable__val1": ["-DFOO"], + "//build/bazel/product_config/config_settings:my_namespace__my_string_variable__val2": ["-DBAR"], + "//conditions:default": [], + })`, + "local_includes": `["."]`, + "target_compatible_with": `["@platforms//:incompatible"]`, + }), + }, + } + runSoongConfigModuleTypeTest(t, tc) +} From 846ce68a2f1bbfa68bc472cc028b84052ab9a57d Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Tue, 8 Aug 2023 02:00:22 +0000 Subject: [PATCH 2/2] Handle enabled: false via conditions_default In this Android.bp file ``` my_cc_defaults { enabled: false, soong_config_variables: { my_bool_variable: { conditions_default: {enabled: false}, } } } ``` The inner enabled: false is a no-op because the top-level enabled is false. Currently, bp2build will raise an exception for this Android.bp file. However, it does not need to. `productVariableConfigEnableLabels` runs only if the top-level enabled is false. If it sees enabled: false via conditions_default, it should just ignore it since it is a no-op. Test: go test ./bp2build Bug: 210546943 Change-Id: I816f209eaf21de65ddfbc2893e5255be94bcaa11 --- android/module.go | 4 +++ ...oong_config_module_type_conversion_test.go | 26 ++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/android/module.go b/android/module.go index a662130a7..19502bae8 100644 --- a/android/module.go +++ b/android/module.go @@ -1441,6 +1441,10 @@ func productVariableConfigEnableAttribute(ctx *topDownMutatorContext) bazel.Labe axis := productConfigProp.ConfigurationAxis() result.SetSelectValue(axis, bazel.ConditionsDefaultConfigKey, bazel.MakeLabelList([]bazel.Label{{Label: "@platforms//:incompatible"}})) result.SetSelectValue(axis, productConfigProp.SelectKey(), bazel.LabelList{Includes: []bazel.Label{}}) + } else if scp, isSoongConfigProperty := productConfigProp.(SoongConfigProperty); isSoongConfigProperty && scp.value == bazel.ConditionsDefaultConfigKey { + // productVariableConfigEnableAttribute runs only if `enabled: false` is set at the top-level outside soong_config_variables + // conditions_default { enabled: false} is a no-op in this case + continue } else { // TODO(b/210546943): handle negative case where `enabled: false` ctx.ModuleErrorf("`enabled: false` is not currently supported for configuration variables. See b/210546943") diff --git a/bp2build/soong_config_module_type_conversion_test.go b/bp2build/soong_config_module_type_conversion_test.go index b91cf6d53..8302ce87b 100644 --- a/bp2build/soong_config_module_type_conversion_test.go +++ b/bp2build/soong_config_module_type_conversion_test.go @@ -1243,6 +1243,24 @@ cc_binary { srcs: ["main.cc"], defaults: ["alphabet_sample_cc_defaults"], enabled: false, +} + +alphabet_cc_defaults { + name: "alphabet_sample_cc_defaults_conditions_default", + soong_config_variables: { + special_build: { + conditions_default: { + enabled: false, + }, + }, + }, +} + +cc_binary { + name: "alphabet_binary_conditions_default", + srcs: ["main.cc"], + defaults: ["alphabet_sample_cc_defaults_conditions_default"], + enabled: false, }` runSoongConfigModuleTypeTest(t, Bp2buildTestCase{ @@ -1259,7 +1277,13 @@ cc_binary { "//build/bazel/product_config/config_settings:alphabet_module__special_build": [], "//conditions:default": ["@platforms//:incompatible"], }), -)`}}) +)`, + MakeBazelTarget("cc_binary", "alphabet_binary_conditions_default", AttrNameToString{ + "local_includes": `["."]`, + "srcs": `["main.cc"]`, + "target_compatible_with": `["@platforms//:incompatible"]`, + }), + }}) } func TestSoongConfigModuleType_ProductVariableIgnoredIfEnabledByDefault(t *testing.T) {