From 46fb7aba4dd84a1960a94a53b6b561197f722ae2 Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Wed, 1 Dec 2021 10:09:34 -0500 Subject: [PATCH] Support empty strings in bp2build Previously, could not set an empty string as a value of an attribute; however, this is necessary in some cases. To not unnecessarily create an empty string, use string pointers for attributes rather than strings. Test: go test bp2build tests Change-Id: I03b3a3567452d455246d22d81f86c317d06b7c39 --- apex/apex.go | 6 +++--- bp2build/build_conversion.go | 6 +++--- bp2build/build_conversion_test.go | 28 ++++++++++++++++++++++++---- bp2build/testing.go | 14 +++++++------- cc/bp2build.go | 10 +++++++++- python/binary.go | 12 ++++++------ python/library.go | 9 +++++---- 7 files changed, 57 insertions(+), 28 deletions(-) diff --git a/apex/apex.go b/apex/apex.go index 9a299dee5..01da1daab 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -3186,7 +3186,7 @@ type bazelApexBundleAttributes struct { File_contexts bazel.LabelAttribute Key bazel.LabelAttribute Certificate bazel.LabelAttribute - Min_sdk_version string + Min_sdk_version *string Updatable bazel.BoolAttribute Installable bazel.BoolAttribute Native_shared_libs bazel.LabelListAttribute @@ -3226,9 +3226,9 @@ func apexBundleBp2BuildInternal(ctx android.TopDownMutatorContext, module *apexB fileContextsLabelAttribute.SetValue(android.BazelLabelForModuleDepSingle(ctx, *module.properties.File_contexts)) } - var minSdkVersion string + var minSdkVersion *string if module.properties.Min_sdk_version != nil { - minSdkVersion = *module.properties.Min_sdk_version + minSdkVersion = module.properties.Min_sdk_version } var keyLabelAttribute bazel.LabelAttribute diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index 10ee58294..eb60cd16c 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -698,9 +698,9 @@ func isZero(value reflect.Value) bool { } else { return true } - // Always print bools, if you want a bool attribute to be able to take the default value, use a - // bool pointer instead - case reflect.Bool: + // Always print bool/strings, if you want a bool/string attribute to be able to take the default value, use a + // pointer instead + case reflect.Bool, reflect.String: return false default: if !value.IsValid() { diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go index 983604b93..95a26a963 100644 --- a/bp2build/build_conversion_test.go +++ b/bp2build/build_conversion_test.go @@ -41,6 +41,7 @@ func TestGenerateSoongModuleTargets(t *testing.T) { soong_module_deps = [ ], bool_prop = False, + string_prop = "", )`, }, { @@ -58,6 +59,7 @@ func TestGenerateSoongModuleTargets(t *testing.T) { soong_module_deps = [ ], bool_prop = True, + string_prop = "", )`, }, { @@ -76,6 +78,7 @@ func TestGenerateSoongModuleTargets(t *testing.T) { ], bool_prop = False, owner = "a_string_with\"quotes\"_and_\\backslashes\\\\", + string_prop = "", )`, }, { @@ -94,6 +97,7 @@ func TestGenerateSoongModuleTargets(t *testing.T) { ], bool_prop = False, required = ["bar"], + string_prop = "", )`, }, { @@ -111,6 +115,7 @@ func TestGenerateSoongModuleTargets(t *testing.T) { soong_module_deps = [ ], bool_prop = False, + string_prop = "", target_required = [ "qux", "bazqux", @@ -147,6 +152,7 @@ func TestGenerateSoongModuleTargets(t *testing.T) { "tag": ".bar", "targets": ["goal_bar"], }], + string_prop = "", )`, }, { @@ -179,6 +185,7 @@ func TestGenerateSoongModuleTargets(t *testing.T) { }], owner = "custom_owner", required = ["bar"], + string_prop = "", target_required = [ "qux", "bazqux", @@ -222,12 +229,25 @@ func TestGenerateSoongModuleTargets(t *testing.T) { func TestGenerateBazelTargetModules(t *testing.T) { testCases := []bp2buildTestCase{ + { + description: "string ptr props", + blueprint: `custom { + name: "foo", + string_ptr_prop: "", + bazel_module: { bp2build_available: true }, +}`, + expectedBazelTargets: []string{ + makeBazelTarget("custom", "foo", attrNameToString{ + "string_ptr_prop": `""`, + }), + }, + }, { description: "string props", blueprint: `custom { name: "foo", string_list_prop: ["a", "b"], - string_prop: "a", + string_ptr_prop: "a", bazel_module: { bp2build_available: true }, }`, expectedBazelTargets: []string{ @@ -236,7 +256,7 @@ func TestGenerateBazelTargetModules(t *testing.T) { "a", "b", ]`, - "string_prop": `"a"`, + "string_ptr_prop": `"a"`, }), }, }, @@ -245,7 +265,7 @@ func TestGenerateBazelTargetModules(t *testing.T) { blueprint: `custom { name: "foo", string_list_prop: ["\t", "\n"], - string_prop: "a\t\n\r", + string_ptr_prop: "a\t\n\r", bazel_module: { bp2build_available: true }, }`, expectedBazelTargets: []string{ @@ -254,7 +274,7 @@ func TestGenerateBazelTargetModules(t *testing.T) { "\t", "\n", ]`, - "string_prop": `"a\t\n\r"`, + "string_ptr_prop": `"a\t\n\r"`, }), }, }, diff --git a/bp2build/testing.go b/bp2build/testing.go index daa9c22db..cd84519f2 100644 --- a/bp2build/testing.go +++ b/bp2build/testing.go @@ -149,15 +149,15 @@ func runBp2BuildTestCase(t *testing.T, registerModuleTypes func(ctx android.Regi } type nestedProps struct { - Nested_prop string + Nested_prop *string } type EmbeddedProps struct { - Embedded_prop string + Embedded_prop *string } type OtherEmbeddedProps struct { - Other_embedded_prop string + Other_embedded_prop *string } type customProps struct { @@ -262,17 +262,17 @@ func customDefaultsModuleFactory() android.Module { } type EmbeddedAttr struct { - Embedded_attr string + Embedded_attr *string } type OtherEmbeddedAttr struct { - Other_embedded_attr string + Other_embedded_attr *string } type customBazelModuleAttributes struct { EmbeddedAttr *OtherEmbeddedAttr - String_prop string + String_ptr_prop *string String_list_prop []string Arch_paths bazel.LabelListAttribute } @@ -296,7 +296,7 @@ func customBp2BuildMutator(ctx android.TopDownMutatorContext) { paths.ResolveExcludes() attrs := &customBazelModuleAttributes{ - String_prop: m.props.String_prop, + String_ptr_prop: m.props.String_ptr_prop, String_list_prop: m.props.String_list_prop, Arch_paths: paths, } diff --git a/cc/bp2build.go b/cc/bp2build.go index eabd814c6..888c3bacb 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -388,7 +388,15 @@ func bp2buildResolveCppStdValue(c_std *string, cpp_std *string, gnu_extensions * } cStdVal, cppStdVal = maybeReplaceGnuToC(gnu_extensions, cStdVal, cppStdVal) - return &cStdVal, &cppStdVal + var c_std_prop, cpp_std_prop *string + if cStdVal != "" { + c_std_prop = &cStdVal + } + if cppStdVal != "" { + cpp_std_prop = &cppStdVal + } + + return c_std_prop, cpp_std_prop } // bp2BuildParseCompilerProps returns copts, srcs and hdrs and other attributes. diff --git a/python/binary.go b/python/binary.go index bf6167c3c..af02de63d 100644 --- a/python/binary.go +++ b/python/binary.go @@ -35,10 +35,10 @@ func registerPythonBinaryComponents(ctx android.RegistrationContext) { } type bazelPythonBinaryAttributes struct { - Main string + Main *string Srcs bazel.LabelListAttribute Deps bazel.LabelListAttribute - Python_version string + Python_version *string } func PythonBinaryBp2Build(ctx android.TopDownMutatorContext) { @@ -52,12 +52,12 @@ func PythonBinaryBp2Build(ctx android.TopDownMutatorContext) { return } - var main string + var main *string for _, propIntf := range m.GetProperties() { if props, ok := propIntf.(*BinaryProperties); ok { // main is optional. if props.Main != nil { - main = *props.Main + main = props.Main break } } @@ -69,13 +69,13 @@ func PythonBinaryBp2Build(ctx android.TopDownMutatorContext) { // under Bionic. py3Enabled := proptools.BoolDefault(m.properties.Version.Py3.Enabled, false) py2Enabled := proptools.BoolDefault(m.properties.Version.Py2.Enabled, false) - var python_version string + var python_version *string if py3Enabled && py2Enabled { panic(fmt.Errorf( "error for '%s' module: bp2build's python_binary_host converter does not support "+ "converting a module that is enabled for both Python 2 and 3 at the same time.", m.Name())) } else if py2Enabled { - python_version = "PY2" + python_version = &pyVersion2 } else { // do nothing, since python_version defaults to PY3. } diff --git a/python/library.go b/python/library.go index d136a4efb..b9201177d 100644 --- a/python/library.go +++ b/python/library.go @@ -21,6 +21,7 @@ import ( "android/soong/android" "android/soong/bazel" + "github.com/google/blueprint/proptools" ) @@ -46,7 +47,7 @@ func PythonLibraryHostFactory() android.Module { type bazelPythonLibraryAttributes struct { Srcs bazel.LabelListAttribute Deps bazel.LabelListAttribute - Srcs_version string + Srcs_version *string } func PythonLibraryHostBp2Build(ctx android.TopDownMutatorContext) { @@ -74,11 +75,11 @@ func pythonLibBp2Build(ctx android.TopDownMutatorContext, modType string) { // Bionic. py3Enabled := proptools.BoolDefault(m.properties.Version.Py3.Enabled, true) py2Enabled := proptools.BoolDefault(m.properties.Version.Py2.Enabled, false) - var python_version string + var python_version *string if py2Enabled && !py3Enabled { - python_version = "PY2" + python_version = &pyVersion2 } else if !py2Enabled && py3Enabled { - python_version = "PY3" + python_version = &pyVersion3 } else if !py2Enabled && !py3Enabled { panic(fmt.Errorf( "error for '%s' module: bp2build's %s converter doesn't understand having "+