diff --git a/bp2build/cc_binary_conversion_test.go b/bp2build/cc_binary_conversion_test.go index c23779e24..1b60f2baf 100644 --- a/bp2build/cc_binary_conversion_test.go +++ b/bp2build/cc_binary_conversion_test.go @@ -221,6 +221,38 @@ func TestCcBinaryVersionScript(t *testing.T) { }) } +func TestCcBinaryLdflagsSplitBySpaceExceptSoongAdded(t *testing.T) { + runCcBinaryTests(t, ccBinaryBp2buildTestCase{ + description: "ldflags are split by spaces except for the ones added by soong (version script and dynamic list)", + blueprint: ` +{rule_name} { + name: "foo", + ldflags: [ + "--nospace_flag", + "-z spaceflag", + ], + version_script: "version_script", + dynamic_list: "dynamic.list", + include_build_directory: false, +} +`, + targets: []testBazelTarget{ + {"cc_binary", "foo", AttrNameToString{ + "additional_linker_inputs": `[ + "version_script", + "dynamic.list", + ]`, + "linkopts": `[ + "--nospace_flag", + "-z", + "spaceflag", + "-Wl,--version-script,$(location version_script)", + "-Wl,--dynamic-list,$(location dynamic.list)", + ]`, + }}}, + }) +} + func TestCcBinarySplitSrcsByLang(t *testing.T) { runCcHostBinaryTestCase(t, ccBinaryBp2buildTestCase{ description: "split srcs by lang", diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index 1b8e9b457..b7f79878b 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/bp2build/cc_library_conversion_test.go @@ -940,6 +940,46 @@ cc_library { ) } +func TestCcLibraryLdflagsSplitBySpaceExceptSoongAdded(t *testing.T) { + runCcLibraryTestCase(t, Bp2buildTestCase{ + Description: "ldflags are split by spaces except for the ones added by soong (version script and dynamic list)", + ModuleTypeUnderTest: "cc_library", + ModuleTypeUnderTestFactory: cc.LibraryFactory, + Filesystem: map[string]string{ + "version_script": "", + "dynamic.list": "", + }, + Blueprint: ` +cc_library { + name: "foo", + ldflags: [ + "--nospace_flag", + "-z spaceflag", + ], + version_script: "version_script", + dynamic_list: "dynamic.list", + include_build_directory: false, +} +`, + ExpectedBazelTargets: []string{ + MakeBazelTarget("cc_library_static", "foo_bp2build_cc_library_static", AttrNameToString{}), + MakeBazelTarget("cc_library_shared", "foo", AttrNameToString{ + "additional_linker_inputs": `[ + "version_script", + "dynamic.list", + ]`, + "linkopts": `[ + "--nospace_flag", + "-z", + "spaceflag", + "-Wl,--version-script,$(location version_script)", + "-Wl,--dynamic-list,$(location dynamic.list)", + ]`, + }), + }, + }) +} + func TestCcLibrarySharedLibs(t *testing.T) { runCcLibraryTestCase(t, Bp2buildTestCase{ Description: "cc_library shared_libs", diff --git a/bp2build/cc_library_shared_conversion_test.go b/bp2build/cc_library_shared_conversion_test.go index 6253de6c7..b86f60778 100644 --- a/bp2build/cc_library_shared_conversion_test.go +++ b/bp2build/cc_library_shared_conversion_test.go @@ -367,6 +367,42 @@ cc_library_shared { }) } +func TestCcLibraryLdflagsSplitBySpaceSoongAdded(t *testing.T) { + runCcLibrarySharedTestCase(t, Bp2buildTestCase{ + Description: "ldflags are split by spaces except for the ones added by soong (version script and dynamic list)", + Filesystem: map[string]string{ + "version_script": "", + "dynamic.list": "", + }, + Blueprint: ` +cc_library_shared { + name: "foo", + ldflags: [ + "--nospace_flag", + "-z spaceflag", + ], + version_script: "version_script", + dynamic_list: "dynamic.list", + include_build_directory: false, +}`, + ExpectedBazelTargets: []string{ + MakeBazelTarget("cc_library_shared", "foo", AttrNameToString{ + "additional_linker_inputs": `[ + "version_script", + "dynamic.list", + ]`, + "linkopts": `[ + "--nospace_flag", + "-z", + "spaceflag", + "-Wl,--version-script,$(location version_script)", + "-Wl,--dynamic-list,$(location dynamic.list)", + ]`, + }), + }, + }) +} + func TestCcLibrarySharedNoCrtTrue(t *testing.T) { runCcLibrarySharedTestCase(t, Bp2buildTestCase{ Description: "cc_library_shared - nocrt: true emits attribute", diff --git a/cc/bp2build.go b/cc/bp2build.go index 83368a392..868ff0db5 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -174,7 +174,7 @@ func bp2buildParseStaticOrSharedProps(ctx android.BazelConversionPathContext, mo attrs := staticOrSharedAttributes{} setAttrs := func(axis bazel.ConfigurationAxis, config string, props StaticOrSharedProperties) { - attrs.Copts.SetSelectValue(axis, config, parseCommandLineFlags(props.Cflags, true, filterOutStdFlag)) + attrs.Copts.SetSelectValue(axis, config, parseCommandLineFlags(props.Cflags, filterOutStdFlag)) attrs.Srcs.SetSelectValue(axis, config, android.BazelLabelForModuleSrc(ctx, props.Srcs)) attrs.System_dynamic_deps.SetSelectValue(axis, config, bazelLabelForSharedDeps(ctx, props.System_shared_libs)) @@ -365,7 +365,7 @@ func filterOutClangUnknownCflags(flag string) bool { return false } -func parseCommandLineFlags(soongFlags []string, noCoptsTokenization bool, filterOut ...filterOutFn) []string { +func parseCommandLineFlags(soongFlags []string, filterOut ...filterOutFn) []string { var result []string for _, flag := range soongFlags { skipFlag := false @@ -380,15 +380,7 @@ func parseCommandLineFlags(soongFlags []string, noCoptsTokenization bool, filter // Soong's cflags can contain spaces, like `-include header.h`. For // Bazel's copts, split them up to be compatible with the // no_copts_tokenization feature. - if noCoptsTokenization { - result = append(result, strings.Split(flag, " ")...) - } else { - // Soong's Version Script and Dynamic List Properties are added as flags - // to Bazel's linkopts using "($location label)" syntax. - // Splitting on spaces would separate this into two different flags - // "($ location" and "label)" - result = append(result, flag) - } + result = append(result, strings.Split(flag, " ")...) } return result } @@ -422,10 +414,10 @@ func (ca *compilerAttributes) bp2buildForAxisAndConfig(ctx android.BazelConversi // overridden. In Bazel we always allow overriding, via flags; however, this can cause // incompatibilities, so we remove "-std=" flags from Cflag properties while leaving it in other // cases. - ca.copts.SetSelectValue(axis, config, parseCommandLineFlags(props.Cflags, true, filterOutStdFlag, filterOutClangUnknownCflags)) - ca.asFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Asflags, true, nil)) - ca.conlyFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Conlyflags, true, filterOutClangUnknownCflags)) - ca.cppFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Cppflags, true, filterOutClangUnknownCflags)) + ca.copts.SetSelectValue(axis, config, parseCommandLineFlags(props.Cflags, filterOutStdFlag, filterOutClangUnknownCflags)) + ca.asFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Asflags, nil)) + ca.conlyFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Conlyflags, filterOutClangUnknownCflags)) + ca.cppFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Cppflags, filterOutClangUnknownCflags)) ca.rtti.SetSelectValue(axis, config, props.Rtti) } @@ -1031,6 +1023,11 @@ func (la *linkerAttributes) bp2buildForAxisAndConfig(ctx android.BazelConversion axisFeatures = append(axisFeatures, "-static_flag") } } + + // This must happen before the addition of flags for Version Script and + // Dynamic List, as these flags must be split on spaces and those must not + linkerFlags = parseCommandLineFlags(linkerFlags, filterOutClangUnknownCflags) + additionalLinkerInputs := bazel.LabelList{} if props.Version_script != nil { label := android.BazelLabelForModuleSrcSingle(ctx, *props.Version_script) @@ -1045,7 +1042,7 @@ func (la *linkerAttributes) bp2buildForAxisAndConfig(ctx android.BazelConversion } la.additionalLinkerInputs.SetSelectValue(axis, config, additionalLinkerInputs) - la.linkopts.SetSelectValue(axis, config, parseCommandLineFlags(linkerFlags, false, filterOutClangUnknownCflags)) + la.linkopts.SetSelectValue(axis, config, linkerFlags) la.useLibcrt.SetSelectValue(axis, config, props.libCrt()) // it's very unlikely for nocrt to be arch variant, so bp2build doesn't support it.