From 91220d7334e5b3f99c717c3e2bf5040d7d2751c5 Mon Sep 17 00:00:00 2001 From: Jingwen Chen Date: Wed, 24 Mar 2021 02:18:33 -0400 Subject: [PATCH] Add os/target configurable selects for label list attributes. This CL is pretty large, so I recommend starting with reading the newly added tests for the expected behavior. This change works in conjunction with the linked CLs in the Gerrit topic. Those CLs add support for new platform() definitions for OS targets specified in Soong's arch.go, which are configurable through Android.bp's `target {}` property. It works similary to previous CLs adding support for the `arch {}` property. These configurable props are keyed by the OS: android, linux_bionic, windows, and so on. They map to `select` statements in label list attributes, which this CL enables for cc_library_headers' header_libs and export_header_lib_headers props. This enables //bionic/libc:libc_headers to be generated correctly, from: cc_library_headers { name: "libc_headers", target: { android: { header_libs: ["libc_headers_arch"], export_header_lib_headers: ["libc_headers_arch"], }, linux_bionic: { header_libs: ["libc_headers_arch"], export_header_lib_headers: ["libc_headers_arch"], }, }, // omitted props } to: cc_library_headers( name = "libc_headers", deps = [] + select({ "//build/bazel/platforms/os:android": [ ":libc_headers_arch", ], "//build/bazel/platforms/os:linux_bionic": [ ":libc_headers_arch", ], "//conditions:default": [], }), ) Test: TH Test: Verify generated //bionic/libc:libc_headers Fixes: 183597786 Change-Id: I01016cc2cc9a71449f02300d747f01decebf3f6e --- android/arch.go | 87 +++++++++ android/bazel.go | 10 + android/bazel_handler.go | 26 ++- bazel/properties.go | 176 +++++++++++++----- bp2build/build_conversion.go | 56 +----- .../cc_library_headers_conversion_test.go | 106 ++++++++++- bp2build/cc_object_conversion_test.go | 20 +- bp2build/configurability.go | 115 +++++++++++- bp2build/testing.go | 7 + cc/Android.bp | 1 + cc/bp2build.go | 106 +++++++++++ cc/library.go | 36 +--- cc/library_headers.go | 4 +- 13 files changed, 580 insertions(+), 170 deletions(-) create mode 100644 cc/bp2build.go diff --git a/android/arch.go b/android/arch.go index 20b4ab07d..3eff5d5e0 100644 --- a/android/arch.go +++ b/android/arch.go @@ -1709,3 +1709,90 @@ func (m *ModuleBase) GetArchProperties(dst interface{}) map[ArchType]interface{} } return archToProp } + +// GetTargetProperties returns a map of OS target (e.g. android, windows) to the +// values of the properties of the 'dst' struct that are specific to that OS +// target. +// +// For example, passing a struct { Foo bool, Bar string } will return an +// interface{} that can be type asserted back into the same struct, containing +// the os-specific property value specified by the module if defined. +// +// While this looks similar to GetArchProperties, the internal representation of +// the properties have a slightly different layout to warrant a standalone +// lookup function. +func (m *ModuleBase) GetTargetProperties(dst interface{}) map[OsType]interface{} { + // Return value of the arch types to the prop values for that arch. + osToProp := map[OsType]interface{}{} + + // Nothing to do for non-OS/arch-specific modules. + if !m.ArchSpecific() { + return osToProp + } + + // archProperties has the type of [][]interface{}. Looks complicated, so + // let's explain this step by step. + // + // Loop over the outer index, which determines the property struct that + // contains a matching set of properties in dst that we're interested in. + // For example, BaseCompilerProperties or BaseLinkerProperties. + for i := range m.archProperties { + if m.archProperties[i] == nil { + continue + } + + // Iterate over the supported OS types + for _, os := range OsTypeList { + // e.g android, linux_bionic + field := os.Field + + // If it's not nil, loop over the inner index, which determines the arch variant + // of the prop type. In an Android.bp file, this is like looping over: + // + // target: { android: { key: value, ... }, linux_bionic: { key: value, ... } } + for _, archProperties := range m.archProperties[i] { + archPropValues := reflect.ValueOf(archProperties).Elem() + + // This is the archPropRoot struct. Traverse into the Targetnested struct. + src := archPropValues.FieldByName("Target").Elem() + + // Step into non-nil pointers to structs in the src value. + if src.Kind() == reflect.Ptr { + if src.IsNil() { + continue + } + src = src.Elem() + } + + // Find the requested field (e.g. android, linux_bionic) in the src struct. + src = src.FieldByName(field) + + // Validation steps. We want valid non-nil pointers to structs. + if !src.IsValid() || src.IsNil() { + continue + } + + if src.Kind() != reflect.Ptr || src.Elem().Kind() != reflect.Struct { + continue + } + + // Clone the destination prop, since we want a unique prop struct per arch. + dstClone := reflect.New(reflect.ValueOf(dst).Elem().Type()).Interface() + + // Copy the located property struct into the cloned destination property struct. + err := proptools.ExtendMatchingProperties([]interface{}{dstClone}, src.Interface(), nil, proptools.OrderReplace) + if err != nil { + // This is fine, it just means the src struct doesn't match. + continue + } + + // Found the prop for the os, you have. + osToProp[os] = dstClone + + // Go to the next prop. + break + } + } + } + return osToProp +} diff --git a/android/bazel.go b/android/bazel.go index 51ff3cb27..2587328a9 100644 --- a/android/bazel.go +++ b/android/bazel.go @@ -108,6 +108,11 @@ type Bp2BuildConfig map[string]BazelConversionConfigEntry type BazelConversionConfigEntry int const ( + // A sentinel value to be used as a key in Bp2BuildConfig for modules with + // no package path. This is also the module dir for top level Android.bp + // modules. + BP2BUILD_TOPLEVEL = "." + // iota + 1 ensures that the int value is not 0 when used in the Bp2buildAllowlist map, // which can also mean that the key doesn't exist in a lookup. @@ -224,10 +229,15 @@ func (b *BazelModuleBase) ConvertWithBp2build(ctx BazelConversionPathContext) bo func bp2buildDefaultTrueRecursively(packagePath string, config Bp2BuildConfig) bool { ret := false + // Return exact matches in the config. + if config[packagePath] == Bp2BuildDefaultTrueRecursively { + return true + } if config[packagePath] == Bp2BuildDefaultFalse { return false } + // If not, check for the config recursively. packagePrefix := "" // e.g. for x/y/z, iterate over x, x/y, then x/y/z, taking the final value from the allowlist. for _, part := range strings.Split(packagePath, "/") { diff --git a/android/bazel_handler.go b/android/bazel_handler.go index abc793f8e..65115d1c3 100644 --- a/android/bazel_handler.go +++ b/android/bazel_handler.go @@ -270,13 +270,23 @@ func (context *bazelContext) issueBazelCommand(runName bazel.RunName, command st cmdFlags = append(cmdFlags, labels...) cmdFlags = append(cmdFlags, "--package_path=%workspace%/"+context.intermediatesDir()) cmdFlags = append(cmdFlags, "--profile="+shared.BazelMetricsFilename(context, runName)) - // Set default platforms to canonicalized values for mixed builds requests. If these are set - // in the bazelrc, they will have values that are non-canonicalized, and thus be invalid. - // The actual platform values here may be overridden by configuration transitions from the buildroot. + + // Set default platforms to canonicalized values for mixed builds requests. + // If these are set in the bazelrc, they will have values that are + // non-canonicalized to @sourceroot labels, and thus be invalid when + // referenced from the buildroot. + // + // The actual platform values here may be overridden by configuration + // transitions from the buildroot. cmdFlags = append(cmdFlags, - fmt.Sprintf("--platforms=%s", canonicalizeLabel("//build/bazel/platforms:generic_x86_64"))) + fmt.Sprintf("--platforms=%s", canonicalizeLabel("//build/bazel/platforms:android_x86_64"))) cmdFlags = append(cmdFlags, fmt.Sprintf("--extra_toolchains=%s", canonicalizeLabel("//prebuilts/clang/host/linux-x86:all"))) + // This should be parameterized on the host OS, but let's restrict to linux + // to keep things simple for now. + cmdFlags = append(cmdFlags, + fmt.Sprintf("--host_platform=%s", canonicalizeLabel("//build/bazel/platforms:linux_x86_64"))) + // Explicitly disable downloading rules (such as canonical C++ and Java rules) from the network. cmdFlags = append(cmdFlags, "--experimental_repository_disable_download") cmdFlags = append(cmdFlags, extraFlags...) @@ -328,7 +338,7 @@ func (context *bazelContext) mainBzlFileContents() []byte { def _config_node_transition_impl(settings, attr): return { - "//command_line_option:platforms": "@sourceroot//build/bazel/platforms:generic_%s" % attr.arch, + "//command_line_option:platforms": "@sourceroot//build/bazel/platforms:android_%s" % attr.arch, } _config_node_transition = transition( @@ -504,10 +514,10 @@ def get_arch(target): platform_name = build_options(target)["//command_line_option:platforms"][0].name if platform_name == "host": return "HOST" - elif not platform_name.startswith("generic_"): - fail("expected platform name of the form 'generic_', but was " + str(platforms)) + elif not platform_name.startswith("android_"): + fail("expected platform name of the form 'android_', but was " + str(platforms)) return "UNKNOWN" - return platform_name[len("generic_"):] + return platform_name[len("android_"):] def format(target): id_string = str(target.label) + "|" + get_arch(target) diff --git a/bazel/properties.go b/bazel/properties.go index 1763f2dc0..250fea4cd 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -80,10 +80,19 @@ func UniqueBazelLabelList(originalLabelList LabelList) LabelList { } const ( - ARCH_X86 = "x86" - ARCH_X86_64 = "x86_64" + // ArchType names in arch.go ARCH_ARM = "arm" ARCH_ARM64 = "arm64" + ARCH_X86 = "x86" + ARCH_X86_64 = "x86_64" + + // OsType names in arch.go + OS_ANDROID = "android" + OS_DARWIN = "darwin" + OS_FUCHSIA = "fuchsia" + OS_LINUX = "linux_glibc" + OS_LINUX_BIONIC = "linux_bionic" + OS_WINDOWS = "windows" ) var ( @@ -92,6 +101,36 @@ var ( // android package depends on the bazel package, so a cyclic dependency // prevents using that here. selectableArchs = []string{ARCH_X86, ARCH_X86_64, ARCH_ARM, ARCH_ARM64} + + // Likewise, this is the list of target operating systems. + selectableTargetOs = []string{ + OS_ANDROID, + OS_DARWIN, + OS_FUCHSIA, + OS_LINUX, + OS_LINUX_BIONIC, + OS_WINDOWS, + } + + // 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", + } + + // 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", + } ) // Arch-specific label_list typed Bazel attribute values. This should correspond @@ -101,8 +140,16 @@ type labelListArchValues struct { X86_64 LabelList Arm LabelList Arm64 LabelList - // TODO(b/181299724): this is currently missing the "common" arch, which - // doesn't have an equivalent platform() definition yet. + Common LabelList +} + +type labelListOsValues struct { + Android LabelList + Darwin LabelList + Fuchsia LabelList + Linux LabelList + LinuxBionic LabelList + Windows LabelList } // LabelListAttribute is used to represent a list of Bazel labels as an @@ -115,6 +162,11 @@ type LabelListAttribute struct { // are generated in a select statement and appended to the non-arch specific // label list Value. ArchValues labelListArchValues + + // The os-specific attribute label list values. Optional. If used, these + // are generated in a select statement and appended to the non-os specific + // label list Value. + OsValues labelListOsValues } // MakeLabelListAttribute initializes a LabelListAttribute with the non-arch specific value. @@ -124,45 +176,75 @@ func MakeLabelListAttribute(value LabelList) LabelListAttribute { // HasArchSpecificValues returns true if the attribute contains // architecture-specific label_list values. -func (attrs *LabelListAttribute) HasArchSpecificValues() bool { +func (attrs *LabelListAttribute) HasConfigurableValues() bool { for _, arch := range selectableArchs { - if len(attrs.GetValueForArch(arch).Includes) > 0 || len(attrs.GetValueForArch(arch).Excludes) > 0 { + if len(attrs.GetValueForArch(arch).Includes) > 0 { + return true + } + } + + for _, os := range selectableTargetOs { + if len(attrs.GetValueForOS(os).Includes) > 0 { return true } } return false } +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, + } +} + // GetValueForArch returns the label_list attribute value for an architecture. func (attrs *LabelListAttribute) GetValueForArch(arch string) LabelList { - switch arch { - case ARCH_X86: - return attrs.ArchValues.X86 - case ARCH_X86_64: - return attrs.ArchValues.X86_64 - case ARCH_ARM: - return attrs.ArchValues.Arm - case ARCH_ARM64: - return attrs.ArchValues.Arm64 - default: + var v *LabelList + if v = attrs.archValuePtrs()[arch]; v == nil { panic(fmt.Errorf("Unknown arch: %s", arch)) } + return *v } // SetValueForArch sets the label_list attribute value for an architecture. func (attrs *LabelListAttribute) SetValueForArch(arch string, value LabelList) { - switch arch { - case "x86": - attrs.ArchValues.X86 = value - case "x86_64": - attrs.ArchValues.X86_64 = value - case "arm": - attrs.ArchValues.Arm = value - case "arm64": - attrs.ArchValues.Arm64 = value - default: + var v *LabelList + if v = attrs.archValuePtrs()[arch]; v == nil { panic(fmt.Errorf("Unknown arch: %s", arch)) } + *v = value +} + +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, + } +} + +// GetValueForOS returns the label_list attribute value for an OS target. +func (attrs *LabelListAttribute) GetValueForOS(os string) LabelList { + var v *LabelList + if v = attrs.osValuePtrs()[os]; v == nil { + panic(fmt.Errorf("Unknown os: %s", os)) + } + return *v +} + +// SetValueForArch sets the label_list attribute value for an OS target. +func (attrs *LabelListAttribute) SetValueForOS(os string, value LabelList) { + var v *LabelList + if v = attrs.osValuePtrs()[os]; v == nil { + panic(fmt.Errorf("Unknown os: %s", os)) + } + *v = value } // StringListAttribute corresponds to the string_list Bazel attribute type with @@ -182,13 +264,12 @@ type stringListArchValues struct { X86_64 []string Arm []string Arm64 []string - // TODO(b/181299724): this is currently missing the "common" arch, which - // doesn't have an equivalent platform() definition yet. + Common []string } -// HasArchSpecificValues returns true if the attribute contains +// HasConfigurableValues returns true if the attribute contains // architecture-specific string_list values. -func (attrs *StringListAttribute) HasArchSpecificValues() bool { +func (attrs *StringListAttribute) HasConfigurableValues() bool { for _, arch := range selectableArchs { if len(attrs.GetValueForArch(arch)) > 0 { return true @@ -197,36 +278,31 @@ func (attrs *StringListAttribute) HasArchSpecificValues() bool { return false } +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, + } +} + // GetValueForArch returns the string_list attribute value for an architecture. func (attrs *StringListAttribute) GetValueForArch(arch string) []string { - switch arch { - case ARCH_X86: - return attrs.ArchValues.X86 - case ARCH_X86_64: - return attrs.ArchValues.X86_64 - case ARCH_ARM: - return attrs.ArchValues.Arm - case ARCH_ARM64: - return attrs.ArchValues.Arm64 - default: + var v *[]string + if v = attrs.archValuePtrs()[arch]; v == nil { panic(fmt.Errorf("Unknown arch: %s", arch)) } + return *v } // SetValueForArch sets the string_list attribute value for an architecture. func (attrs *StringListAttribute) SetValueForArch(arch string, value []string) { - switch arch { - case ARCH_X86: - attrs.ArchValues.X86 = value - case ARCH_X86_64: - attrs.ArchValues.X86_64 = value - case ARCH_ARM: - attrs.ArchValues.Arm = value - case ARCH_ARM64: - attrs.ArchValues.Arm64 = value - default: + var v *[]string + if v = attrs.archValuePtrs()[arch]; v == nil { panic(fmt.Errorf("Unknown arch: %s", arch)) } + *v = value } // TryVariableSubstitution, replace string substitution formatting within each string in slice with diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index e93b3dca1..1d254c855 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -416,63 +416,11 @@ func prettyPrint(propertyValue reflect.Value, indent int) (string, error) { // Special cases where the bp2build sends additional information to the codegenerator // by wrapping the attributes in a custom struct type. if labels, ok := propertyValue.Interface().(bazel.LabelListAttribute); ok { - // TODO(b/165114590): convert glob syntax - ret, err := prettyPrint(reflect.ValueOf(labels.Value.Includes), indent) - if err != nil { - return ret, err - } - - if !labels.HasArchSpecificValues() { - // Select statement not needed. - return ret, nil - } - - ret += " + " + "select({\n" - for _, arch := range android.ArchTypeList() { - value := labels.GetValueForArch(arch.Name) - if len(value.Includes) > 0 { - ret += makeIndent(indent + 1) - list, _ := prettyPrint(reflect.ValueOf(value.Includes), indent+1) - ret += fmt.Sprintf("\"%s\": %s,\n", platformArchMap[arch], list) - } - } - - ret += makeIndent(indent + 1) - ret += fmt.Sprintf("\"%s\": [],\n", "//conditions:default") - - ret += makeIndent(indent) - ret += "})" - return ret, err + return prettyPrintLabelListAttribute(labels, indent) } else if label, ok := propertyValue.Interface().(bazel.Label); ok { return fmt.Sprintf("%q", label.Label), nil } else if stringList, ok := propertyValue.Interface().(bazel.StringListAttribute); ok { - // A Bazel string_list attribute that may contain a select statement. - ret, err := prettyPrint(reflect.ValueOf(stringList.Value), indent) - if err != nil { - return ret, err - } - - if !stringList.HasArchSpecificValues() { - // Select statement not needed. - return ret, nil - } - - ret += " + " + "select({\n" - for _, arch := range android.ArchTypeList() { - value := stringList.GetValueForArch(arch.Name) - if len(value) > 0 { - ret += makeIndent(indent + 1) - list, _ := prettyPrint(reflect.ValueOf(value), indent+1) - ret += fmt.Sprintf("\"%s\": %s,\n", platformArchMap[arch], list) - } - } - - ret += makeIndent(indent + 1) - ret += fmt.Sprintf("\"%s\": [],\n", "//conditions:default") - - ret += makeIndent(indent) - ret += "})" - return ret, err + return prettyPrintStringListAttribute(stringList, indent) } ret = "{\n" diff --git a/bp2build/cc_library_headers_conversion_test.go b/bp2build/cc_library_headers_conversion_test.go index 049f84a02..d828168ac 100644 --- a/bp2build/cc_library_headers_conversion_test.go +++ b/bp2build/cc_library_headers_conversion_test.go @@ -110,13 +110,11 @@ func TestCcLibraryHeadersBp2Build(t *testing.T) { cc_library_headers { name: "lib-1", export_include_dirs: ["lib-1"], - bazel_module: { bp2build_available: true }, } cc_library_headers { name: "lib-2", export_include_dirs: ["lib-2"], - bazel_module: { bp2build_available: true }, } cc_library_headers { @@ -125,7 +123,6 @@ cc_library_headers { header_libs: ["lib-1", "lib-2"], // TODO: Also support export_header_lib_headers - bazel_module: { bp2build_available: true }, }`, expectedBazelTargets: []string{`cc_library_headers( name = "foo_headers", @@ -161,6 +158,106 @@ cc_library_headers { includes = [ "lib-2", ], +)`}, + }, + { + description: "cc_library_headers test with os-specific header_libs props", + moduleTypeUnderTest: "cc_library_headers", + moduleTypeUnderTestFactory: cc.LibraryHeaderFactory, + moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryHeadersBp2Build, + depsMutators: []android.RegisterMutatorFunc{cc.RegisterDepsBp2Build}, + filesystem: map[string]string{}, + bp: soongCcLibraryPreamble + ` +cc_library_headers { name: "android-lib" } +cc_library_headers { name: "base-lib" } +cc_library_headers { name: "darwin-lib" } +cc_library_headers { name: "fuchsia-lib" } +cc_library_headers { name: "linux-lib" } +cc_library_headers { name: "linux_bionic-lib" } +cc_library_headers { name: "windows-lib" } +cc_library_headers { + name: "foo_headers", + header_libs: ["base-lib"], + target: { + android: { header_libs: ["android-lib"] }, + darwin: { header_libs: ["darwin-lib"] }, + fuchsia: { header_libs: ["fuchsia-lib"] }, + linux_bionic: { header_libs: ["linux_bionic-lib"] }, + linux_glibc: { header_libs: ["linux-lib"] }, + windows: { header_libs: ["windows-lib"] }, + }, + bazel_module: { bp2build_available: true }, +}`, + expectedBazelTargets: []string{`cc_library_headers( + name = "android-lib", +)`, `cc_library_headers( + name = "base-lib", +)`, `cc_library_headers( + name = "darwin-lib", +)`, `cc_library_headers( + name = "foo_headers", + deps = [ + ":base-lib", + ] + select({ + "//build/bazel/platforms/os:android": [ + ":android-lib", + ], + "//build/bazel/platforms/os:darwin": [ + ":darwin-lib", + ], + "//build/bazel/platforms/os:fuchsia": [ + ":fuchsia-lib", + ], + "//build/bazel/platforms/os:linux": [ + ":linux-lib", + ], + "//build/bazel/platforms/os:linux_bionic": [ + ":linux_bionic-lib", + ], + "//build/bazel/platforms/os:windows": [ + ":windows-lib", + ], + "//conditions:default": [], + }), +)`, `cc_library_headers( + name = "fuchsia-lib", +)`, `cc_library_headers( + name = "linux-lib", +)`, `cc_library_headers( + name = "linux_bionic-lib", +)`, `cc_library_headers( + name = "windows-lib", +)`}, + }, + { + description: "cc_library_headers test with os-specific header_libs and export_header_lib_headers props", + moduleTypeUnderTest: "cc_library_headers", + moduleTypeUnderTestFactory: cc.LibraryHeaderFactory, + moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryHeadersBp2Build, + depsMutators: []android.RegisterMutatorFunc{cc.RegisterDepsBp2Build}, + filesystem: map[string]string{}, + bp: soongCcLibraryPreamble + ` +cc_library_headers { name: "android-lib" } +cc_library_headers { name: "exported-lib" } +cc_library_headers { + name: "foo_headers", + target: { + android: { header_libs: ["android-lib"], export_header_lib_headers: ["exported-lib"] }, + }, +}`, + expectedBazelTargets: []string{`cc_library_headers( + name = "android-lib", +)`, `cc_library_headers( + name = "exported-lib", +)`, `cc_library_headers( + name = "foo_headers", + deps = [] + select({ + "//build/bazel/platforms/os:android": [ + ":android-lib", + ":exported-lib", + ], + "//conditions:default": [], + }), )`}, }, } @@ -180,6 +277,9 @@ cc_library_headers { config := android.TestConfig(buildDir, nil, testCase.bp, filesystem) ctx := android.NewTestContext(config) + // TODO(jingwen): make this default for all bp2build tests + ctx.RegisterBp2BuildConfig(bp2buildConfig) + cc.RegisterCCBuildComponents(ctx) ctx.RegisterModuleType("toolchain_library", cc.ToolchainLibraryFactory) diff --git a/bp2build/cc_object_conversion_test.go b/bp2build/cc_object_conversion_test.go index 946173927..fcc308030 100644 --- a/bp2build/cc_object_conversion_test.go +++ b/bp2build/cc_object_conversion_test.go @@ -324,7 +324,7 @@ func TestCcObjectConfigurableAttributesBp2Build(t *testing.T) { copts = [ "-fno-addrsig", ] + select({ - "@bazel_tools//platforms:x86_32": [ + "//build/bazel/platforms/arch:x86": [ "-fPIC", ], "//conditions:default": [], @@ -335,7 +335,7 @@ func TestCcObjectConfigurableAttributesBp2Build(t *testing.T) { srcs = [ "a.cpp", ] + select({ - "@bazel_tools//platforms:arm": [ + "//build/bazel/platforms/arch:arm": [ "arch/arm/file.S", ], "//conditions:default": [], @@ -378,16 +378,16 @@ func TestCcObjectConfigurableAttributesBp2Build(t *testing.T) { copts = [ "-fno-addrsig", ] + select({ - "@bazel_tools//platforms:arm": [ + "//build/bazel/platforms/arch:arm": [ "-Wall", ], - "@bazel_tools//platforms:aarch64": [ + "//build/bazel/platforms/arch:arm64": [ "-Wall", ], - "@bazel_tools//platforms:x86_32": [ + "//build/bazel/platforms/arch:x86": [ "-fPIC", ], - "@bazel_tools//platforms:x86_64": [ + "//build/bazel/platforms/arch:x86_64": [ "-fPIC", ], "//conditions:default": [], @@ -398,16 +398,16 @@ func TestCcObjectConfigurableAttributesBp2Build(t *testing.T) { srcs = [ "base.cpp", ] + select({ - "@bazel_tools//platforms:arm": [ + "//build/bazel/platforms/arch:arm": [ "arm.cpp", ], - "@bazel_tools//platforms:aarch64": [ + "//build/bazel/platforms/arch:arm64": [ "arm64.cpp", ], - "@bazel_tools//platforms:x86_32": [ + "//build/bazel/platforms/arch:x86": [ "x86.cpp", ], - "@bazel_tools//platforms:x86_64": [ + "//build/bazel/platforms/arch:x86_64": [ "x86_64.cpp", ], "//conditions:default": [], diff --git a/bp2build/configurability.go b/bp2build/configurability.go index 47cf3c612..6ca0d6db1 100644 --- a/bp2build/configurability.go +++ b/bp2build/configurability.go @@ -1,15 +1,112 @@ package bp2build -import "android/soong/android" +import ( + "android/soong/android" + "android/soong/bazel" + "fmt" + "reflect" +) // Configurability support for bp2build. -var ( - // A map of architectures to the Bazel label of the constraint_value. - platformArchMap = map[android.ArchType]string{ - android.Arm: "@bazel_tools//platforms:arm", - android.Arm64: "@bazel_tools//platforms:aarch64", - android.X86: "@bazel_tools//platforms:x86_32", - android.X86_64: "@bazel_tools//platforms:x86_64", +// prettyPrintStringListAttribute converts a StringListAttribute to its Bazel +// syntax. May contain a select statement. +func prettyPrintStringListAttribute(stringList bazel.StringListAttribute, indent int) (string, error) { + ret, err := prettyPrint(reflect.ValueOf(stringList.Value), indent) + if err != nil { + return ret, err } -) + + if !stringList.HasConfigurableValues() { + // Select statement not needed. + return ret, nil + } + + // Create the selects for arch specific values. + selects := map[string]reflect.Value{} + for arch, selectKey := range bazel.PlatformArchMap { + selects[selectKey] = reflect.ValueOf(stringList.GetValueForArch(arch)) + } + + selectMap, err := prettyPrintSelectMap(selects, "[]", indent) + return ret + selectMap, err +} + +// prettyPrintLabelListAttribute converts a LabelListAttribute to its Bazel +// syntax. May contain select statements. +func prettyPrintLabelListAttribute(labels bazel.LabelListAttribute, indent int) (string, error) { + // TODO(b/165114590): convert glob syntax + ret, err := prettyPrint(reflect.ValueOf(labels.Value.Includes), indent) + if err != nil { + return ret, err + } + + if !labels.HasConfigurableValues() { + // Select statements not needed. + return ret, nil + } + + // Create the selects for arch specific values. + archSelects := map[string]reflect.Value{} + for arch, selectKey := range bazel.PlatformArchMap { + archSelects[selectKey] = reflect.ValueOf(labels.GetValueForArch(arch).Includes) + } + selectMap, err := prettyPrintSelectMap(archSelects, "[]", indent) + if err != nil { + return "", err + } + ret += selectMap + + // Create the selects for target os specific values. + osSelects := map[string]reflect.Value{} + for os, selectKey := range bazel.PlatformOsMap { + osSelects[selectKey] = reflect.ValueOf(labels.GetValueForOS(os).Includes) + } + selectMap, err = prettyPrintSelectMap(osSelects, "[]", indent) + return ret + selectMap, err +} + +// prettyPrintSelectMap converts a map of select keys to reflected Values as a generic way +// to construct a select map for any kind of attribute type. +func prettyPrintSelectMap(selectMap map[string]reflect.Value, defaultValue string, indent int) (string, error) { + var selects string + for _, selectKey := range android.SortedStringKeys(selectMap) { + value := selectMap[selectKey] + if isZero(value) { + // Ignore zero values to not generate empty lists. + continue + } + s, err := prettyPrintSelectEntry(value, selectKey, indent) + if err != nil { + return "", err + } + selects += s + ",\n" + } + + if len(selects) == 0 { + // No conditions (or all values are empty lists), so no need for a map. + return "", nil + } + + // 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) + ret += makeIndent(indent) + ret += "})" + + return ret, nil +} + +// prettyPrintSelectEntry converts a reflect.Value into an entry in a select map +// with a provided key. +func prettyPrintSelectEntry(value reflect.Value, key string, indent int) (string, error) { + s := makeIndent(indent + 1) + v, err := prettyPrint(value, indent+1) + if err != nil { + return "", err + } + s += fmt.Sprintf("\"%s\": %s", key, v) + return s, nil +} diff --git a/bp2build/testing.go b/bp2build/testing.go index ede804413..ef3a78fd3 100644 --- a/bp2build/testing.go +++ b/bp2build/testing.go @@ -5,6 +5,13 @@ import ( "android/soong/bazel" ) +var ( + // A default configuration for tests to not have to specify bp2build_available on top level targets. + bp2buildConfig = android.Bp2BuildConfig{ + android.BP2BUILD_TOPLEVEL: android.Bp2BuildDefaultTrueRecursively, + } +) + type nestedProps struct { Nested_prop string } diff --git a/cc/Android.bp b/cc/Android.bp index 79e92cb23..cc4d9bc45 100644 --- a/cc/Android.bp +++ b/cc/Android.bp @@ -20,6 +20,7 @@ bootstrap_go_package { "androidmk.go", "api_level.go", "builder.go", + "bp2build.go", "cc.go", "ccdeps.go", "check.go", diff --git a/cc/bp2build.go b/cc/bp2build.go new file mode 100644 index 000000000..2a590eb84 --- /dev/null +++ b/cc/bp2build.go @@ -0,0 +1,106 @@ +// Copyright 2021 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package cc + +import ( + "android/soong/android" + "android/soong/bazel" +) + +// bp2build functions and helpers for converting cc_* modules to Bazel. + +func init() { + android.DepsBp2BuildMutators(RegisterDepsBp2Build) +} + +func RegisterDepsBp2Build(ctx android.RegisterMutatorsContext) { + ctx.BottomUp("cc_bp2build_deps", depsBp2BuildMutator) +} + +// A naive deps mutator to add deps on all modules across all combinations of +// target props for cc modules. This is needed to make module -> bazel label +// resolution work in the bp2build mutator later. This is probably +// the wrong way to do it, but it works. +// +// TODO(jingwen): can we create a custom os mutator in depsBp2BuildMutator to do this? +func depsBp2BuildMutator(ctx android.BottomUpMutatorContext) { + module, ok := ctx.Module().(*Module) + if !ok { + // Not a cc module + return + } + + if !module.ConvertWithBp2build(ctx) { + return + } + + var allDeps []string + + for _, p := range module.GetTargetProperties(&BaseLinkerProperties{}) { + // arch specific linker props + if baseLinkerProps, ok := p.(*BaseLinkerProperties); ok { + allDeps = append(allDeps, baseLinkerProps.Header_libs...) + allDeps = append(allDeps, baseLinkerProps.Export_header_lib_headers...) + } + } + + ctx.AddDependency(module, nil, android.SortedUniqueStrings(allDeps)...) +} + +// bp2BuildParseHeaderLibs creates a label list attribute containing the header library deps of a module, including +// configurable attribute values. +func bp2BuildParseHeaderLibs(ctx android.TopDownMutatorContext, module *Module) bazel.LabelListAttribute { + var ret bazel.LabelListAttribute + for _, linkerProps := range module.linker.linkerProps() { + if baseLinkerProps, ok := linkerProps.(*BaseLinkerProperties); ok { + libs := baseLinkerProps.Header_libs + libs = append(libs, baseLinkerProps.Export_header_lib_headers...) + ret = bazel.MakeLabelListAttribute( + android.BazelLabelForModuleDeps(ctx, android.SortedUniqueStrings(libs))) + break + } + } + + for os, p := range module.GetTargetProperties(&BaseLinkerProperties{}) { + if baseLinkerProps, ok := p.(*BaseLinkerProperties); ok { + libs := baseLinkerProps.Header_libs + libs = append(libs, baseLinkerProps.Export_header_lib_headers...) + libs = android.SortedUniqueStrings(libs) + ret.SetValueForOS(os.Name, android.BazelLabelForModuleDeps(ctx, libs)) + } + } + + return ret +} + +// bp2BuildParseExportedIncludes creates a label list attribute contains the +// exported included directories of a module. +func bp2BuildParseExportedIncludes(ctx android.TopDownMutatorContext, module *Module) (bazel.LabelListAttribute, bazel.LabelListAttribute) { + libraryDecorator := module.linker.(*libraryDecorator) + + includeDirs := libraryDecorator.flagExporter.Properties.Export_system_include_dirs + includeDirs = append(includeDirs, libraryDecorator.flagExporter.Properties.Export_include_dirs...) + + includeDirsLabels := android.BazelLabelForModuleSrc(ctx, includeDirs) + + var includeDirGlobs []string + for _, includeDir := range includeDirs { + includeDirGlobs = append(includeDirGlobs, includeDir+"/**/*.h") + includeDirGlobs = append(includeDirGlobs, includeDir+"/**/*.inc") + includeDirGlobs = append(includeDirGlobs, includeDir+"/**/*.hpp") + } + + headersLabels := android.BazelLabelForModuleSrc(ctx, includeDirGlobs) + return bazel.MakeLabelListAttribute(includeDirsLabels), bazel.MakeLabelListAttribute(headersLabels) +} diff --git a/cc/library.go b/cc/library.go index 091acfe08..50fff7f9d 100644 --- a/cc/library.go +++ b/cc/library.go @@ -2046,38 +2046,6 @@ func maybeInjectBoringSSLHash(ctx android.ModuleContext, outputFile android.Modu return outputFile } -func Bp2BuildParseHeaderLibs(ctx android.TopDownMutatorContext, module *Module) bazel.LabelListAttribute { - var headerLibs []string - for _, linkerProps := range module.linker.linkerProps() { - if baseLinkerProps, ok := linkerProps.(*BaseLinkerProperties); ok { - headerLibs = baseLinkerProps.Header_libs - // FIXME: re-export include dirs from baseLinkerProps.Export_header_lib_headers? - break - } - } - headerLibsLabels := bazel.MakeLabelListAttribute(android.BazelLabelForModuleDeps(ctx, headerLibs)) - return headerLibsLabels -} - -func Bp2BuildParseExportedIncludes(ctx android.TopDownMutatorContext, module *Module) (bazel.LabelListAttribute, bazel.LabelListAttribute) { - libraryDecorator := module.linker.(*libraryDecorator) - - includeDirs := libraryDecorator.flagExporter.Properties.Export_system_include_dirs - includeDirs = append(includeDirs, libraryDecorator.flagExporter.Properties.Export_include_dirs...) - - includeDirsLabels := android.BazelLabelForModuleSrc(ctx, includeDirs) - - var includeDirGlobs []string - for _, includeDir := range includeDirs { - includeDirGlobs = append(includeDirGlobs, includeDir+"/**/*.h") - includeDirGlobs = append(includeDirGlobs, includeDir+"/**/*.inc") - includeDirGlobs = append(includeDirGlobs, includeDir+"/**/*.hpp") - } - - headersLabels := android.BazelLabelForModuleSrc(ctx, includeDirGlobs) - return bazel.MakeLabelListAttribute(includeDirsLabels), bazel.MakeLabelListAttribute(headersLabels) -} - type bazelCcLibraryStaticAttributes struct { Copts []string Srcs bazel.LabelListAttribute @@ -2149,10 +2117,10 @@ func CcLibraryStaticBp2Build(ctx android.TopDownMutatorContext) { allIncludes = append(allIncludes, localIncludeDirs...) includesLabels := android.BazelLabelForModuleSrc(ctx, allIncludes) - exportedIncludesLabels, exportedIncludesHeadersLabels := Bp2BuildParseExportedIncludes(ctx, module) + exportedIncludesLabels, exportedIncludesHeadersLabels := bp2BuildParseExportedIncludes(ctx, module) includesLabels.Append(exportedIncludesLabels.Value) - headerLibsLabels := Bp2BuildParseHeaderLibs(ctx, module) + headerLibsLabels := bp2BuildParseHeaderLibs(ctx, module) depsLabels.Append(headerLibsLabels.Value) attrs := &bazelCcLibraryStaticAttributes{ diff --git a/cc/library_headers.go b/cc/library_headers.go index 82868484a..c8dd2c26e 100644 --- a/cc/library_headers.go +++ b/cc/library_headers.go @@ -94,9 +94,9 @@ func CcLibraryHeadersBp2Build(ctx android.TopDownMutatorContext) { return } - exportedIncludesLabels, exportedIncludesHeadersLabels := Bp2BuildParseExportedIncludes(ctx, module) + exportedIncludesLabels, exportedIncludesHeadersLabels := bp2BuildParseExportedIncludes(ctx, module) - headerLibsLabels := Bp2BuildParseHeaderLibs(ctx, module) + headerLibsLabels := bp2BuildParseHeaderLibs(ctx, module) attrs := &bazelCcLibraryHeadersAttributes{ Includes: exportedIncludesLabels,