From 84817de033adc58708e0a39d5076abd8b5c2a212 Mon Sep 17 00:00:00 2001 From: Jingwen Chen Date: Wed, 17 Nov 2021 10:57:35 +0000 Subject: [PATCH] Add support for merging defaults soong_config_module_types into bp2build select statements. This supports defaults from the same or different namespaces, and transitively defaults as well. Test: soong unit tests Test: CI Change-Id: I99435bacfcfbfe20ad753b8021a1779531d7595a --- android/bazel.go | 20 +- android/defaults.go | 50 ++++ android/soong_config_modules.go | 2 +- android/variable.go | 61 ++-- ...oong_config_module_type_conversion_test.go | 266 ++++++++++++++++++ 5 files changed, 371 insertions(+), 28 deletions(-) diff --git a/android/bazel.go b/android/bazel.go index bf214a541..32a55eefc 100644 --- a/android/bazel.go +++ b/android/bazel.go @@ -33,10 +33,10 @@ type properties struct { } // namespacedVariableProperties is a map from a string representing a Soong -// config variable namespace, like "android" or "vendor_name" to a struct -// pointer representing the soong_config_variables property of a module created -// by a soong_config_module_type or soong_config_module_type_import. -type namespacedVariableProperties map[string]interface{} +// config variable namespace, like "android" or "vendor_name" to a slice of +// pointer to a struct containing a single field called Soong_config_variables +// whose value mirrors the structure in the Blueprint file. +type namespacedVariableProperties map[string][]interface{} // BazelModuleBase contains the property structs with metadata for modules which can be converted to // Bazel. @@ -68,11 +68,19 @@ type Bazelable interface { convertWithBp2build(ctx BazelConversionContext, module blueprint.Module) bool GetBazelBuildFileContents(c Config, path, name string) (string, error) - // For namespaced config variable support + // namespacedVariableProps is a map from a soong config variable namespace + // (e.g. acme, android) to a map of interfaces{}, which are really + // reflect.Struct pointers, representing the value of the + // soong_config_variables property of a module. The struct pointer is the + // one with the single member called Soong_config_variables, which itself is + // a struct containing fields for each supported feature in that namespace. + // + // The reason for using an slice of interface{} is to support defaults + // propagation of the struct pointers. namespacedVariableProps() namespacedVariableProperties setNamespacedVariableProps(props namespacedVariableProperties) BaseModuleType() string - SetBaseModuleType(string) + SetBaseModuleType(baseModuleType string) } // BazelModule is a lightweight wrapper interface around Module for Bazel-convertible modules. diff --git a/android/defaults.go b/android/defaults.go index be80cf10f..9046002cc 100644 --- a/android/defaults.go +++ b/android/defaults.go @@ -213,10 +213,60 @@ func InitDefaultsModule(module DefaultsModule) { var _ Defaults = (*DefaultsModuleBase)(nil) +// applyNamespacedVariableDefaults only runs in bp2build mode for +// defaultable/defaults modules. Its purpose is to merge namespaced product +// variable props from defaults deps, even if those defaults are custom module +// types created from soong_config_module_type, e.g. one that's wrapping a +// cc_defaults or java_defaults. +func applyNamespacedVariableDefaults(defaultDep Defaults, ctx TopDownMutatorContext) { + var dep, b Bazelable + + dep, ok := defaultDep.(Bazelable) + if !ok { + if depMod, ok := defaultDep.(Module); ok { + // Track that this dependency hasn't been converted to bp2build yet. + ctx.AddUnconvertedBp2buildDep(depMod.Name()) + return + } else { + panic("Expected default dep to be a Module.") + } + } + + b, ok = ctx.Module().(Bazelable) + if !ok { + return + } + + // namespacedVariableProps is a map from namespaces (e.g. acme, android, + // vendor_foo) to a slice of soong_config_variable struct pointers, + // containing properties for that particular module. + src := dep.namespacedVariableProps() + dst := b.namespacedVariableProps() + if dst == nil { + dst = make(namespacedVariableProperties) + } + + // Propagate all soong_config_variable structs from the dep. We'll merge the + // actual property values later in variable.go. + for namespace := range src { + if dst[namespace] == nil { + dst[namespace] = []interface{}{} + } + for _, i := range src[namespace] { + dst[namespace] = append(dst[namespace], i) + } + } + + b.setNamespacedVariableProps(dst) +} + func (defaultable *DefaultableModuleBase) applyDefaults(ctx TopDownMutatorContext, defaultsList []Defaults) { for _, defaults := range defaultsList { + if ctx.Config().runningAsBp2Build { + applyNamespacedVariableDefaults(defaults, ctx) + } for _, prop := range defaultable.defaultableProperties { if prop == defaultable.defaultableVariableProperties { defaultable.applyDefaultVariableProperties(ctx, defaults, prop) diff --git a/android/soong_config_modules.go b/android/soong_config_modules.go index 065440d1f..dcf2223ef 100644 --- a/android/soong_config_modules.go +++ b/android/soong_config_modules.go @@ -434,7 +434,7 @@ func configModuleFactory(factory blueprint.ModuleFactory, moduleType *soongconfi // Instead of applying all properties, keep the entire conditionalProps struct as // part of the custom module so dependent modules can create the selects accordingly m.setNamespacedVariableProps(namespacedVariableProperties{ - moduleType.ConfigNamespace: conditionalProps.Interface(), + moduleType.ConfigNamespace: []interface{}{conditionalProps.Interface()}, }) } }) diff --git a/android/variable.go b/android/variable.go index 89cd59ef2..20b268eab 100644 --- a/android/variable.go +++ b/android/variable.go @@ -640,13 +640,15 @@ func ProductVariableProperties(ctx BazelConversionPathContext) ProductConfigProp } if m, ok := module.(Bazelable); ok && m.namespacedVariableProps() != nil { - for namespace, namespacedVariableProp := range m.namespacedVariableProps() { - productVariableValues( - soongconfig.SoongConfigProperty, - namespacedVariableProp, - namespace, - "", - &productConfigProperties) + for namespace, namespacedVariableProps := range m.namespacedVariableProps() { + for _, namespacedVariableProp := range namespacedVariableProps { + productVariableValues( + soongconfig.SoongConfigProperty, + namespacedVariableProp, + namespace, + "", + &productConfigProperties) + } } } @@ -665,7 +667,19 @@ func (p *ProductConfigProperties) AddProductConfigProperty( FullConfig: config, // e.g. size, feature1-x86, size__conditions_default } - (*p)[propertyName][productConfigProp] = property + if existing, ok := (*p)[propertyName][productConfigProp]; ok && namespace != "" { + switch dst := existing.(type) { + case []string: + if src, ok := property.([]string); ok { + dst = append(dst, src...) + (*p)[propertyName][productConfigProp] = dst + } + default: + // TODO(jingwen): Add support for more types. + } + } else { + (*p)[propertyName][productConfigProp] = property + } } var ( @@ -701,19 +715,10 @@ func maybeExtractConfigVarProp(v reflect.Value) (reflect.Value, bool) { return v, true } -// productVariableValues uses reflection to convert a property struct for -// product_variables and soong_config_variables to structs that can be generated -// as select statements. -func productVariableValues( - fieldName string, variableProps interface{}, namespace, suffix string, productConfigProperties *ProductConfigProperties) { - if suffix != "" { - suffix = "-" + suffix - } - - // variableValues represent the product_variables or soong_config_variables - // struct. - variableValues := reflect.ValueOf(variableProps).Elem().FieldByName(fieldName) - +func (productConfigProperties *ProductConfigProperties) AddProductConfigProperties(namespace, suffix string, variableValues reflect.Value) { + // variableValues can either be a product_variables or + // soong_config_variables struct. + // // Example of product_variables: // // product_variables: { @@ -834,6 +839,20 @@ func productVariableValues( } } +// productVariableValues uses reflection to convert a property struct for +// product_variables and soong_config_variables to structs that can be generated +// as select statements. +func productVariableValues( + fieldName string, variableProps interface{}, namespace, suffix string, productConfigProperties *ProductConfigProperties) { + if suffix != "" { + suffix = "-" + suffix + } + + // variableValues represent the product_variables or soong_config_variables struct. + variableValues := reflect.ValueOf(variableProps).Elem().FieldByName(fieldName) + productConfigProperties.AddProductConfigProperties(namespace, suffix, variableValues) +} + func VariableMutator(mctx BottomUpMutatorContext) { var module Module var ok bool diff --git a/bp2build/soong_config_module_type_conversion_test.go b/bp2build/soong_config_module_type_conversion_test.go index 2e6f54c8e..5669a8383 100644 --- a/bp2build/soong_config_module_type_conversion_test.go +++ b/bp2build/soong_config_module_type_conversion_test.go @@ -324,3 +324,269 @@ cc_library_static { name: "soc_default_static_dep", bazel_module: { bp2build_ava local_includes = ["."], )`}}) } + +func TestSoongConfigModuleType_Defaults_SingleNamespace(t *testing.T) { + bp := ` +soong_config_module_type { + name: "vendor_foo_cc_defaults", + module_type: "cc_defaults", + config_namespace: "vendor_foo", + bool_variables: ["feature"], + properties: ["cflags", "cppflags"], + bazel_module: { bp2build_available: true }, +} + +vendor_foo_cc_defaults { + name: "foo_defaults_1", + soong_config_variables: { + feature: { + cflags: ["-cflag_feature_1"], + conditions_default: { + cflags: ["-cflag_default_1"], + }, + }, + }, +} + +vendor_foo_cc_defaults { + name: "foo_defaults_2", + defaults: ["foo_defaults_1"], + soong_config_variables: { + feature: { + cflags: ["-cflag_feature_2"], + conditions_default: { + cflags: ["-cflag_default_2"], + }, + }, + }, +} + +cc_library_static { + name: "lib", + defaults: ["foo_defaults_2"], + bazel_module: { bp2build_available: true }, +} +` + + runSoongConfigModuleTypeTest(t, bp2buildTestCase{ + description: "soong config variables - defaults with a single namespace", + moduleTypeUnderTest: "cc_library_static", + moduleTypeUnderTestFactory: cc.LibraryStaticFactory, + moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryStaticBp2Build, + blueprint: bp, + expectedBazelTargets: []string{`cc_library_static( + name = "lib", + copts = select({ + "//build/bazel/product_variables:vendor_foo__feature__enabled": [ + "-cflag_feature_2", + "-cflag_feature_1", + ], + "//conditions:default": [ + "-cflag_default_2", + "-cflag_default_1", + ], + }), + local_includes = ["."], +)`}}) +} + +func TestSoongConfigModuleType_MultipleDefaults_SingleNamespace(t *testing.T) { + bp := ` +soong_config_module_type { + name: "foo_cc_defaults", + module_type: "cc_defaults", + config_namespace: "acme", + bool_variables: ["feature"], + properties: ["cflags"], + bazel_module: { bp2build_available: true }, +} + +soong_config_module_type { + name: "bar_cc_defaults", + module_type: "cc_defaults", + config_namespace: "acme", + bool_variables: ["feature"], + properties: ["cflags", "asflags"], + bazel_module: { bp2build_available: true }, +} + +foo_cc_defaults { + name: "foo_defaults", + soong_config_variables: { + feature: { + cflags: ["-cflag_foo"], + conditions_default: { + cflags: ["-cflag_default_foo"], + }, + }, + }, +} + +bar_cc_defaults { + name: "bar_defaults", + srcs: ["file.S"], + soong_config_variables: { + feature: { + cflags: ["-cflag_bar"], + asflags: ["-asflag_bar"], + conditions_default: { + asflags: ["-asflag_default_bar"], + cflags: ["-cflag_default_bar"], + }, + }, + }, +} + +cc_library_static { + name: "lib", + defaults: ["foo_defaults", "bar_defaults"], + bazel_module: { bp2build_available: true }, +} + +cc_library_static { + name: "lib2", + defaults: ["bar_defaults", "foo_defaults"], + bazel_module: { bp2build_available: true }, +} +` + + runSoongConfigModuleTypeTest(t, bp2buildTestCase{ + description: "soong config variables - multiple defaults with a single namespace", + moduleTypeUnderTest: "cc_library_static", + moduleTypeUnderTestFactory: cc.LibraryStaticFactory, + moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryStaticBp2Build, + blueprint: bp, + expectedBazelTargets: []string{`cc_library_static( + name = "lib", + asflags = select({ + "//build/bazel/product_variables:acme__feature__enabled": ["-asflag_bar"], + "//conditions:default": ["-asflag_default_bar"], + }), + copts = select({ + "//build/bazel/product_variables:acme__feature__enabled": [ + "-cflag_foo", + "-cflag_bar", + ], + "//conditions:default": [ + "-cflag_default_foo", + "-cflag_default_bar", + ], + }), + local_includes = ["."], + srcs_as = ["file.S"], +)`, + `cc_library_static( + name = "lib2", + asflags = select({ + "//build/bazel/product_variables:acme__feature__enabled": ["-asflag_bar"], + "//conditions:default": ["-asflag_default_bar"], + }), + copts = select({ + "//build/bazel/product_variables:acme__feature__enabled": [ + "-cflag_bar", + "-cflag_foo", + ], + "//conditions:default": [ + "-cflag_default_bar", + "-cflag_default_foo", + ], + }), + local_includes = ["."], + srcs_as = ["file.S"], +)`}}) +} + +func TestSoongConfigModuleType_Defaults_MultipleNamespaces(t *testing.T) { + bp := ` +soong_config_module_type { + name: "vendor_foo_cc_defaults", + module_type: "cc_defaults", + config_namespace: "vendor_foo", + bool_variables: ["feature"], + properties: ["cflags"], + bazel_module: { bp2build_available: true }, +} + +soong_config_module_type { + name: "vendor_bar_cc_defaults", + module_type: "cc_defaults", + config_namespace: "vendor_bar", + bool_variables: ["feature"], + properties: ["cflags"], + bazel_module: { bp2build_available: true }, +} + +soong_config_module_type { + name: "vendor_qux_cc_defaults", + module_type: "cc_defaults", + config_namespace: "vendor_qux", + bool_variables: ["feature"], + properties: ["cflags"], + bazel_module: { bp2build_available: true }, +} + +vendor_foo_cc_defaults { + name: "foo_defaults", + soong_config_variables: { + feature: { + cflags: ["-DVENDOR_FOO_FEATURE"], + conditions_default: { + cflags: ["-DVENDOR_FOO_DEFAULT"], + }, + }, + }, +} + +vendor_bar_cc_defaults { + name: "bar_defaults", + soong_config_variables: { + feature: { + cflags: ["-DVENDOR_BAR_FEATURE"], + conditions_default: { + cflags: ["-DVENDOR_BAR_DEFAULT"], + }, + }, + }, +} + +vendor_qux_cc_defaults { + name: "qux_defaults", + defaults: ["bar_defaults"], + soong_config_variables: { + feature: { + cflags: ["-DVENDOR_QUX_FEATURE"], + conditions_default: { + cflags: ["-DVENDOR_QUX_DEFAULT"], + }, + }, + }, +} + +cc_library_static { + name: "lib", + defaults: ["foo_defaults", "qux_defaults"], + bazel_module: { bp2build_available: true }, +} +` + + runSoongConfigModuleTypeTest(t, bp2buildTestCase{ + description: "soong config variables - defaults with multiple namespaces", + moduleTypeUnderTest: "cc_library_static", + moduleTypeUnderTestFactory: cc.LibraryStaticFactory, + moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryStaticBp2Build, + blueprint: bp, + expectedBazelTargets: []string{`cc_library_static( + name = "lib", + copts = select({ + "//build/bazel/product_variables:vendor_bar__feature__enabled": ["-DVENDOR_BAR_FEATURE"], + "//conditions:default": ["-DVENDOR_BAR_DEFAULT"], + }) + select({ + "//build/bazel/product_variables:vendor_foo__feature__enabled": ["-DVENDOR_FOO_FEATURE"], + "//conditions:default": ["-DVENDOR_FOO_DEFAULT"], + }) + select({ + "//build/bazel/product_variables:vendor_qux__feature__enabled": ["-DVENDOR_QUX_FEATURE"], + "//conditions:default": ["-DVENDOR_QUX_DEFAULT"], + }), + local_includes = ["."], +)`}}) +}