diff --git a/android/bazel.go b/android/bazel.go index ba5ae3133..4b18ad6e2 100644 --- a/android/bazel.go +++ b/android/bazel.go @@ -220,8 +220,7 @@ var ( // Per-module denylist to opt modules out of mixed builds. Such modules will // still be generated via bp2build. mixedBuildsDisabledList = []string{ - "libc_gdtoa", // ruperts@, cc_library_static, OK for bp2build but undefined symbol: __strtorQ for mixed builds - "libc_openbsd", // ruperts@, cc_library_static, OK for bp2build but error: duplicate symbol: strcpy for mixed builds + "libc_gdtoa", // ruperts@, cc_library_static, OK for bp2build but undefined symbol: __strtorQ for mixed builds } // Used for quicker lookups diff --git a/bazel/properties.go b/bazel/properties.go index a03b0270f..037150dbb 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -183,6 +183,15 @@ const ( OS_LINUX = "linux_glibc" OS_LINUX_BIONIC = "linux_bionic" OS_WINDOWS = "windows" + + // This is the string representation of the default condition wherever a + // configurable attribute is used in a select statement, i.e. + // //conditions:default for Bazel. + // + // This is consistently named "conditions_default" to mirror the Soong + // config variable default key in an Android.bp file, although there's no + // integration with Soong config variables (yet). + CONDITIONS_DEFAULT = "conditions_default" ) var ( @@ -194,21 +203,23 @@ var ( // A map of architectures to the Bazel label of the constraint_value // for the @platforms//cpu:cpu constraint_setting PlatformArchMap = map[string]string{ - ARCH_ARM: "//build/bazel/platforms/arch:arm", - ARCH_ARM64: "//build/bazel/platforms/arch:arm64", - ARCH_X86: "//build/bazel/platforms/arch:x86", - ARCH_X86_64: "//build/bazel/platforms/arch:x86_64", + ARCH_ARM: "//build/bazel/platforms/arch:arm", + ARCH_ARM64: "//build/bazel/platforms/arch:arm64", + ARCH_X86: "//build/bazel/platforms/arch:x86", + ARCH_X86_64: "//build/bazel/platforms/arch:x86_64", + CONDITIONS_DEFAULT: "//conditions:default", // The default condition of as arch select map. } // A map of target operating systems to the Bazel label of the // constraint_value for the @platforms//os:os constraint_setting PlatformOsMap = map[string]string{ - OS_ANDROID: "//build/bazel/platforms/os:android", - OS_DARWIN: "//build/bazel/platforms/os:darwin", - OS_FUCHSIA: "//build/bazel/platforms/os:fuchsia", - OS_LINUX: "//build/bazel/platforms/os:linux", - OS_LINUX_BIONIC: "//build/bazel/platforms/os:linux_bionic", - OS_WINDOWS: "//build/bazel/platforms/os:windows", + OS_ANDROID: "//build/bazel/platforms/os:android", + OS_DARWIN: "//build/bazel/platforms/os:darwin", + OS_FUCHSIA: "//build/bazel/platforms/os:fuchsia", + OS_LINUX: "//build/bazel/platforms/os:linux", + OS_LINUX_BIONIC: "//build/bazel/platforms/os:linux_bionic", + OS_WINDOWS: "//build/bazel/platforms/os:windows", + CONDITIONS_DEFAULT: "//conditions:default", // The default condition of an os select map. } ) @@ -224,6 +235,8 @@ type labelListArchValues struct { Arm LabelList Arm64 LabelList Common LabelList + + ConditionsDefault LabelList } type labelListOsValues struct { @@ -233,6 +246,8 @@ type labelListOsValues struct { Linux LabelList LinuxBionic LabelList Windows LabelList + + ConditionsDefault LabelList } // LabelListAttribute is used to represent a list of Bazel labels as an @@ -296,10 +311,11 @@ func (attrs LabelListAttribute) HasConfigurableValues() bool { func (attrs *LabelListAttribute) archValuePtrs() map[string]*LabelList { return map[string]*LabelList{ - ARCH_X86: &attrs.ArchValues.X86, - ARCH_X86_64: &attrs.ArchValues.X86_64, - ARCH_ARM: &attrs.ArchValues.Arm, - ARCH_ARM64: &attrs.ArchValues.Arm64, + ARCH_X86: &attrs.ArchValues.X86, + ARCH_X86_64: &attrs.ArchValues.X86_64, + ARCH_ARM: &attrs.ArchValues.Arm, + ARCH_ARM64: &attrs.ArchValues.Arm64, + CONDITIONS_DEFAULT: &attrs.ArchValues.ConditionsDefault, } } @@ -323,12 +339,13 @@ func (attrs *LabelListAttribute) SetValueForArch(arch string, value LabelList) { func (attrs *LabelListAttribute) osValuePtrs() map[string]*LabelList { return map[string]*LabelList{ - OS_ANDROID: &attrs.OsValues.Android, - OS_DARWIN: &attrs.OsValues.Darwin, - OS_FUCHSIA: &attrs.OsValues.Fuchsia, - OS_LINUX: &attrs.OsValues.Linux, - OS_LINUX_BIONIC: &attrs.OsValues.LinuxBionic, - OS_WINDOWS: &attrs.OsValues.Windows, + OS_ANDROID: &attrs.OsValues.Android, + OS_DARWIN: &attrs.OsValues.Darwin, + OS_FUCHSIA: &attrs.OsValues.Fuchsia, + OS_LINUX: &attrs.OsValues.Linux, + OS_LINUX_BIONIC: &attrs.OsValues.LinuxBionic, + OS_WINDOWS: &attrs.OsValues.Windows, + CONDITIONS_DEFAULT: &attrs.OsValues.ConditionsDefault, } } @@ -381,6 +398,8 @@ type stringListArchValues struct { Arm []string Arm64 []string Common []string + + ConditionsDefault []string } type stringListOsValues struct { @@ -390,6 +409,8 @@ type stringListOsValues struct { Linux []string LinuxBionic []string Windows []string + + ConditionsDefault []string } // HasConfigurableValues returns true if the attribute contains @@ -411,10 +432,11 @@ func (attrs StringListAttribute) HasConfigurableValues() bool { func (attrs *StringListAttribute) archValuePtrs() map[string]*[]string { return map[string]*[]string{ - ARCH_X86: &attrs.ArchValues.X86, - ARCH_X86_64: &attrs.ArchValues.X86_64, - ARCH_ARM: &attrs.ArchValues.Arm, - ARCH_ARM64: &attrs.ArchValues.Arm64, + ARCH_X86: &attrs.ArchValues.X86, + ARCH_X86_64: &attrs.ArchValues.X86_64, + ARCH_ARM: &attrs.ArchValues.Arm, + ARCH_ARM64: &attrs.ArchValues.Arm64, + CONDITIONS_DEFAULT: &attrs.ArchValues.ConditionsDefault, } } @@ -438,12 +460,13 @@ func (attrs *StringListAttribute) SetValueForArch(arch string, value []string) { func (attrs *StringListAttribute) osValuePtrs() map[string]*[]string { return map[string]*[]string{ - OS_ANDROID: &attrs.OsValues.Android, - OS_DARWIN: &attrs.OsValues.Darwin, - OS_FUCHSIA: &attrs.OsValues.Fuchsia, - OS_LINUX: &attrs.OsValues.Linux, - OS_LINUX_BIONIC: &attrs.OsValues.LinuxBionic, - OS_WINDOWS: &attrs.OsValues.Windows, + OS_ANDROID: &attrs.OsValues.Android, + OS_DARWIN: &attrs.OsValues.Darwin, + OS_FUCHSIA: &attrs.OsValues.Fuchsia, + OS_LINUX: &attrs.OsValues.Linux, + OS_LINUX_BIONIC: &attrs.OsValues.LinuxBionic, + OS_WINDOWS: &attrs.OsValues.Windows, + CONDITIONS_DEFAULT: &attrs.OsValues.ConditionsDefault, } } diff --git a/bp2build/cc_library_static_conversion_test.go b/bp2build/cc_library_static_conversion_test.go index 207a080e8..9d23cc599 100644 --- a/bp2build/cc_library_static_conversion_test.go +++ b/bp2build/cc_library_static_conversion_test.go @@ -524,6 +524,201 @@ cc_library_static { name = "static_dep4", copts = ["-I."], linkstatic = True, +)`}, + }, + { + description: "cc_library_static simple exclude_srcs", + moduleTypeUnderTest: "cc_library_static", + moduleTypeUnderTestFactory: cc.LibraryStaticFactory, + moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryStaticBp2Build, + depsMutators: []android.RegisterMutatorFunc{cc.RegisterDepsBp2Build}, + filesystem: map[string]string{ + "common.c": "", + "foo-a.c": "", + "foo-excluded.c": "", + }, + bp: soongCcLibraryStaticPreamble + ` +cc_library_static { + name: "foo_static", + srcs: ["common.c", "foo-*.c"], + exclude_srcs: ["foo-excluded.c"], +}`, + expectedBazelTargets: []string{`cc_library_static( + name = "foo_static", + copts = ["-I."], + linkstatic = True, + srcs = [ + "common.c", + "foo-a.c", + ], +)`}, + }, + { + description: "cc_library_static one arch specific srcs", + moduleTypeUnderTest: "cc_library_static", + moduleTypeUnderTestFactory: cc.LibraryStaticFactory, + moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryStaticBp2Build, + depsMutators: []android.RegisterMutatorFunc{cc.RegisterDepsBp2Build}, + filesystem: map[string]string{ + "common.c": "", + "foo-arm.c": "", + }, + bp: soongCcLibraryStaticPreamble + ` +cc_library_static { + name: "foo_static", + srcs: ["common.c"], + arch: { arm: { srcs: ["foo-arm.c"] } } +}`, + expectedBazelTargets: []string{`cc_library_static( + name = "foo_static", + copts = ["-I."], + linkstatic = True, + srcs = ["common.c"] + select({ + "//build/bazel/platforms/arch:arm": ["foo-arm.c"], + "//conditions:default": [], + }), +)`}, + }, + { + description: "cc_library_static one arch specific srcs and exclude_srcs", + moduleTypeUnderTest: "cc_library_static", + moduleTypeUnderTestFactory: cc.LibraryStaticFactory, + moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryStaticBp2Build, + depsMutators: []android.RegisterMutatorFunc{cc.RegisterDepsBp2Build}, + filesystem: map[string]string{ + "common.c": "", + "for-arm.c": "", + "not-for-arm.c": "", + "not-for-anything.c": "", + }, + bp: soongCcLibraryStaticPreamble + ` +cc_library_static { + name: "foo_static", + srcs: ["common.c", "not-for-*.c"], + exclude_srcs: ["not-for-anything.c"], + arch: { + arm: { srcs: ["for-arm.c"], exclude_srcs: ["not-for-arm.c"] }, + }, +}`, + expectedBazelTargets: []string{`cc_library_static( + name = "foo_static", + copts = ["-I."], + linkstatic = True, + srcs = ["common.c"] + select({ + "//build/bazel/platforms/arch:arm": ["for-arm.c"], + "//conditions:default": ["not-for-arm.c"], + }), +)`}, + }, + { + description: "cc_library_static arch specific exclude_srcs for 2 architectures", + moduleTypeUnderTest: "cc_library_static", + moduleTypeUnderTestFactory: cc.LibraryStaticFactory, + moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryStaticBp2Build, + depsMutators: []android.RegisterMutatorFunc{cc.RegisterDepsBp2Build}, + filesystem: map[string]string{ + "common.c": "", + "for-arm.c": "", + "for-x86.c": "", + "not-for-arm.c": "", + "not-for-x86.c": "", + }, + bp: soongCcLibraryStaticPreamble + ` +cc_library_static { + name: "foo_static", + srcs: ["common.c", "not-for-*.c"], + exclude_srcs: ["not-for-everything.c"], + arch: { + arm: { srcs: ["for-arm.c"], exclude_srcs: ["not-for-arm.c"] }, + x86: { srcs: ["for-x86.c"], exclude_srcs: ["not-for-x86.c"] }, + }, +} `, + expectedBazelTargets: []string{`cc_library_static( + name = "foo_static", + copts = ["-I."], + linkstatic = True, + srcs = ["common.c"] + select({ + "//build/bazel/platforms/arch:arm": [ + "for-arm.c", + "not-for-x86.c", + ], + "//build/bazel/platforms/arch:x86": [ + "for-x86.c", + "not-for-arm.c", + ], + "//conditions:default": [ + "not-for-arm.c", + "not-for-x86.c", + ], + }), +)`}, + }, + { + description: "cc_library_static arch specific exclude_srcs for 4 architectures", + moduleTypeUnderTest: "cc_library_static", + moduleTypeUnderTestFactory: cc.LibraryStaticFactory, + moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryStaticBp2Build, + depsMutators: []android.RegisterMutatorFunc{cc.RegisterDepsBp2Build}, + filesystem: map[string]string{ + "common.c": "", + "for-arm.c": "", + "for-arm64.c": "", + "for-x86.c": "", + "for-x86_64.c": "", + "not-for-arm.c": "", + "not-for-arm64.c": "", + "not-for-x86.c": "", + "not-for-x86_64.c": "", + "not-for-everything.c": "", + }, + bp: soongCcLibraryStaticPreamble + ` +cc_library_static { + name: "foo_static", + srcs: ["common.c", "not-for-*.c"], + exclude_srcs: ["not-for-everything.c"], + arch: { + arm: { srcs: ["for-arm.c"], exclude_srcs: ["not-for-arm.c"] }, + arm64: { srcs: ["for-arm64.c"], exclude_srcs: ["not-for-arm64.c"] }, + x86: { srcs: ["for-x86.c"], exclude_srcs: ["not-for-x86.c"] }, + x86_64: { srcs: ["for-x86_64.c"], exclude_srcs: ["not-for-x86_64.c"] }, + }, +} `, + expectedBazelTargets: []string{`cc_library_static( + name = "foo_static", + copts = ["-I."], + linkstatic = True, + srcs = ["common.c"] + select({ + "//build/bazel/platforms/arch:arm": [ + "for-arm.c", + "not-for-arm64.c", + "not-for-x86.c", + "not-for-x86_64.c", + ], + "//build/bazel/platforms/arch:arm64": [ + "for-arm64.c", + "not-for-arm.c", + "not-for-x86.c", + "not-for-x86_64.c", + ], + "//build/bazel/platforms/arch:x86": [ + "for-x86.c", + "not-for-arm.c", + "not-for-arm64.c", + "not-for-x86_64.c", + ], + "//build/bazel/platforms/arch:x86_64": [ + "for-x86_64.c", + "not-for-arm.c", + "not-for-arm64.c", + "not-for-x86.c", + ], + "//conditions:default": [ + "not-for-arm.c", + "not-for-arm64.c", + "not-for-x86.c", + "not-for-x86_64.c", + ], + }), )`}, }, } diff --git a/bp2build/configurability.go b/bp2build/configurability.go index b9ffc0450..52afb556d 100644 --- a/bp2build/configurability.go +++ b/bp2build/configurability.go @@ -99,8 +99,15 @@ func prettyPrintSelectMap(selectMap map[string]reflect.Value, defaultValue strin return "", nil } + // addConditionsDefault := false + conditionsDefaultKey := bazel.PlatformArchMap[bazel.CONDITIONS_DEFAULT] + var selects string for _, selectKey := range android.SortedStringKeys(selectMap) { + if selectKey == conditionsDefaultKey { + // Handle default condition later. + continue + } value := selectMap[selectKey] if isZero(value) { // Ignore zero values to not generate empty lists. @@ -125,8 +132,22 @@ func prettyPrintSelectMap(selectMap map[string]reflect.Value, defaultValue strin // Create the map. ret := "select({\n" ret += selects - // default condition comes last. - ret += fmt.Sprintf("%s\"%s\": %s,\n", makeIndent(indent+1), "//conditions:default", defaultValue) + + // Handle the default condition + s, err := prettyPrintSelectEntry(selectMap[conditionsDefaultKey], conditionsDefaultKey, indent) + if err != nil { + return "", err + } + if s == "" { + // Print an explicit empty list (the default value) even if the value is + // empty, to avoid errors about not finding a configuration that matches. + ret += fmt.Sprintf("%s\"%s\": %s,\n", makeIndent(indent+1), "//conditions:default", defaultValue) + } else { + // Print the custom default value. + ret += s + ret += ",\n" + } + ret += makeIndent(indent) ret += "})" diff --git a/cc/bp2build.go b/cc/bp2build.go index b11602dd4..9c4bf6e54 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -90,11 +90,6 @@ func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Modul return "-I" + filepath.Join(ctx.ModuleDir(), dir) } - // Parse the list of srcs, excluding files from exclude_srcs. - parseSrcs := func(baseCompilerProps *BaseCompilerProperties) bazel.LabelList { - return android.BazelLabelForModuleSrcExcludes(ctx, baseCompilerProps.Srcs, baseCompilerProps.Exclude_srcs) - } - // Parse the list of module-relative include directories (-I). parseLocalIncludeDirs := func(baseCompilerProps *BaseCompilerProperties) []string { // include_dirs are root-relative, not module-relative. @@ -111,14 +106,40 @@ func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Modul return copts } + // baseSrcs contain the list of src files that are used for every configuration. + var baseSrcs []string + // baseExcludeSrcs contain the list of src files that are excluded for every configuration. + var baseExcludeSrcs []string + // baseSrcsLabelList is a clone of the base srcs LabelList, used for computing the + // arch or os specific srcs later. + var baseSrcsLabelList bazel.LabelList + + // Parse srcs from an arch or OS's props value, taking the base srcs and + // exclude srcs into account. + parseSrcs := func(baseCompilerProps *BaseCompilerProperties) bazel.LabelList { + // Combine the base srcs and arch-specific srcs + allSrcs := append(baseSrcs, baseCompilerProps.Srcs...) + // Combine the base exclude_srcs and configuration-specific exclude_srcs + allExcludeSrcs := append(baseExcludeSrcs, baseCompilerProps.Exclude_srcs...) + return android.BazelLabelForModuleSrcExcludes(ctx, allSrcs, allExcludeSrcs) + } + for _, props := range module.compiler.compilerProps() { if baseCompilerProps, ok := props.(*BaseCompilerProperties); ok { srcs.Value = parseSrcs(baseCompilerProps) copts.Value = parseCopts(baseCompilerProps) + + // Used for arch-specific srcs later. + baseSrcs = baseCompilerProps.Srcs + baseExcludeSrcs = baseCompilerProps.Exclude_srcs + baseSrcsLabelList = parseSrcs(baseCompilerProps) break } } + // Handle include_build_directory prop. If the property is true, then the + // target has access to all headers recursively in the package, and has + // "-I" in its copts. if c, ok := module.compiler.(*baseCompiler); ok && c.includeBuildDirectory() { copts.Value = append(copts.Value, includeFlag(".")) } else if c, ok := module.compiler.(*libraryDecorator); ok && c.includeBuildDirectory() { @@ -127,16 +148,40 @@ func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Modul for arch, props := range module.GetArchProperties(&BaseCompilerProperties{}) { if baseCompilerProps, ok := props.(*BaseCompilerProperties); ok { - srcsList := parseSrcs(baseCompilerProps) - srcs.SetValueForArch(arch.Name, bazel.SubtractBazelLabelList(srcsList, srcs.Value)) + // If there's arch specific srcs or exclude_srcs, generate a select entry for it. + // TODO(b/186153868): do this for OS specific srcs and exclude_srcs too. + if len(baseCompilerProps.Srcs) > 0 || len(baseCompilerProps.Exclude_srcs) > 0 { + srcsList := parseSrcs(baseCompilerProps) + srcs.SetValueForArch(arch.Name, srcsList) + // The base srcs value should not contain any arch-specific excludes. + srcs.Value = bazel.SubtractBazelLabelList(srcs.Value, bazel.LabelList{Includes: srcsList.Excludes}) + } + copts.SetValueForArch(arch.Name, parseCopts(baseCompilerProps)) } } + // After going through all archs, delete the duplicate files in the arch + // values that are already in the base srcs.Value. + for arch, props := range module.GetArchProperties(&BaseCompilerProperties{}) { + if _, ok := props.(*BaseCompilerProperties); ok { + srcs.SetValueForArch(arch.Name, bazel.SubtractBazelLabelList(srcs.GetValueForArch(arch.Name), srcs.Value)) + } + } + + // Now that the srcs.Value list is finalized, compare it with the original + // list, and put the difference into the default condition for the arch + // select. + defaultsSrcs := bazel.SubtractBazelLabelList(baseSrcsLabelList, srcs.Value) + // TODO(b/186153868): handle the case with multiple variant types, e.g. when arch and os are both used. + srcs.SetValueForArch(bazel.CONDITIONS_DEFAULT, defaultsSrcs) + + // Handle OS specific props. for os, props := range module.GetTargetProperties(&BaseCompilerProperties{}) { if baseCompilerProps, ok := props.(*BaseCompilerProperties); ok { srcsList := parseSrcs(baseCompilerProps) - srcs.SetValueForOS(os.Name, bazel.SubtractBazelLabelList(srcsList, srcs.Value)) + // 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)) } }