Merge "Split ldflags in bp2build"

bp2build/cc_binary_conversion_test.go

@@ -228,6 +228,38 @@ func TestCcBinaryVersionScriptAndDynamicList(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",

bp2build/cc_library_conversion_test.go

@@ -961,6 +961,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",

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",

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)
-		}
 	}
 	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.copts.SetSelectValue(axis, config, parseCommandLineFlags(props.Cflags, filterOutStdFlag, filterOutClangUnknownCflags))
-	ca.asFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Asflags, true, nil))
+	ca.asFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Asflags, nil))
-	ca.conlyFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Conlyflags, true, filterOutClangUnknownCflags))
+	ca.conlyFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Conlyflags, filterOutClangUnknownCflags))
-	ca.cppFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Cppflags, true, 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.