From 6fd7b3fee9fdf80126da91dbbb78e5d0d662402b Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Thu, 6 May 2021 13:54:29 -0400 Subject: [PATCH] Handle product config vars in bp2build. Test: bp2build ci & mixed build libc Bug: 183595873 Change-Id: I2d87434ff4df5a24efc5e3e38f087de035228934 --- bazel/properties.go | 43 +++++++++++++- bp2build/cc_library_static_conversion_test.go | 45 ++++++++++++++ bp2build/cc_object_conversion_test.go | 5 +- bp2build/configurability.go | 59 ++++++++++++------- cc/bp2build.go | 15 +++++ cc/object.go | 13 ++-- 6 files changed, 149 insertions(+), 31 deletions(-) diff --git a/bazel/properties.go b/bazel/properties.go index 3e778bb69..640275f6f 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -19,6 +19,7 @@ import ( "path/filepath" "regexp" "sort" + "strings" ) // BazelTargetModuleProperties contain properties and metadata used for @@ -200,6 +201,10 @@ const ( // config variable default key in an Android.bp file, although there's no // integration with Soong config variables (yet). CONDITIONS_DEFAULT = "conditions_default" + + ConditionsDefaultSelectKey = "//conditions:default" + + productVariableBazelPackage = "//build/bazel/product_variables" ) var ( @@ -215,7 +220,7 @@ var ( ARCH_ARM64: "//build/bazel/platforms/arch:arm64", ARCH_X86: "//build/bazel/platforms/arch:x86", ARCH_X86_64: "//build/bazel/platforms/arch:x86_64", - CONDITIONS_DEFAULT: "//conditions:default", // The default condition of as arch select map. + CONDITIONS_DEFAULT: ConditionsDefaultSelectKey, // The default condition of as arch select map. } // A map of target operating systems to the Bazel label of the @@ -227,7 +232,7 @@ var ( OS_LINUX: "//build/bazel/platforms/os:linux", OS_LINUX_BIONIC: "//build/bazel/platforms/os:linux_bionic", OS_WINDOWS: "//build/bazel/platforms/os:windows", - CONDITIONS_DEFAULT: "//conditions:default", // The default condition of an os select map. + CONDITIONS_DEFAULT: ConditionsDefaultSelectKey, // The default condition of an os select map. } ) @@ -435,6 +440,10 @@ type StringListAttribute struct { // are generated in a select statement and appended to the non-os specific // label list Value. OsValues stringListOsValues + + // list of product-variable string list values. Optional. if used, each will generate a select + // statement appended to the label list Value. + ProductValues []ProductVariableValues } // MakeStringListAttribute initializes a StringListAttribute with the non-arch specific value. @@ -466,6 +475,18 @@ type stringListOsValues struct { ConditionsDefault []string } +// Product Variable values for StringListAttribute +type ProductVariableValues struct { + ProductVariable string + + Values []string +} + +// SelectKey returns the appropriate select key for the receiving ProductVariableValues. +func (p ProductVariableValues) SelectKey() string { + return fmt.Sprintf("%s:%s", productVariableBazelPackage, strings.ToLower(p.ProductVariable)) +} + // HasConfigurableValues returns true if the attribute contains // architecture-specific string_list values. func (attrs StringListAttribute) HasConfigurableValues() bool { @@ -480,7 +501,8 @@ func (attrs StringListAttribute) HasConfigurableValues() bool { return true } } - return false + + return len(attrs.ProductValues) > 0 } func (attrs *StringListAttribute) archValuePtrs() map[string]*[]string { @@ -558,6 +580,21 @@ func (attrs *StringListAttribute) Append(other StringListAttribute) { attrs.SetValueForOS(os, this) } + productValues := make(map[string][]string, 0) + for _, pv := range attrs.ProductValues { + productValues[pv.ProductVariable] = pv.Values + } + for _, pv := range other.ProductValues { + productValues[pv.ProductVariable] = append(productValues[pv.ProductVariable], pv.Values...) + } + attrs.ProductValues = make([]ProductVariableValues, 0, len(productValues)) + for pv, vals := range productValues { + attrs.ProductValues = append(attrs.ProductValues, ProductVariableValues{ + ProductVariable: pv, + Values: vals, + }) + } + attrs.Value = append(attrs.Value, other.Value...) } diff --git a/bp2build/cc_library_static_conversion_test.go b/bp2build/cc_library_static_conversion_test.go index 62084a544..44c6ad4d1 100644 --- a/bp2build/cc_library_static_conversion_test.go +++ b/bp2build/cc_library_static_conversion_test.go @@ -1156,3 +1156,48 @@ cc_library_static { )`}, }) } + +func TestCcLibraryStaticProductVariableSelects(t *testing.T) { + runCcLibraryStaticTestCase(t, bp2buildTestCase{ + description: "cc_library_static product variable selects", + moduleTypeUnderTest: "cc_library_static", + moduleTypeUnderTestFactory: cc.LibraryStaticFactory, + moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryStaticBp2Build, + depsMutators: []android.RegisterMutatorFunc{cc.RegisterDepsBp2Build}, + filesystem: map[string]string{}, + blueprint: soongCcLibraryStaticPreamble + ` +cc_library_static { + name: "foo_static", + srcs: ["common.c"], + product_variables: { + malloc_not_svelte: { + cflags: ["-Wmalloc_not_svelte"], + }, + malloc_zero_contents: { + cflags: ["-Wmalloc_zero_contents"], + }, + binder32bit: { + cflags: ["-Wbinder32bit"], + }, + }, +} `, + expectedBazelTargets: []string{`cc_library_static( + name = "foo_static", + copts = [ + "-I.", + "-I$(BINDIR)/.", + ] + select({ + "//build/bazel/product_variables:malloc_not_svelte": ["-Wmalloc_not_svelte"], + "//conditions:default": [], + }) + select({ + "//build/bazel/product_variables:malloc_zero_contents": ["-Wmalloc_zero_contents"], + "//conditions:default": [], + }) + select({ + "//build/bazel/product_variables:binder32bit": ["-Wbinder32bit"], + "//conditions:default": [], + }), + linkstatic = True, + srcs = ["common.c"], +)`}, + }) +} diff --git a/bp2build/cc_object_conversion_test.go b/bp2build/cc_object_conversion_test.go index b69135b6e..d4eeb7cba 100644 --- a/bp2build/cc_object_conversion_test.go +++ b/bp2build/cc_object_conversion_test.go @@ -208,7 +208,10 @@ func TestCcObjectProductVariable(t *testing.T) { `, expectedBazelTargets: []string{`cc_object( name = "foo", - asflags = ["-DPLATFORM_SDK_VERSION={Platform_sdk_version}"], + asflags = select({ + "//build/bazel/product_variables:platform_sdk_version": ["-DPLATFORM_SDK_VERSION={Platform_sdk_version}"], + "//conditions:default": [], + }), copts = ["-fno-addrsig"], )`, }, diff --git a/bp2build/configurability.go b/bp2build/configurability.go index 2b8f6cc2e..3cdc99437 100644 --- a/bp2build/configurability.go +++ b/bp2build/configurability.go @@ -11,26 +11,42 @@ import ( type selects map[string]reflect.Value -func getStringListValues(list bazel.StringListAttribute) (reflect.Value, selects, selects) { +func getStringListValues(list bazel.StringListAttribute) (reflect.Value, []selects) { value := reflect.ValueOf(list.Value) if !list.HasConfigurableValues() { - return value, nil, nil + return value, []selects{} } + selectValues := make([]selects, 0) archSelects := map[string]reflect.Value{} for arch, selectKey := range bazel.PlatformArchMap { archSelects[selectKey] = reflect.ValueOf(list.GetValueForArch(arch)) } + if len(archSelects) > 0 { + selectValues = append(selectValues, archSelects) + } osSelects := map[string]reflect.Value{} for os, selectKey := range bazel.PlatformOsMap { osSelects[selectKey] = reflect.ValueOf(list.GetValueForOS(os)) } + if len(osSelects) > 0 { + selectValues = append(selectValues, osSelects) + } - return value, archSelects, osSelects + for _, pv := range list.ProductValues { + s := make(selects) + if len(pv.Values) > 0 { + s[pv.SelectKey()] = reflect.ValueOf(pv.Values) + s[bazel.ConditionsDefaultSelectKey] = reflect.ValueOf([]string{}) + selectValues = append(selectValues, s) + } + } + + return value, selectValues } -func getLabelValue(label bazel.LabelAttribute) (reflect.Value, selects, selects) { +func getLabelValue(label bazel.LabelAttribute) (reflect.Value, []selects) { var value reflect.Value var archSelects selects @@ -43,13 +59,13 @@ func getLabelValue(label bazel.LabelAttribute) (reflect.Value, selects, selects) value = reflect.ValueOf(label.Value) } - return value, archSelects, nil + return value, []selects{archSelects} } -func getLabelListValues(list bazel.LabelListAttribute) (reflect.Value, selects, selects) { +func getLabelListValues(list bazel.LabelListAttribute) (reflect.Value, []selects) { value := reflect.ValueOf(list.Value.Includes) if !list.HasConfigurableValues() { - return value, nil, nil + return value, []selects{} } archSelects := map[string]reflect.Value{} @@ -62,29 +78,30 @@ func getLabelListValues(list bazel.LabelListAttribute) (reflect.Value, selects, osSelects[selectKey] = reflect.ValueOf(list.GetValueForOS(os).Includes) } - return value, archSelects, osSelects + return value, []selects{archSelects, osSelects} } // prettyPrintAttribute converts an Attribute to its Bazel syntax. May contain // select statements. func prettyPrintAttribute(v bazel.Attribute, indent int) (string, error) { var value reflect.Value - var archSelects, osSelects selects + var configurableAttrs []selects var defaultSelectValue string switch list := v.(type) { case bazel.StringListAttribute: - value, archSelects, osSelects = getStringListValues(list) + value, configurableAttrs = getStringListValues(list) defaultSelectValue = "[]" case bazel.LabelListAttribute: - value, archSelects, osSelects = getLabelListValues(list) + value, configurableAttrs = getLabelListValues(list) defaultSelectValue = "[]" case bazel.LabelAttribute: - value, archSelects, osSelects = getLabelValue(list) + value, configurableAttrs = getLabelValue(list) defaultSelectValue = "None" default: return "", fmt.Errorf("Not a supported Bazel attribute type: %s", v) } + var err error ret := "" if value.Kind() != reflect.Invalid { s, err := prettyPrint(value, indent) @@ -108,13 +125,14 @@ func prettyPrintAttribute(v bazel.Attribute, indent int) (string, error) { return s, nil } - ret, err := appendSelects(archSelects, defaultSelectValue, ret) - if err != nil { - return "", err + for _, configurableAttr := range configurableAttrs { + ret, err = appendSelects(configurableAttr, defaultSelectValue, ret) + if err != nil { + return "", err + } } - ret, err = appendSelects(osSelects, defaultSelectValue, ret) - return ret, err + return ret, nil } // prettyPrintSelectMap converts a map of select keys to reflected Values as a generic way @@ -125,11 +143,10 @@ func prettyPrintSelectMap(selectMap map[string]reflect.Value, defaultValue strin } // addConditionsDefault := false - conditionsDefaultKey := bazel.PlatformArchMap[bazel.CONDITIONS_DEFAULT] var selects string for _, selectKey := range android.SortedStringKeys(selectMap) { - if selectKey == conditionsDefaultKey { + if selectKey == bazel.ConditionsDefaultSelectKey { // Handle default condition later. continue } @@ -159,14 +176,14 @@ func prettyPrintSelectMap(selectMap map[string]reflect.Value, defaultValue strin ret += selects // Handle the default condition - s, err := prettyPrintSelectEntry(selectMap[conditionsDefaultKey], conditionsDefaultKey, indent) + s, err := prettyPrintSelectEntry(selectMap[bazel.ConditionsDefaultSelectKey], bazel.ConditionsDefaultSelectKey, indent) if err != nil { return "", err } if s == "" { // Print an explicit empty list (the default value) even if the value is // empty, to avoid errors about not finding a configuration that matches. - ret += fmt.Sprintf("%s\"%s\": %s,\n", makeIndent(indent+1), "//conditions:default", defaultValue) + ret += fmt.Sprintf("%s\"%s\": %s,\n", makeIndent(indent+1), bazel.ConditionsDefaultSelectKey, defaultValue) } else { // Print the custom default value. ret += s diff --git a/cc/bp2build.go b/cc/bp2build.go index 95a3fe157..0c827c5ed 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -318,6 +318,21 @@ func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Modul } } + productVariableProps := android.ProductVariableProperties(ctx) + if props, exists := productVariableProps["Cflags"]; exists { + for _, prop := range props { + flags, ok := prop.Property.([]string) + if !ok { + ctx.ModuleErrorf("Could not convert product variable cflag property") + } + newFlags, _ := bazel.TryVariableSubstitutions(flags, prop.ProductConfigVariable) + copts.ProductValues = append(copts.ProductValues, bazel.ProductVariableValues{ + ProductVariable: prop.ProductConfigVariable, + Values: newFlags, + }) + } + } + return compilerAttributes{ srcs: srcs, copts: copts, diff --git a/cc/object.go b/cc/object.go index d8f1ababa..704cb697d 100644 --- a/cc/object.go +++ b/cc/object.go @@ -116,7 +116,7 @@ type bazelObjectAttributes struct { Hdrs bazel.LabelListAttribute Deps bazel.LabelListAttribute Copts bazel.StringListAttribute - Asflags []string + Asflags bazel.StringListAttribute } type bazelObject struct { @@ -157,7 +157,7 @@ func ObjectBp2Build(ctx android.TopDownMutatorContext) { // Set arch-specific configurable attributes compilerAttrs := bp2BuildParseCompilerProps(ctx, m) - var asFlags []string + var asFlags bazel.StringListAttribute var deps bazel.LabelListAttribute for _, props := range m.linker.linkerProps() { @@ -176,10 +176,11 @@ func ObjectBp2Build(ctx android.TopDownMutatorContext) { ctx.ModuleErrorf("Could not convert product variable asflag property") return } - // TODO(b/183595873) handle other product variable usages -- as selects? - if newFlags, subbed := bazel.TryVariableSubstitutions(flags, prop.ProductConfigVariable); subbed { - asFlags = append(asFlags, newFlags...) - } + newFlags, _ := bazel.TryVariableSubstitutions(flags, prop.ProductConfigVariable) + asFlags.ProductValues = append(asFlags.ProductValues, bazel.ProductVariableValues{ + ProductVariable: prop.ProductConfigVariable, + Values: newFlags, + }) } } // TODO(b/183595872) warn/error if we're not handling product variables