From ba7a9c5e4ad7f71127bc42bdeef3e6e47d840a62 Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Wed, 26 May 2021 08:45:30 -0400 Subject: [PATCH] Handle product vars *flags props in cc* modules Test: build/bazel/ci/bp2build.sh Bug: 188497994 Change-Id: Ifb379e370075d6a7bc55b82d79c210c31ef377e9 --- bazel/properties.go | 2 +- bp2build/cc_library_static_conversion_test.go | 34 +++++++++++++++++++ bp2build/cc_object_conversion_test.go | 2 +- cc/bp2build.go | 29 ++++++++++------ cc/object.go | 22 +----------- 5 files changed, 56 insertions(+), 33 deletions(-) diff --git a/bazel/properties.go b/bazel/properties.go index b9d6ead59..4cae21e42 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -993,6 +993,6 @@ func TryVariableSubstitutions(slice []string, productVariable string) ([]string, // TryVariableSubstitution, replace string substitution formatting within s with Starlark // string.format compatible tag for productVariable. func TryVariableSubstitution(s string, productVariable string) (string, bool) { - sub := productVariableSubstitutionPattern.ReplaceAllString(s, "{"+productVariable+"}") + sub := productVariableSubstitutionPattern.ReplaceAllString(s, "$("+productVariable+")") return sub, s != sub } diff --git a/bp2build/cc_library_static_conversion_test.go b/bp2build/cc_library_static_conversion_test.go index 770925466..0257b28f9 100644 --- a/bp2build/cc_library_static_conversion_test.go +++ b/bp2build/cc_library_static_conversion_test.go @@ -1412,3 +1412,37 @@ cc_library_static { )`}, }) } + +func TestCcLibraryStaticProductVariableStringReplacement(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: { + platform_sdk_version: { + asflags: ["-DPLATFORM_SDK_VERSION=%d"], + }, + }, +} `, + expectedBazelTargets: []string{`cc_library_static( + name = "foo_static", + asflags = select({ + "//build/bazel/product_variables:platform_sdk_version": ["-DPLATFORM_SDK_VERSION=$(Platform_sdk_version)"], + "//conditions:default": [], + }), + copts = [ + "-I.", + "-I$(BINDIR)/.", + ], + linkstatic = True, + srcs_c = ["common.c"], +)`}, + }) +} diff --git a/bp2build/cc_object_conversion_test.go b/bp2build/cc_object_conversion_test.go index f7e94c28c..57f75eabb 100644 --- a/bp2build/cc_object_conversion_test.go +++ b/bp2build/cc_object_conversion_test.go @@ -210,7 +210,7 @@ func TestCcObjectProductVariable(t *testing.T) { expectedBazelTargets: []string{`cc_object( name = "foo", asflags = select({ - "//build/bazel/product_variables:platform_sdk_version": ["-DPLATFORM_SDK_VERSION={Platform_sdk_version}"], + "//build/bazel/product_variables:platform_sdk_version": ["-DPLATFORM_SDK_VERSION=$(Platform_sdk_version)"], "//conditions:default": [], }), copts = ["-fno-addrsig"], diff --git a/cc/bp2build.go b/cc/bp2build.go index 5d024f865..79e72ac16 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -19,6 +19,8 @@ import ( "android/soong/android" "android/soong/bazel" + + "github.com/google/blueprint/proptools" ) // bp2build functions and helpers for converting cc_* modules to Bazel. @@ -444,18 +446,25 @@ func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Modul } } + productVarPropNameToAttribute := map[string]*bazel.StringListAttribute{ + "Cflags": &copts, + "Asflags": &asFlags, + "CppFlags": &cppFlags, + } 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") + for propName, attr := range productVarPropNameToAttribute { + if props, exists := productVariableProps[propName]; exists { + for _, prop := range props { + flags, ok := prop.Property.([]string) + if !ok { + ctx.ModuleErrorf("Could not convert product variable %s property", proptools.PropertyNameForField(propName)) + } + newFlags, _ := bazel.TryVariableSubstitutions(flags, prop.ProductConfigVariable) + attr.ProductValues = append(attr.ProductValues, bazel.ProductVariableValues{ + ProductVariable: prop.ProductConfigVariable, + Values: newFlags, + }) } - newFlags, _ := bazel.TryVariableSubstitutions(flags, prop.ProductConfigVariable) - copts.ProductValues = append(copts.ProductValues, bazel.ProductVariableValues{ - ProductVariable: prop.ProductConfigVariable, - Values: newFlags, - }) } } diff --git a/cc/object.go b/cc/object.go index cd711617b..39fc43dca 100644 --- a/cc/object.go +++ b/cc/object.go @@ -157,8 +157,6 @@ func ObjectBp2Build(ctx android.TopDownMutatorContext) { // Set arch-specific configurable attributes compilerAttrs := bp2BuildParseCompilerProps(ctx, m) - var asFlags bazel.StringListAttribute - var deps bazel.LabelListAttribute for _, props := range m.linker.linkerProps() { if objectLinkerProps, ok := props.(*ObjectLinkerProperties); ok { @@ -167,24 +165,6 @@ func ObjectBp2Build(ctx android.TopDownMutatorContext) { } } - productVariableProps := android.ProductVariableProperties(ctx) - if props, exists := productVariableProps["Asflags"]; exists { - // TODO(b/183595873): consider deduplicating handling of product variable properties - for _, prop := range props { - flags, ok := prop.Property.([]string) - if !ok { - ctx.ModuleErrorf("Could not convert product variable asflag property") - return - } - 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 - // Don't split cc_object srcs across languages. Doing so would add complexity, // and this isn't typically done for cc_object. srcs := compilerAttrs.srcs @@ -195,7 +175,7 @@ func ObjectBp2Build(ctx android.TopDownMutatorContext) { Srcs: srcs, Deps: deps, Copts: compilerAttrs.copts, - Asflags: asFlags, + Asflags: compilerAttrs.asFlags, } props := bazel.BazelTargetModuleProperties{