From cac7f690ebeaccd65482ff0322a76e8d973ef872 Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Thu, 16 Dec 2021 14:19:32 -0500 Subject: [PATCH] bp2build remove "-std" from cflags Soong overrides "-std" flags when provided via cppflags or conlyflags; however, any user-provided "-std=" cflag will be overridden by Soong's std flag handling. For Bazel, we _always_ allow user to override via user provided flags. To prevent conflicts, we remove the silently ignored values from Android.bp files in the bp2build conversion. Test: build/bazel/ci/bp2build.sh Test: build/bazel/ci/mixed_droid.sh Change-Id: I4c33b2ae593a7ff3ff8e3ad15ef3461354fc0c83 --- android/bazel.go | 2 -- bp2build/cc_library_static_conversion_test.go | 18 +++++++++++++ cc/bp2build.go | 25 ++++++++++++++----- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/android/bazel.go b/android/bazel.go index 851a305cd..f4f9a7242 100644 --- a/android/bazel.go +++ b/android/bazel.go @@ -364,8 +364,6 @@ var ( // Per-module denylist to always opt modules out of both bp2build and mixed builds. bp2buildModuleDoNotConvertList = []string{ - "libnativehelper_compat_libc++", // Broken compile: implicit declaration of function 'strerror_r' is invalid in C99 - "libandroid_runtime_lazy", // depends on unconverted modules: libbinder_headers "libcmd", // depends on unconverted modules: libbinder diff --git a/bp2build/cc_library_static_conversion_test.go b/bp2build/cc_library_static_conversion_test.go index e2e55ddee..e65a1fa0b 100644 --- a/bp2build/cc_library_static_conversion_test.go +++ b/bp2build/cc_library_static_conversion_test.go @@ -1433,3 +1433,21 @@ func TestCcLibraryStaticUseVersionLib(t *testing.T) { }, }) } + +func TestCcLibraryStaticStdInFlags(t *testing.T) { + runCcLibraryStaticTestCase(t, bp2buildTestCase{ + blueprint: soongCcProtoPreamble + `cc_library_static { + name: "foo", + cflags: ["-std=candcpp"], + conlyflags: ["-std=conly"], + cppflags: ["-std=cpp"], + include_build_directory: false, +}`, + expectedBazelTargets: []string{ + makeBazelTarget("cc_library_static", "foo", attrNameToString{ + "conlyflags": `["-std=conly"]`, + "cppflags": `["-std=cpp"]`, + }), + }, + }) +} diff --git a/cc/bp2build.go b/cc/bp2build.go index fad40beea..2119ee43f 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -162,7 +162,7 @@ func bp2buildParseStaticOrSharedProps(ctx android.BazelConversionPathContext, mo attrs := staticOrSharedAttributes{} setAttrs := func(axis bazel.ConfigurationAxis, config string, props StaticOrSharedProperties) { - attrs.Copts.SetSelectValue(axis, config, props.Cflags) + 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)) @@ -279,9 +279,18 @@ type compilerAttributes struct { protoSrcs bazel.LabelListAttribute } -func parseCommandLineFlags(soongFlags []string) []string { +type filterOutFn func(string) bool + +func filterOutStdFlag(flag string) bool { + return strings.HasPrefix(flag, "-std=") +} + +func parseCommandLineFlags(soongFlags []string, filterOut filterOutFn) []string { var result []string for _, flag := range soongFlags { + if filterOut != nil && filterOut(flag) { + continue + } // 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. @@ -308,10 +317,14 @@ func (ca *compilerAttributes) bp2buildForAxisAndConfig(ctx android.BazelConversi ca.absoluteIncludes.SetSelectValue(axis, config, props.Include_dirs) ca.localIncludes.SetSelectValue(axis, config, localIncludeDirs) - ca.copts.SetSelectValue(axis, config, parseCommandLineFlags(props.Cflags)) - ca.asFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Asflags)) - ca.conlyFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Conlyflags)) - ca.cppFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Cppflags)) + // In Soong, cflags occur on the command line before -std= flag, resulting in the value being + // 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, filterOutStdFlag)) + ca.asFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Asflags, nil)) + ca.conlyFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Conlyflags, nil)) + ca.cppFlags.SetSelectValue(axis, config, parseCommandLineFlags(props.Cppflags, nil)) ca.rtti.SetSelectValue(axis, config, props.Rtti) }