Split asm and c flags and srcs in bp2build output
This allows removal of almost all current items from the mixed build
denylist, which were previously broken due to being unable to separately
control flags for compilations of different languages within the same
target.
Note that this does not appropriately implement asm/c srcs and flags for
either the shared variant or the static variant. This will require a
followup.
Test: bp2build.sh and mixed_libc.sh CI scripts
Test: Updated b2build tests
Change-Id: I28cf7437ee96cdf2fdbcb1eda2303691cff08ba4
diff --git a/android/bazel.go b/android/bazel.go
index f56c24e..c7669ac 100644
--- a/android/bazel.go
+++ b/android/bazel.go
@@ -220,15 +220,7 @@
// Per-module denylist to opt modules out of mixed builds. Such modules will
// still be generated via bp2build.
mixedBuildsDisabledList = []string{
- "libc_common", // cparsons@ cc_library_static, depends on //bionic/libc:libc_nopthread
- "libc_common_static", // cparsons@ cc_library_static, depends on //bionic/libc:libc_common
- "libc_common_shared", // cparsons@ cc_library_static, depends on //bionic/libc:libc_common
- "libc_netbsd", // lberki@, cc_library_static, version script assignment of 'LIBC_PRIVATE' to symbol 'SHA1Final' failed: symbol not defined
- "libc_nopthread", // cparsons@ cc_library_static, version script assignment of 'LIBC' to symbol 'memcmp' failed: symbol not defined
- "libc_openbsd", // ruperts@, cc_library_static, OK for bp2build but error: duplicate symbol: strcpy for mixed builds
- "libarm-optimized-routines-string", // jingwen@, cc_library_static, OK for bp2build but b/186615213 (asflags not handled in bp2build), version script assignment of 'LIBC' to symbol 'memcmp' failed: symbol not defined (also for memrchr, strnlen)
- "fmtlib_ndk", // http://b/187040371, cc_library_static, OK for bp2build but format-inl.h:11:10: fatal error: 'cassert' file not found for mixed builds
- "libc_nomalloc", // cc_library_static, OK for bp2build but ld.lld: error: undefined symbol: pthread_mutex_lock (and others)
+ "libc_common_shared", // cparsons@ cc_library_static, version script assignment of 'LIBC' to symbol '__cxa_atexit' failed: symbol not defined
}
// Used for quicker lookups
diff --git a/bazel/properties.go b/bazel/properties.go
index 640275f..49c99c2 100644
--- a/bazel/properties.go
+++ b/bazel/properties.go
@@ -137,6 +137,54 @@
return strings
}
+// Return all needles in a given haystack, where needleFn is true for needles.
+func FilterLabelList(haystack LabelList, needleFn func(string) bool) LabelList {
+ var includes []Label
+
+ for _, inc := range haystack.Includes {
+ if needleFn(inc.Label) {
+ includes = append(includes, inc)
+ }
+ }
+ return LabelList{Includes: includes, Excludes: haystack.Excludes}
+}
+
+// Return all needles in a given haystack, where needleFn is true for needles.
+func FilterLabelListAttribute(haystack LabelListAttribute, needleFn func(string) bool) LabelListAttribute {
+ var result LabelListAttribute
+
+ result.Value = FilterLabelList(haystack.Value, needleFn)
+
+ for arch := range PlatformArchMap {
+ result.SetValueForArch(arch, FilterLabelList(haystack.GetValueForArch(arch), needleFn))
+ }
+
+ for os := range PlatformOsMap {
+ result.SetValueForOS(os, FilterLabelList(haystack.GetValueForOS(os), needleFn))
+ }
+
+ return result
+}
+
+// Subtract needle from haystack
+func SubtractBazelLabelListAttribute(haystack LabelListAttribute, needle LabelListAttribute) LabelListAttribute {
+ var result LabelListAttribute
+
+ for arch := range PlatformArchMap {
+ result.SetValueForArch(arch,
+ SubtractBazelLabelList(haystack.GetValueForArch(arch), needle.GetValueForArch(arch)))
+ }
+
+ for os := range PlatformOsMap {
+ result.SetValueForOS(os,
+ SubtractBazelLabelList(haystack.GetValueForOS(os), needle.GetValueForOS(os)))
+ }
+
+ result.Value = SubtractBazelLabelList(haystack.Value, needle.Value)
+
+ return result
+}
+
// Subtract needle from haystack
func SubtractBazelLabels(haystack []Label, needle []Label) []Label {
// This is really a set
diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go
index 0a72937..454b153 100644
--- a/bp2build/cc_library_conversion_test.go
+++ b/bp2build/cc_library_conversion_test.go
@@ -311,7 +311,7 @@
"//build/bazel/platforms/arch:arm64": ["-DHAVE_FAST_FMA=1"],
"//conditions:default": [],
}),
- srcs = ["math/cosf.c"],
+ srcs_c = ["math/cosf.c"],
)`},
})
}
@@ -618,7 +618,7 @@
func TestCcLibraryCppFlagsGoesIntoCopts(t *testing.T) {
runCcLibraryTestCase(t, bp2buildTestCase{
- description: "cc_library cppflags goes into copts",
+ description: "cc_library cppflags usage",
moduleTypeUnderTest: "cc_library",
moduleTypeUnderTestFactory: cc.LibraryFactory,
moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryBp2Build,
@@ -654,10 +654,12 @@
name = "a",
copts = [
"-Wall",
- "-fsigned-char",
- "-pedantic",
"-Ifoo/bar",
"-I$(BINDIR)/foo/bar",
+ ],
+ cppflags = [
+ "-fsigned-char",
+ "-pedantic",
] + select({
"//build/bazel/platforms/arch:arm64": ["-DARM64=1"],
"//conditions:default": [],
diff --git a/bp2build/cc_library_static_conversion_test.go b/bp2build/cc_library_static_conversion_test.go
index 44c6ad4..6c2b317 100644
--- a/bp2build/cc_library_static_conversion_test.go
+++ b/bp2build/cc_library_static_conversion_test.go
@@ -650,7 +650,7 @@
"-I$(BINDIR)/.",
],
linkstatic = True,
- srcs = [
+ srcs_c = [
"common.c",
"foo-a.c",
],
@@ -682,7 +682,7 @@
"-I$(BINDIR)/.",
],
linkstatic = True,
- srcs = ["common.c"] + select({
+ srcs_c = ["common.c"] + select({
"//build/bazel/platforms/arch:arm": ["foo-arm.c"],
"//conditions:default": [],
}),
@@ -719,7 +719,7 @@
"-I$(BINDIR)/.",
],
linkstatic = True,
- srcs = ["common.c"] + select({
+ srcs_c = ["common.c"] + select({
"//build/bazel/platforms/arch:arm": ["for-arm.c"],
"//conditions:default": ["not-for-arm.c"],
}),
@@ -758,7 +758,7 @@
"-I$(BINDIR)/.",
],
linkstatic = True,
- srcs = ["common.c"] + select({
+ srcs_c = ["common.c"] + select({
"//build/bazel/platforms/arch:arm": [
"for-arm.c",
"not-for-x86.c",
@@ -813,7 +813,7 @@
"-I$(BINDIR)/.",
],
linkstatic = True,
- srcs = ["common.c"] + select({
+ srcs_c = ["common.c"] + select({
"//build/bazel/platforms/arch:arm": [
"for-arm.c",
"not-for-arm64.c",
@@ -909,7 +909,7 @@
"-I$(BINDIR)/.",
],
linkstatic = True,
- srcs = ["common.c"] + select({
+ srcs_c = ["common.c"] + select({
"//build/bazel/platforms/arch:arm": ["for-lib32.c"],
"//build/bazel/platforms/arch:x86": ["for-lib32.c"],
"//conditions:default": ["not-for-lib32.c"],
@@ -948,7 +948,7 @@
"-I$(BINDIR)/.",
],
linkstatic = True,
- srcs = ["common.c"] + select({
+ srcs_c = ["common.c"] + select({
"//build/bazel/platforms/arch:arm": [
"for-lib32.c",
"not-for-lib64.c",
@@ -1020,7 +1020,7 @@
"-I$(BINDIR)/.",
],
linkstatic = True,
- srcs = ["common.c"] + select({
+ srcs_c = ["common.c"] + select({
"//build/bazel/platforms/arch:arm": [
"for-arm.c",
"for-lib32.c",
@@ -1074,10 +1074,10 @@
moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryStaticBp2Build,
depsMutators: []android.RegisterMutatorFunc{cc.RegisterDepsBp2Build},
filesystem: map[string]string{
- "common.c": "",
- "for-x86.c": "",
- "not-for-x86.c": "",
- "not-for-everything.c": "",
+ "common.cpp": "",
+ "for-x86.cpp": "",
+ "not-for-x86.cpp": "",
+ "not-for-everything.cpp": "",
"dep/Android.bp": `
genrule {
name: "generated_src_other_pkg",
@@ -1118,14 +1118,14 @@
cc_library_static {
name: "foo_static3",
- srcs: ["common.c", "not-for-*.c"],
- exclude_srcs: ["not-for-everything.c"],
+ srcs: ["common.cpp", "not-for-*.cpp"],
+ exclude_srcs: ["not-for-everything.cpp"],
generated_sources: ["generated_src", "generated_src_other_pkg"],
generated_headers: ["generated_hdr", "generated_hdr_other_pkg"],
arch: {
x86: {
- srcs: ["for-x86.c"],
- exclude_srcs: ["not-for-x86.c"],
+ srcs: ["for-x86.cpp"],
+ exclude_srcs: ["not-for-x86.cpp"],
generated_sources: ["generated_src_x86"],
generated_headers: ["generated_hdr_other_pkg_x86"],
},
@@ -1144,14 +1144,14 @@
"//dep:generated_src_other_pkg",
":generated_hdr",
":generated_src",
- "common.c",
+ "common.cpp",
] + select({
"//build/bazel/platforms/arch:x86": [
"//dep:generated_hdr_other_pkg_x86",
":generated_src_x86",
- "for-x86.c",
+ "for-x86.cpp",
],
- "//conditions:default": ["not-for-x86.c"],
+ "//conditions:default": ["not-for-x86.cpp"],
}),
)`},
})
@@ -1197,7 +1197,7 @@
"//conditions:default": [],
}),
linkstatic = True,
- srcs = ["common.c"],
+ srcs_c = ["common.c"],
)`},
})
}
diff --git a/cc/bp2build.go b/cc/bp2build.go
index 0c827c5..e417c69 100644
--- a/cc/bp2build.go
+++ b/cc/bp2build.go
@@ -183,15 +183,26 @@
// Convenience struct to hold all attributes parsed from compiler properties.
type compilerAttributes struct {
- copts bazel.StringListAttribute
+ // Options for all languages
+ copts bazel.StringListAttribute
+ // Assembly options and sources
+ asFlags bazel.StringListAttribute
+ asSrcs bazel.LabelListAttribute
+ // C options and sources
+ conlyFlags bazel.StringListAttribute
+ cSrcs bazel.LabelListAttribute
+ // C++ options and sources
+ cppFlags bazel.StringListAttribute
srcs bazel.LabelListAttribute
- includes bazel.StringListAttribute
}
// bp2BuildParseCompilerProps returns copts, srcs and hdrs and other attributes.
func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Module) compilerAttributes {
var srcs bazel.LabelListAttribute
var copts bazel.StringListAttribute
+ var asFlags bazel.StringListAttribute
+ var conlyFlags bazel.StringListAttribute
+ var cppFlags bazel.StringListAttribute
// Creates the -I flags for a directory, while making the directory relative
// to the exec root for Bazel to work.
@@ -215,15 +226,21 @@
return append(includeDirs, baseCompilerProps.Local_include_dirs...)
}
- // Parse the list of copts.
- parseCopts := func(baseCompilerProps *BaseCompilerProperties) []string {
- var copts []string
- for _, flag := range append(baseCompilerProps.Cflags, baseCompilerProps.Cppflags...) {
+ parseCommandLineFlags := func(soongFlags []string) []string {
+ var result []string
+ for _, flag := range soongFlags {
// 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.
- copts = append(copts, strings.Split(flag, " ")...)
+ result = append(result, strings.Split(flag, " ")...)
}
+ return result
+ }
+
+ // Parse the list of copts.
+ parseCopts := func(baseCompilerProps *BaseCompilerProperties) []string {
+ var copts []string
+ copts = append(copts, parseCommandLineFlags(baseCompilerProps.Cflags)...)
for _, dir := range parseLocalIncludeDirs(baseCompilerProps) {
copts = append(copts, includeFlags(dir)...)
}
@@ -260,6 +277,9 @@
if baseCompilerProps, ok := props.(*BaseCompilerProperties); ok {
srcs.Value = parseSrcs(baseCompilerProps)
copts.Value = parseCopts(baseCompilerProps)
+ asFlags.Value = parseCommandLineFlags(baseCompilerProps.Asflags)
+ conlyFlags.Value = parseCommandLineFlags(baseCompilerProps.Conlyflags)
+ cppFlags.Value = parseCommandLineFlags(baseCompilerProps.Cppflags)
// Used for arch-specific srcs later.
baseSrcs = baseCompilerProps.Srcs
@@ -290,6 +310,9 @@
}
copts.SetValueForArch(arch.Name, parseCopts(baseCompilerProps))
+ asFlags.SetValueForArch(arch.Name, parseCommandLineFlags(baseCompilerProps.Asflags))
+ conlyFlags.SetValueForArch(arch.Name, parseCommandLineFlags(baseCompilerProps.Conlyflags))
+ cppFlags.SetValueForArch(arch.Name, parseCommandLineFlags(baseCompilerProps.Cppflags))
}
}
@@ -315,6 +338,9 @@
// TODO(b/186153868): add support for os-specific srcs and exclude_srcs
srcs.SetValueForOS(os.Name, bazel.SubtractBazelLabelList(srcsList, baseSrcsLabelList))
copts.SetValueForOS(os.Name, parseCopts(baseCompilerProps))
+ asFlags.SetValueForOS(os.Name, parseCommandLineFlags(baseCompilerProps.Asflags))
+ conlyFlags.SetValueForOS(os.Name, parseCommandLineFlags(baseCompilerProps.Conlyflags))
+ cppFlags.SetValueForOS(os.Name, parseCommandLineFlags(baseCompilerProps.Cppflags))
}
}
@@ -333,9 +359,28 @@
}
}
+ // Branch srcs into three language-specific groups.
+ // C++ is the "catch-all" group, and comprises generated sources because we don't
+ // know the language of these sources until the genrule is executed.
+ // TODO(b/): Handle language detection of sources in a Bazel rule.
+ isCSrc := func(s string) bool {
+ return strings.HasSuffix(s, ".c")
+ }
+ isAsmSrc := func(s string) bool {
+ return strings.HasSuffix(s, ".S") || strings.HasSuffix(s, ".s")
+ }
+ cSrcs := bazel.FilterLabelListAttribute(srcs, isCSrc)
+ asSrcs := bazel.FilterLabelListAttribute(srcs, isAsmSrc)
+ srcs = bazel.SubtractBazelLabelListAttribute(srcs, cSrcs)
+ srcs = bazel.SubtractBazelLabelListAttribute(srcs, asSrcs)
return compilerAttributes{
- srcs: srcs,
- copts: copts,
+ copts: copts,
+ srcs: srcs,
+ asFlags: asFlags,
+ asSrcs: asSrcs,
+ cSrcs: cSrcs,
+ conlyFlags: conlyFlags,
+ cppFlags: cppFlags,
}
}
diff --git a/cc/library.go b/cc/library.go
index c918b96..9fb7a24 100644
--- a/cc/library.go
+++ b/cc/library.go
@@ -230,6 +230,13 @@
Copts bazel.StringListAttribute
Includes bazel.StringListAttribute
Linkopts bazel.StringListAttribute
+
+ Cppflags bazel.StringListAttribute
+ Srcs_c bazel.LabelListAttribute
+ Conlyflags bazel.StringListAttribute
+ Srcs_as bazel.LabelListAttribute
+ Asflags bazel.StringListAttribute
+
// Attributes pertaining to shared variant.
Shared_copts bazel.StringListAttribute
Shared_srcs bazel.LabelListAttribute
@@ -239,6 +246,7 @@
Whole_archive_deps_for_shared bazel.LabelListAttribute
User_link_flags bazel.StringListAttribute
Version_script bazel.LabelAttribute
+
// Attributes pertaining to static variant.
Static_copts bazel.StringListAttribute
Static_srcs bazel.LabelListAttribute
@@ -294,20 +302,27 @@
srcs.Append(compilerAttrs.srcs)
attrs := &bazelCcLibraryAttributes{
- Srcs: srcs,
- Implementation_deps: linkerAttrs.deps,
- Deps: linkerAttrs.exportedDeps,
- Dynamic_deps: linkerAttrs.dynamicDeps,
- Whole_archive_deps: linkerAttrs.wholeArchiveDeps,
- Copts: compilerAttrs.copts,
- Includes: exportedIncludes,
- Linkopts: linkerAttrs.linkopts,
+ Srcs: srcs,
+ Implementation_deps: linkerAttrs.deps,
+ Deps: linkerAttrs.exportedDeps,
+ Dynamic_deps: linkerAttrs.dynamicDeps,
+ Whole_archive_deps: linkerAttrs.wholeArchiveDeps,
+ Copts: compilerAttrs.copts,
+ Includes: exportedIncludes,
+ Linkopts: linkerAttrs.linkopts,
+ Cppflags: compilerAttrs.cppFlags,
+ Srcs_c: compilerAttrs.cSrcs,
+ Conlyflags: compilerAttrs.conlyFlags,
+ Srcs_as: compilerAttrs.asSrcs,
+ Asflags: compilerAttrs.asFlags,
+
Shared_copts: sharedAttrs.copts,
Shared_srcs: sharedAttrs.srcs,
Static_deps_for_shared: sharedAttrs.staticDeps,
Whole_archive_deps_for_shared: sharedAttrs.wholeArchiveDeps,
Dynamic_deps_for_shared: sharedAttrs.dynamicDeps,
Version_script: linkerAttrs.versionScript,
+
Static_copts: staticAttrs.copts,
Static_srcs: staticAttrs.srcs,
Static_deps_for_static: staticAttrs.staticDeps,
@@ -2230,6 +2245,12 @@
Linkstatic bool
Includes bazel.StringListAttribute
Hdrs bazel.LabelListAttribute
+
+ Cppflags bazel.StringListAttribute
+ Srcs_c bazel.LabelListAttribute
+ Conlyflags bazel.StringListAttribute
+ Srcs_as bazel.LabelListAttribute
+ Asflags bazel.StringListAttribute
}
type bazelCcLibraryStatic struct {
@@ -2259,6 +2280,12 @@
Linkopts: linkerAttrs.linkopts,
Linkstatic: true,
Includes: exportedIncludes,
+
+ Cppflags: compilerAttrs.cppFlags,
+ Srcs_c: compilerAttrs.cSrcs,
+ Conlyflags: compilerAttrs.conlyFlags,
+ Srcs_as: compilerAttrs.asSrcs,
+ Asflags: compilerAttrs.asFlags,
}
props := bazel.BazelTargetModuleProperties{
diff --git a/cc/object.go b/cc/object.go
index 704cb69..cd71161 100644
--- a/cc/object.go
+++ b/cc/object.go
@@ -185,8 +185,14 @@
}
// 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
+ srcs.Append(compilerAttrs.cSrcs)
+ srcs.Append(compilerAttrs.asSrcs)
+
attrs := &bazelObjectAttributes{
- Srcs: compilerAttrs.srcs,
+ Srcs: srcs,
Deps: deps,
Copts: compilerAttrs.copts,
Asflags: asFlags,