From c194ffbcf39db04af3800c9240b400e6c2bac0b2 Mon Sep 17 00:00:00 2001 From: Rupert Shuttleworth Date: Wed, 19 May 2021 06:49:02 -0400 Subject: [PATCH] Make GetTargetProperties() aware of more complex targets, like 'android_arm', instead of just 'android'. Test: Added new unit test and updated existing tests. Test: bazel build //bionic/... //external/... //frameworks/... //system/... Test: ./build/bazel/scripts/run_presubmits.sh Change-Id: I250d1964f5cf42b92ddb929379d35d8c844423f7 --- android/arch.go | 103 ++++-- android/variable.go | 5 +- bazel/properties.go | 331 ++++++++++++++---- bp2build/build_conversion.go | 3 + bp2build/cc_library_static_conversion_test.go | 60 ++++ bp2build/configurability.go | 37 +- cc/bp2build.go | 157 ++++++--- 7 files changed, 534 insertions(+), 162 deletions(-) diff --git a/android/arch.go b/android/arch.go index 10c827ba2..9ff439ccc 100644 --- a/android/arch.go +++ b/android/arch.go @@ -897,7 +897,7 @@ func createArchPropTypeDesc(props reflect.Type) []archPropTypeDesc { // Add the OS/Arch combinations, e.g. "android_arm64". for _, archType := range osArchTypeMap[os] { - targets = append(targets, os.Field+"_"+archType.Name) + targets = append(targets, GetCompoundTargetName(os, archType)) // Also add the special "linux_" and "bionic_" property structs. if os.Linux() { @@ -1217,6 +1217,10 @@ func getMultilibStruct(ctx ArchVariantContext, archProperties interface{}, archT return getChildPropertyStruct(ctx, multilibProp, archType.Multilib, "multilib."+archType.Multilib) } +func GetCompoundTargetName(os OsType, arch ArchType) string { + return os.Field + "_" + arch.Name +} + // Returns the structs corresponding to the properties specific to the given // architecture and OS in archProperties. func getArchProperties(ctx BaseMutatorContext, archProperties interface{}, arch Arch, os OsType, nativeBridgeEnabled bool) []reflect.Value { @@ -1323,7 +1327,7 @@ func getArchProperties(ctx BaseMutatorContext, archProperties interface{}, arch // key: value, // }, // }, - field := os.Field + "_" + archType.Name + field := GetCompoundTargetName(os, archType) userFriendlyField := "target." + os.Name + "_" + archType.Name if osArchProperties, ok := getChildPropertyStruct(ctx, targetProp, field, userFriendlyField); ok { result = append(result, osArchProperties) @@ -1950,19 +1954,47 @@ func (m *ModuleBase) GetArchProperties(ctx ArchVariantContext, propertySet inter return archToProp } -// Returns the struct containing the properties specific to the given -// architecture type. These look like this in Blueprint files: -// target: { -// android: { -// key: value, -// }, -// }, -// This struct will also contain sub-structs containing to the architecture/CPU -// variants and features that themselves contain properties specific to those. -func getTargetStruct(ctx ArchVariantContext, archProperties interface{}, os OsType) (reflect.Value, bool) { - archPropValues := reflect.ValueOf(archProperties).Elem() - targetProp := archPropValues.FieldByName("Target").Elem() - return getChildPropertyStruct(ctx, targetProp, os.Field, os.Field) +// Returns a struct matching the propertySet interface, containing properties specific to the targetName +// For example, given these arguments: +// propertySet = BaseCompilerProperties +// targetName = "android_arm" +// And given this Android.bp fragment: +// target: +// android_arm: { +// srcs: ["foo.c"], +// } +// android_arm64: { +// srcs: ["bar.c"], +// } +// } +// This would return a BaseCompilerProperties with BaseCompilerProperties.Srcs = ["foo.c"] +func getTargetStruct(ctx ArchVariantContext, propertySet interface{}, archProperties []interface{}, targetName string) interface{} { + propertyStructs := make([]reflect.Value, 0) + for _, archProperty := range archProperties { + archPropValues := reflect.ValueOf(archProperty).Elem() + targetProp := archPropValues.FieldByName("Target").Elem() + targetStruct, ok := getChildPropertyStruct(ctx, targetProp, targetName, targetName) + if ok { + propertyStructs = append(propertyStructs, targetStruct) + } + } + + // Create a new instance of the requested property set + value := reflect.New(reflect.ValueOf(propertySet).Elem().Type()).Interface() + + // Merge all the structs together + for _, propertyStruct := range propertyStructs { + mergePropertyStruct(ctx, value, propertyStruct) + } + + return value +} + +// Properties corresponds to e.g. Target: android: {...} +// ArchProperties corresponds to e.g. Target: android_arm: {...}, android_arm64: {...}, ... +type TargetProperties struct { + Properties interface{} + ArchProperties map[ArchType]interface{} } // GetTargetProperties returns a map of OS target (e.g. android, windows) to the @@ -1974,13 +2006,15 @@ func getTargetStruct(ctx ArchVariantContext, archProperties interface{}, os OsTy // the os-specific property value specified by the module if defined. // // Implemented in a way very similar to GetArchProperties(). -func (m *ModuleBase) GetTargetProperties(ctx ArchVariantContext, propertySet interface{}) map[OsType]interface{} { - // Return value of the arch types to the prop values for that arch. - osToProp := map[OsType]interface{}{} +// +// NOTE: "Target" == OS +func (m *ModuleBase) GetTargetProperties(ctx ArchVariantContext, propertySet interface{}) map[OsType]TargetProperties { + // Return value of the target types to the prop values for that target. + targetToProp := map[OsType]TargetProperties{} - // Nothing to do for non-OS/arch-specific modules. + // Nothing to do for non-target-specific modules. if !m.ArchSpecific() { - return osToProp + return targetToProp } dstType := reflect.ValueOf(propertySet).Type() @@ -1998,33 +2032,26 @@ func (m *ModuleBase) GetTargetProperties(ctx ArchVariantContext, propertySet int if archProperties == nil { // This module does not have the property set requested - return osToProp + return targetToProp } + // For android, linux, ... for _, os := range osTypeList { if os == CommonOS { // It looks like this OS value is not used in Blueprint files continue } - - propertyStructs := make([]reflect.Value, 0) - for _, archProperty := range archProperties { - targetStruct, ok := getTargetStruct(ctx, archProperty, os) - if ok { - propertyStructs = append(propertyStructs, targetStruct) - } + targetProperties := TargetProperties{ + Properties: getTargetStruct(ctx, propertySet, archProperties, os.Field), + ArchProperties: make(map[ArchType]interface{}), } - - // Create a new instance of the requested property set - value := reflect.New(reflect.ValueOf(propertySet).Elem().Type()).Interface() - - // Merge all the structs together - for _, propertyStruct := range propertyStructs { - mergePropertyStruct(ctx, value, propertyStruct) + // For arm, x86, ... + for _, arch := range osArchTypeMap[os] { + targetName := GetCompoundTargetName(os, arch) + targetProperties.ArchProperties[arch] = getTargetStruct(ctx, propertySet, archProperties, targetName) } - - osToProp[os] = value + targetToProp[os] = targetProperties } - return osToProp + return targetToProp } diff --git a/android/variable.go b/android/variable.go index cf7493369..7658cdd4a 100644 --- a/android/variable.go +++ b/android/variable.go @@ -488,7 +488,10 @@ func ProductVariableProperties(ctx BaseMutatorContext) ProductConfigProperties { for os, targetProps := range moduleBase.GetTargetProperties(ctx, moduleBase.variableProperties) { // GetTargetProperties is creating an instance of the requested type // and productVariablesValues expects an interface, so no need to cast - productVariableValues(targetProps, os.Name, &productConfigProperties) + productVariableValues(targetProps.Properties, os.Name, &productConfigProperties) + for arch, archProperties := range targetProps.ArchProperties { + productVariableValues(archProperties, os.Name+"_"+arch.Name, &productConfigProperties) + } } return productConfigProperties diff --git a/bazel/properties.go b/bazel/properties.go index 6ecf6caa6..8adc9d07f 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -160,7 +160,12 @@ func FilterLabelListAttribute(haystack LabelListAttribute, needleFn func(string) } for os := range PlatformOsMap { - result.SetValueForOS(os, FilterLabelList(haystack.GetValueForOS(os), needleFn)) + result.SetOsValueForTarget(os, FilterLabelList(haystack.GetOsValueForTarget(os), needleFn)) + + // TODO(b/187530594): Should we handle arch=CONDITIONS_DEFAULT here? (not in ArchValues) + for _, arch := range AllArches { + result.SetOsArchValueForTarget(os, arch, FilterLabelList(haystack.GetOsArchValueForTarget(os, arch), needleFn)) + } } return result @@ -176,8 +181,12 @@ func SubtractBazelLabelListAttribute(haystack LabelListAttribute, needle LabelLi } for os := range PlatformOsMap { - result.SetValueForOS(os, - SubtractBazelLabelList(haystack.GetValueForOS(os), needle.GetValueForOS(os))) + result.SetOsValueForTarget(os, SubtractBazelLabelList(haystack.GetOsValueForTarget(os), needle.GetOsValueForTarget(os))) + + // TODO(b/187530594): Should we handle arch=CONDITIONS_DEFAULT here? (not in ArchValues) + for _, arch := range AllArches { + result.SetOsArchValueForTarget(os, arch, SubtractBazelLabelList(haystack.GetOsArchValueForTarget(os, arch), needle.GetOsArchValueForTarget(os, arch))) + } } result.Value = SubtractBazelLabelList(haystack.Value, needle.Value) @@ -241,6 +250,21 @@ const ( OS_LINUX_BIONIC = "linux_bionic" OS_WINDOWS = "windows" + // Targets in arch.go + TARGET_ANDROID_ARM = "android_arm" + TARGET_ANDROID_ARM64 = "android_arm64" + TARGET_ANDROID_X86 = "android_x86" + TARGET_ANDROID_X86_64 = "android_x86_64" + TARGET_DARWIN_X86_64 = "darwin_x86_64" + TARGET_FUCHSIA_ARM64 = "fuchsia_arm64" + TARGET_FUCHSIA_X86_64 = "fuchsia_x86_64" + TARGET_LINUX_X86 = "linux_glibc_x86" + TARGET_LINUX_x86_64 = "linux_glibc_x86_64" + TARGET_LINUX_BIONIC_ARM64 = "linux_bionic_arm64" + TARGET_LINUX_BIONIC_X86_64 = "linux_bionic_x86_64" + TARGET_WINDOWS_X86 = "windows_x86" + TARGET_WINDOWS_X86_64 = "windows_x86_64" + // 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. @@ -282,6 +306,26 @@ var ( OS_WINDOWS: "//build/bazel/platforms/os:windows", CONDITIONS_DEFAULT: ConditionsDefaultSelectKey, // The default condition of an os select map. } + + PlatformTargetMap = map[string]string{ + TARGET_ANDROID_ARM: "//build/bazel/platforms:android_arm", + TARGET_ANDROID_ARM64: "//build/bazel/platforms:android_arm64", + TARGET_ANDROID_X86: "//build/bazel/platforms:android_x86", + TARGET_ANDROID_X86_64: "//build/bazel/platforms:android_x86_64", + TARGET_DARWIN_X86_64: "//build/bazel/platforms:darwin_x86_64", + TARGET_FUCHSIA_ARM64: "//build/bazel/platforms:fuchsia_arm64", + TARGET_FUCHSIA_X86_64: "//build/bazel/platforms:fuchsia_x86_64", + TARGET_LINUX_X86: "//build/bazel/platforms:linux_glibc_x86", + TARGET_LINUX_x86_64: "//build/bazel/platforms:linux_glibc_x86_64", + TARGET_LINUX_BIONIC_ARM64: "//build/bazel/platforms:linux_bionic_arm64", + TARGET_LINUX_BIONIC_X86_64: "//build/bazel/platforms:linux_bionic_x86_64", + TARGET_WINDOWS_X86: "//build/bazel/platforms:windows_x86", + TARGET_WINDOWS_X86_64: "//build/bazel/platforms:windows_x86_64", + CONDITIONS_DEFAULT: ConditionsDefaultSelectKey, // The default condition of an os select map. + } + + // TODO(b/187530594): Should we add CONDITIONS_DEFAULT here? + AllArches = []string{ARCH_ARM, ARCH_ARM64, ARCH_X86, ARCH_X86_64} ) type Attribute interface { @@ -345,15 +389,32 @@ type labelListArchValues struct { ConditionsDefault LabelList } -type labelListOsValues struct { - Android LabelList - Darwin LabelList - Fuchsia LabelList - Linux LabelList - LinuxBionic LabelList - Windows LabelList +type labelListTargetValue struct { + // E.g. for android + OsValue LabelList - ConditionsDefault LabelList + // E.g. for android_arm, android_arm64, ... + ArchValues labelListArchValues +} + +func (target *labelListTargetValue) Append(other labelListTargetValue) { + target.OsValue.Append(other.OsValue) + target.ArchValues.X86.Append(other.ArchValues.X86) + target.ArchValues.X86_64.Append(other.ArchValues.X86_64) + target.ArchValues.Arm.Append(other.ArchValues.Arm) + target.ArchValues.Arm64.Append(other.ArchValues.Arm64) + target.ArchValues.ConditionsDefault.Append(other.ArchValues.ConditionsDefault) +} + +type labelListTargetValues struct { + Android labelListTargetValue + Darwin labelListTargetValue + Fuchsia labelListTargetValue + Linux labelListTargetValue + LinuxBionic labelListTargetValue + Windows labelListTargetValue + + ConditionsDefault labelListTargetValue } // LabelListAttribute is used to represent a list of Bazel labels as an @@ -370,7 +431,7 @@ type LabelListAttribute struct { // 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 + TargetValues labelListTargetValues } // MakeLabelListAttribute initializes a LabelListAttribute with the non-arch specific value. @@ -389,10 +450,10 @@ func (attrs *LabelListAttribute) Append(other LabelListAttribute) { } for os := range PlatformOsMap { - this := attrs.GetValueForOS(os) - that := other.GetValueForOS(os) + this := attrs.getValueForTarget(os) + that := other.getValueForTarget(os) this.Append(that) - attrs.SetValueForOS(os, this) + attrs.setValueForTarget(os, this) } attrs.Value.Append(other.Value) @@ -408,9 +469,15 @@ func (attrs LabelListAttribute) HasConfigurableValues() bool { } for os := range PlatformOsMap { - if len(attrs.GetValueForOS(os).Includes) > 0 { + if len(attrs.GetOsValueForTarget(os).Includes) > 0 { return true } + // TODO(b/187530594): Should we also check arch=CONDITIONS_DEFAULT (not in AllArches) + for _, arch := range AllArches { + if len(attrs.GetOsArchValueForTarget(os, arch).Includes) > 0 { + return true + } + } } return false } @@ -443,36 +510,92 @@ func (attrs *LabelListAttribute) SetValueForArch(arch string, value LabelList) { *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, - CONDITIONS_DEFAULT: &attrs.OsValues.ConditionsDefault, +func (attrs *LabelListAttribute) targetValuePtrs() map[string]*labelListTargetValue { + return map[string]*labelListTargetValue{ + OS_ANDROID: &attrs.TargetValues.Android, + OS_DARWIN: &attrs.TargetValues.Darwin, + OS_FUCHSIA: &attrs.TargetValues.Fuchsia, + OS_LINUX: &attrs.TargetValues.Linux, + OS_LINUX_BIONIC: &attrs.TargetValues.LinuxBionic, + OS_WINDOWS: &attrs.TargetValues.Windows, + CONDITIONS_DEFAULT: &attrs.TargetValues.ConditionsDefault, } } -// 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 { +func (attrs *LabelListAttribute) getValueForTarget(os string) labelListTargetValue { + var v *labelListTargetValue + if v = attrs.targetValuePtrs()[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 { +func (attrs *LabelListAttribute) GetOsValueForTarget(os string) LabelList { + var v *labelListTargetValue + if v = attrs.targetValuePtrs()[os]; v == nil { + panic(fmt.Errorf("Unknown os: %s", os)) + } + return v.OsValue +} + +func (attrs *LabelListAttribute) GetOsArchValueForTarget(os string, arch string) LabelList { + var v *labelListTargetValue + if v = attrs.targetValuePtrs()[os]; v == nil { + panic(fmt.Errorf("Unknown os: %s", os)) + } + switch arch { + case ARCH_X86: + return v.ArchValues.X86 + case ARCH_X86_64: + return v.ArchValues.X86_64 + case ARCH_ARM: + return v.ArchValues.Arm + case ARCH_ARM64: + return v.ArchValues.Arm64 + case CONDITIONS_DEFAULT: + return v.ArchValues.ConditionsDefault + default: + panic(fmt.Errorf("Unknown arch: %s\n", arch)) + } +} + +func (attrs *LabelListAttribute) setValueForTarget(os string, value labelListTargetValue) { + var v *labelListTargetValue + if v = attrs.targetValuePtrs()[os]; v == nil { panic(fmt.Errorf("Unknown os: %s", os)) } *v = value } +func (attrs *LabelListAttribute) SetOsValueForTarget(os string, value LabelList) { + var v *labelListTargetValue + if v = attrs.targetValuePtrs()[os]; v == nil { + panic(fmt.Errorf("Unknown os: %s", os)) + } + v.OsValue = value +} + +func (attrs *LabelListAttribute) SetOsArchValueForTarget(os string, arch string, value LabelList) { + var v *labelListTargetValue + if v = attrs.targetValuePtrs()[os]; v == nil { + panic(fmt.Errorf("Unknown os: %s", os)) + } + switch arch { + case ARCH_X86: + v.ArchValues.X86 = value + case ARCH_X86_64: + v.ArchValues.X86_64 = value + case ARCH_ARM: + v.ArchValues.Arm = value + case ARCH_ARM64: + v.ArchValues.Arm64 = value + case CONDITIONS_DEFAULT: + v.ArchValues.ConditionsDefault = value + default: + panic(fmt.Errorf("Unknown arch: %s\n", arch)) + } +} + // StringListAttribute corresponds to the string_list Bazel attribute type with // support for additional metadata, like configurations. type StringListAttribute struct { @@ -487,7 +610,7 @@ type StringListAttribute struct { // The os-specific attribute string list values. Optional. If used, these // are generated in a select statement and appended to the non-os specific // label list Value. - OsValues stringListOsValues + TargetValues stringListTargetValues // list of product-variable string list values. Optional. if used, each will generate a select // statement appended to the label list Value. @@ -512,15 +635,32 @@ type stringListArchValues struct { ConditionsDefault []string } -type stringListOsValues struct { - Android []string - Darwin []string - Fuchsia []string - Linux []string - LinuxBionic []string - Windows []string +type stringListTargetValue struct { + // E.g. for android + OsValue []string - ConditionsDefault []string + // E.g. for android_arm, android_arm64, ... + ArchValues stringListArchValues +} + +func (target *stringListTargetValue) Append(other stringListTargetValue) { + target.OsValue = append(target.OsValue, other.OsValue...) + target.ArchValues.X86 = append(target.ArchValues.X86, other.ArchValues.X86...) + target.ArchValues.X86_64 = append(target.ArchValues.X86_64, other.ArchValues.X86_64...) + target.ArchValues.Arm = append(target.ArchValues.Arm, other.ArchValues.Arm...) + target.ArchValues.Arm64 = append(target.ArchValues.Arm64, other.ArchValues.Arm64...) + target.ArchValues.ConditionsDefault = append(target.ArchValues.ConditionsDefault, other.ArchValues.ConditionsDefault...) +} + +type stringListTargetValues struct { + Android stringListTargetValue + Darwin stringListTargetValue + Fuchsia stringListTargetValue + Linux stringListTargetValue + LinuxBionic stringListTargetValue + Windows stringListTargetValue + + ConditionsDefault stringListTargetValue } // Product Variable values for StringListAttribute @@ -545,9 +685,16 @@ func (attrs StringListAttribute) HasConfigurableValues() bool { } for os := range PlatformOsMap { - if len(attrs.GetValueForOS(os)) > 0 { + if len(attrs.GetOsValueForTarget(os)) > 0 { return true } + // TODO(b/187530594): Should we also check arch=CONDITIONS_DEFAULT? (Not in AllArches) + for _, arch := range AllArches { + if len(attrs.GetOsArchValueForTarget(os, arch)) > 0 { + return true + } + + } } return len(attrs.ProductValues) > 0 @@ -581,31 +728,58 @@ func (attrs *StringListAttribute) SetValueForArch(arch string, value []string) { *v = value } -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, - CONDITIONS_DEFAULT: &attrs.OsValues.ConditionsDefault, +func (attrs *StringListAttribute) targetValuePtrs() map[string]*stringListTargetValue { + return map[string]*stringListTargetValue{ + OS_ANDROID: &attrs.TargetValues.Android, + OS_DARWIN: &attrs.TargetValues.Darwin, + OS_FUCHSIA: &attrs.TargetValues.Fuchsia, + OS_LINUX: &attrs.TargetValues.Linux, + OS_LINUX_BIONIC: &attrs.TargetValues.LinuxBionic, + OS_WINDOWS: &attrs.TargetValues.Windows, + CONDITIONS_DEFAULT: &attrs.TargetValues.ConditionsDefault, } } -// GetValueForOS returns the string_list attribute value for an OS target. -func (attrs *StringListAttribute) GetValueForOS(os string) []string { - var v *[]string - if v = attrs.osValuePtrs()[os]; v == nil { +func (attrs *StringListAttribute) getValueForTarget(os string) stringListTargetValue { + var v *stringListTargetValue + if v = attrs.targetValuePtrs()[os]; v == nil { panic(fmt.Errorf("Unknown os: %s", os)) } return *v } -// SetValueForArch sets the string_list attribute value for an OS target. -func (attrs *StringListAttribute) SetValueForOS(os string, value []string) { - var v *[]string - if v = attrs.osValuePtrs()[os]; v == nil { +func (attrs *StringListAttribute) GetOsValueForTarget(os string) []string { + var v *stringListTargetValue + if v = attrs.targetValuePtrs()[os]; v == nil { + panic(fmt.Errorf("Unknown os: %s", os)) + } + return v.OsValue +} + +func (attrs *StringListAttribute) GetOsArchValueForTarget(os string, arch string) []string { + var v *stringListTargetValue + if v = attrs.targetValuePtrs()[os]; v == nil { + panic(fmt.Errorf("Unknown os: %s", os)) + } + switch arch { + case ARCH_X86: + return v.ArchValues.X86 + case ARCH_X86_64: + return v.ArchValues.X86_64 + case ARCH_ARM: + return v.ArchValues.Arm + case ARCH_ARM64: + return v.ArchValues.Arm64 + case CONDITIONS_DEFAULT: + return v.ArchValues.ConditionsDefault + default: + panic(fmt.Errorf("Unknown arch: %s\n", arch)) + } +} + +func (attrs *StringListAttribute) setValueForTarget(os string, value stringListTargetValue) { + var v *stringListTargetValue + if v = attrs.targetValuePtrs()[os]; v == nil { panic(fmt.Errorf("Unknown os: %s", os)) } *v = value @@ -617,6 +791,35 @@ func (attrs *StringListAttribute) SortedProductVariables() []ProductVariableValu return vals } +func (attrs *StringListAttribute) SetOsValueForTarget(os string, value []string) { + var v *stringListTargetValue + if v = attrs.targetValuePtrs()[os]; v == nil { + panic(fmt.Errorf("Unknown os: %s", os)) + } + v.OsValue = value +} + +func (attrs *StringListAttribute) SetOsArchValueForTarget(os string, arch string, value []string) { + var v *stringListTargetValue + if v = attrs.targetValuePtrs()[os]; v == nil { + panic(fmt.Errorf("Unknown os: %s", os)) + } + switch arch { + case ARCH_X86: + v.ArchValues.X86 = value + case ARCH_X86_64: + v.ArchValues.X86_64 = value + case ARCH_ARM: + v.ArchValues.Arm = value + case ARCH_ARM64: + v.ArchValues.Arm64 = value + case CONDITIONS_DEFAULT: + v.ArchValues.ConditionsDefault = value + default: + panic(fmt.Errorf("Unknown arch: %s\n", arch)) + } +} + // Append appends all values, including os and arch specific ones, from another // StringListAttribute to this StringListAttribute func (attrs *StringListAttribute) Append(other StringListAttribute) { @@ -628,10 +831,10 @@ func (attrs *StringListAttribute) Append(other StringListAttribute) { } for os := range PlatformOsMap { - this := attrs.GetValueForOS(os) - that := other.GetValueForOS(os) - this = append(this, that...) - attrs.SetValueForOS(os, this) + this := attrs.getValueForTarget(os) + that := other.getValueForTarget(os) + this.Append(that) + attrs.setValueForTarget(os, this) } productValues := make(map[string][]string, 0) diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index 7a73e18b8..388c8cf66 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -568,6 +568,9 @@ func isZero(value reflect.Value) bool { return true } default: + if !value.IsValid() { + return true + } zeroValue := reflect.Zero(value.Type()) result := value.Interface() == zeroValue.Interface() return result diff --git a/bp2build/cc_library_static_conversion_test.go b/bp2build/cc_library_static_conversion_test.go index b7245a7d0..833ceba09 100644 --- a/bp2build/cc_library_static_conversion_test.go +++ b/bp2build/cc_library_static_conversion_test.go @@ -1158,6 +1158,66 @@ cc_library_static { }) } +func TestCcLibraryStaticGetTargetProperties(t *testing.T) { + runCcLibraryStaticTestCase(t, bp2buildTestCase{ + + description: "cc_library_static complex GetTargetProperties", + moduleTypeUnderTest: "cc_library_static", + moduleTypeUnderTestFactory: cc.LibraryStaticFactory, + moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryStaticBp2Build, + depsMutators: []android.RegisterMutatorFunc{cc.RegisterDepsBp2Build}, + blueprint: soongCcLibraryStaticPreamble + ` +cc_library_static { + name: "foo_static", + target: { + android: { + srcs: ["android_src.c"], + }, + android_arm: { + srcs: ["android_arm_src.c"], + }, + android_arm64: { + srcs: ["android_arm64_src.c"], + }, + android_x86: { + srcs: ["android_x86_src.c"], + }, + android_x86_64: { + srcs: ["android_x86_64_src.c"], + }, + linux_bionic_arm64: { + srcs: ["linux_bionic_arm64_src.c"], + }, + linux_bionic_x86_64: { + srcs: ["linux_bionic_x86_64_src.c"], + }, + }, +}`, + expectedBazelTargets: []string{`cc_library_static( + name = "foo_static", + copts = [ + "-I.", + "-I$(BINDIR)/.", + ], + linkstatic = True, + srcs_c = select({ + "//build/bazel/platforms/os:android": ["android_src.c"], + "//conditions:default": [], + }) + select({ + "//build/bazel/platforms:android_arm": ["android_arm_src.c"], + "//build/bazel/platforms:android_arm64": ["android_arm64_src.c"], + "//build/bazel/platforms:android_x86": ["android_x86_src.c"], + "//build/bazel/platforms:android_x86_64": ["android_x86_64_src.c"], + "//conditions:default": [], + }) + select({ + "//build/bazel/platforms:linux_bionic_arm64": ["linux_bionic_arm64_src.c"], + "//build/bazel/platforms:linux_bionic_x86_64": ["linux_bionic_x86_64_src.c"], + "//conditions:default": [], + }), +)`}, + }) +} + func TestCcLibraryStaticProductVariableSelects(t *testing.T) { runCcLibraryStaticTestCase(t, bp2buildTestCase{ description: "cc_library_static product variable selects", diff --git a/bp2build/configurability.go b/bp2build/configurability.go index 9869c5d37..b5070b9c9 100644 --- a/bp2build/configurability.go +++ b/bp2build/configurability.go @@ -27,12 +27,25 @@ func getStringListValues(list bazel.StringListAttribute) (reflect.Value, []selec } osSelects := map[string]reflect.Value{} - for os, selectKey := range bazel.PlatformOsMap { - osSelects[selectKey] = reflect.ValueOf(list.GetValueForOS(os)) + osArchSelects := make([]selects, 0) + for _, os := range android.SortedStringKeys(bazel.PlatformOsMap) { + selectKey := bazel.PlatformOsMap[os] + osSelects[selectKey] = reflect.ValueOf(list.GetOsValueForTarget(os)) + archSelects := make(map[string]reflect.Value) + // TODO(b/187530594): Should we also check arch=CONDITIONS_DEFAULT? (not in AllArches) + for _, arch := range bazel.AllArches { + target := os + "_" + arch + selectKey := bazel.PlatformTargetMap[target] + archSelects[selectKey] = reflect.ValueOf(list.GetOsArchValueForTarget(os, arch)) + } + osArchSelects = append(osArchSelects, archSelects) } if len(osSelects) > 0 { selectValues = append(selectValues, osSelects) } + if len(osArchSelects) > 0 { + selectValues = append(selectValues, osArchSelects...) + } for _, pv := range list.SortedProductVariables() { s := make(selects) @@ -74,11 +87,25 @@ func getLabelListValues(list bazel.LabelListAttribute) (reflect.Value, []selects } osSelects := map[string]reflect.Value{} - for os, selectKey := range bazel.PlatformOsMap { - osSelects[selectKey] = reflect.ValueOf(list.GetValueForOS(os).Includes) + osArchSelects := make([]selects, 0) + for _, os := range android.SortedStringKeys(bazel.PlatformOsMap) { + selectKey := bazel.PlatformOsMap[os] + osSelects[selectKey] = reflect.ValueOf(list.GetOsValueForTarget(os).Includes) + archSelects := make(map[string]reflect.Value) + // TODO(b/187530594): Should we also check arch=CONDITIOSN_DEFAULT? (not in AllArches) + for _, arch := range bazel.AllArches { + target := os + "_" + arch + selectKey := bazel.PlatformTargetMap[target] + archSelects[selectKey] = reflect.ValueOf(list.GetOsArchValueForTarget(os, arch).Includes) + } + osArchSelects = append(osArchSelects, archSelects) } - return value, []selects{archSelects, osSelects} + var selects []selects + selects = append(selects, archSelects) + selects = append(selects, osSelects) + selects = append(selects, osArchSelects...) + return value, selects } // prettyPrintAttribute converts an Attribute to its Bazel syntax. May contain diff --git a/cc/bp2build.go b/cc/bp2build.go index 9bf101e1e..e64573c3f 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -50,35 +50,51 @@ func depsBp2BuildMutator(ctx android.BottomUpMutatorContext) { var allDeps []string - for _, p := range module.GetTargetProperties(ctx, &BaseCompilerProperties{}) { - // base compiler props - if baseCompilerProps, ok := p.(*BaseCompilerProperties); ok { + for _, osProps := range module.GetTargetProperties(ctx, &BaseCompilerProperties{}) { + // os base compiler props + if baseCompilerProps, ok := osProps.Properties.(*BaseCompilerProperties); ok { allDeps = append(allDeps, baseCompilerProps.Generated_headers...) allDeps = append(allDeps, baseCompilerProps.Generated_sources...) } + // os + arch base compiler props + for _, archProps := range osProps.ArchProperties { + if baseCompilerProps, ok := archProps.(*BaseCompilerProperties); ok { + allDeps = append(allDeps, baseCompilerProps.Generated_headers...) + allDeps = append(allDeps, baseCompilerProps.Generated_sources...) + } + } } - for _, p := range module.GetArchProperties(ctx, &BaseCompilerProperties{}) { + for _, props := range module.GetArchProperties(ctx, &BaseCompilerProperties{}) { // arch specific compiler props - if baseCompilerProps, ok := p.(*BaseCompilerProperties); ok { + if baseCompilerProps, ok := props.(*BaseCompilerProperties); ok { allDeps = append(allDeps, baseCompilerProps.Generated_headers...) allDeps = append(allDeps, baseCompilerProps.Generated_sources...) } } - for _, p := range module.GetTargetProperties(ctx, &BaseLinkerProperties{}) { - // arch specific linker props - if baseLinkerProps, ok := p.(*BaseLinkerProperties); ok { + for _, osProps := range module.GetTargetProperties(ctx, &BaseLinkerProperties{}) { + // os specific linker props + if baseLinkerProps, ok := osProps.Properties.(*BaseLinkerProperties); ok { allDeps = append(allDeps, baseLinkerProps.Header_libs...) allDeps = append(allDeps, baseLinkerProps.Export_header_lib_headers...) allDeps = append(allDeps, baseLinkerProps.Static_libs...) allDeps = append(allDeps, baseLinkerProps.Whole_static_libs...) } + // os + arch base compiler props + for _, archProps := range osProps.ArchProperties { + if baseLinkerProps, ok := archProps.(*BaseLinkerProperties); ok { + allDeps = append(allDeps, baseLinkerProps.Header_libs...) + allDeps = append(allDeps, baseLinkerProps.Export_header_lib_headers...) + allDeps = append(allDeps, baseLinkerProps.Static_libs...) + allDeps = append(allDeps, baseLinkerProps.Whole_static_libs...) + } + } } - for _, p := range module.GetArchProperties(ctx, &BaseLinkerProperties{}) { + for _, props := range module.GetArchProperties(ctx, &BaseLinkerProperties{}) { // arch specific linker props - if baseLinkerProps, ok := p.(*BaseLinkerProperties); ok { + if baseLinkerProps, ok := props.(*BaseLinkerProperties); ok { allDeps = append(allDeps, baseLinkerProps.Header_libs...) allDeps = append(allDeps, baseLinkerProps.Export_header_lib_headers...) allDeps = append(allDeps, baseLinkerProps.Static_libs...) @@ -309,16 +325,27 @@ func bp2BuildParseCompilerProps(ctx android.TopDownMutatorContext, module *Modul // 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(ctx, &BaseCompilerProperties{}) { - if baseCompilerProps, ok := props.(*BaseCompilerProperties); ok { + // Handle target specific properties. + for os, osProps := range module.GetTargetProperties(ctx, &BaseCompilerProperties{}) { + if baseCompilerProps, ok := osProps.Properties.(*BaseCompilerProperties); ok { srcsList := parseSrcs(baseCompilerProps) // 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)) + srcs.SetOsValueForTarget(os.Name, bazel.SubtractBazelLabelList(srcsList, baseSrcsLabelList)) + copts.SetOsValueForTarget(os.Name, parseCopts(baseCompilerProps)) + asFlags.SetOsValueForTarget(os.Name, parseCommandLineFlags(baseCompilerProps.Asflags)) + conlyFlags.SetOsValueForTarget(os.Name, parseCommandLineFlags(baseCompilerProps.Conlyflags)) + cppFlags.SetOsValueForTarget(os.Name, parseCommandLineFlags(baseCompilerProps.Cppflags)) + } + for arch, archProps := range osProps.ArchProperties { + if baseCompilerProps, ok := archProps.(*BaseCompilerProperties); ok { + srcsList := parseSrcs(baseCompilerProps) + // TODO(b/186153868): add support for os-specific srcs and exclude_srcs + srcs.SetOsArchValueForTarget(os.Name, arch.Name, bazel.SubtractBazelLabelList(srcsList, baseSrcsLabelList)) + copts.SetOsArchValueForTarget(os.Name, arch.Name, parseCopts(baseCompilerProps)) + asFlags.SetOsArchValueForTarget(os.Name, arch.Name, parseCommandLineFlags(baseCompilerProps.Asflags)) + conlyFlags.SetOsArchValueForTarget(os.Name, arch.Name, parseCommandLineFlags(baseCompilerProps.Conlyflags)) + cppFlags.SetOsArchValueForTarget(os.Name, arch.Name, parseCommandLineFlags(baseCompilerProps.Cppflags)) + } } } @@ -391,13 +418,18 @@ func bp2BuildParseLinkerProps(ctx android.TopDownMutatorContext, module *Module) var linkopts bazel.StringListAttribute var versionScript bazel.LabelAttribute + getLibs := func(baseLinkerProps *BaseLinkerProperties) []string { + libs := baseLinkerProps.Header_libs + libs = append(libs, baseLinkerProps.Static_libs...) + libs = android.SortedUniqueStrings(libs) + return libs + } + for _, linkerProps := range module.linker.linkerProps() { if baseLinkerProps, ok := linkerProps.(*BaseLinkerProperties); ok { - libs := baseLinkerProps.Header_libs - libs = append(libs, baseLinkerProps.Static_libs...) + libs := getLibs(baseLinkerProps) exportedLibs := baseLinkerProps.Export_header_lib_headers wholeArchiveLibs := baseLinkerProps.Whole_static_libs - libs = android.SortedUniqueStrings(libs) deps = bazel.MakeLabelListAttribute(android.BazelLabelForModuleDeps(ctx, libs)) exportedDeps = bazel.MakeLabelListAttribute(android.BazelLabelForModuleDeps(ctx, exportedLibs)) linkopts.Value = getBp2BuildLinkerFlags(baseLinkerProps) @@ -414,13 +446,11 @@ func bp2BuildParseLinkerProps(ctx android.TopDownMutatorContext, module *Module) } } - for arch, p := range module.GetArchProperties(ctx, &BaseLinkerProperties{}) { - if baseLinkerProps, ok := p.(*BaseLinkerProperties); ok { - libs := baseLinkerProps.Header_libs - libs = append(libs, baseLinkerProps.Static_libs...) + for arch, props := range module.GetArchProperties(ctx, &BaseLinkerProperties{}) { + if baseLinkerProps, ok := props.(*BaseLinkerProperties); ok { + libs := getLibs(baseLinkerProps) exportedLibs := baseLinkerProps.Export_header_lib_headers wholeArchiveLibs := baseLinkerProps.Whole_static_libs - libs = android.SortedUniqueStrings(libs) deps.SetValueForArch(arch.Name, android.BazelLabelForModuleDeps(ctx, libs)) exportedDeps.SetValueForArch(arch.Name, android.BazelLabelForModuleDeps(ctx, exportedLibs)) linkopts.SetValueForArch(arch.Name, getBp2BuildLinkerFlags(baseLinkerProps)) @@ -436,21 +466,34 @@ func bp2BuildParseLinkerProps(ctx android.TopDownMutatorContext, module *Module) } } - for os, p := range module.GetTargetProperties(ctx, &BaseLinkerProperties{}) { - if baseLinkerProps, ok := p.(*BaseLinkerProperties); ok { - libs := baseLinkerProps.Header_libs - libs = append(libs, baseLinkerProps.Static_libs...) + for os, targetProperties := range module.GetTargetProperties(ctx, &BaseLinkerProperties{}) { + if baseLinkerProps, ok := targetProperties.Properties.(*BaseLinkerProperties); ok { + libs := getLibs(baseLinkerProps) exportedLibs := baseLinkerProps.Export_header_lib_headers wholeArchiveLibs := baseLinkerProps.Whole_static_libs - libs = android.SortedUniqueStrings(libs) - wholeArchiveDeps.SetValueForOS(os.Name, android.BazelLabelForModuleDeps(ctx, wholeArchiveLibs)) - deps.SetValueForOS(os.Name, android.BazelLabelForModuleDeps(ctx, libs)) - exportedDeps.SetValueForOS(os.Name, android.BazelLabelForModuleDeps(ctx, exportedLibs)) + wholeArchiveDeps.SetOsValueForTarget(os.Name, android.BazelLabelForModuleDeps(ctx, wholeArchiveLibs)) + deps.SetOsValueForTarget(os.Name, android.BazelLabelForModuleDeps(ctx, libs)) + exportedDeps.SetOsValueForTarget(os.Name, android.BazelLabelForModuleDeps(ctx, exportedLibs)) - linkopts.SetValueForOS(os.Name, getBp2BuildLinkerFlags(baseLinkerProps)) + linkopts.SetOsValueForTarget(os.Name, getBp2BuildLinkerFlags(baseLinkerProps)) sharedLibs := baseLinkerProps.Shared_libs - dynamicDeps.SetValueForOS(os.Name, android.BazelLabelForModuleDeps(ctx, sharedLibs)) + dynamicDeps.SetOsValueForTarget(os.Name, android.BazelLabelForModuleDeps(ctx, sharedLibs)) + } + for arch, archProperties := range targetProperties.ArchProperties { + if baseLinkerProps, ok := archProperties.(*BaseLinkerProperties); ok { + libs := getLibs(baseLinkerProps) + exportedLibs := baseLinkerProps.Export_header_lib_headers + wholeArchiveLibs := baseLinkerProps.Whole_static_libs + wholeArchiveDeps.SetOsArchValueForTarget(os.Name, arch.Name, android.BazelLabelForModuleDeps(ctx, wholeArchiveLibs)) + deps.SetOsArchValueForTarget(os.Name, arch.Name, android.BazelLabelForModuleDeps(ctx, libs)) + exportedDeps.SetOsArchValueForTarget(os.Name, arch.Name, android.BazelLabelForModuleDeps(ctx, exportedLibs)) + + linkopts.SetOsArchValueForTarget(os.Name, arch.Name, getBp2BuildLinkerFlags(baseLinkerProps)) + + sharedLibs := baseLinkerProps.Shared_libs + dynamicDeps.SetOsArchValueForTarget(os.Name, arch.Name, android.BazelLabelForModuleDeps(ctx, sharedLibs)) + } } } @@ -499,32 +542,38 @@ func bp2BuildParseExportedIncludes(ctx android.TopDownMutatorContext, module *Mo includeDirs = append(includeDirs, libraryDecorator.flagExporter.Properties.Export_include_dirs...) includeDirsAttribute := bazel.MakeStringListAttribute(includeDirs) + getVariantIncludeDirs := func(includeDirs []string, flagExporterProperties *FlagExporterProperties) []string { + variantIncludeDirs := flagExporterProperties.Export_system_include_dirs + variantIncludeDirs = append(variantIncludeDirs, flagExporterProperties.Export_include_dirs...) + + // To avoid duplicate includes when base includes + arch includes are combined + // TODO: This doesn't take conflicts between arch and os includes into account + variantIncludeDirs = bazel.SubtractStrings(variantIncludeDirs, includeDirs) + return variantIncludeDirs + } + for arch, props := range module.GetArchProperties(ctx, &FlagExporterProperties{}) { if flagExporterProperties, ok := props.(*FlagExporterProperties); ok { - archIncludeDirs := flagExporterProperties.Export_system_include_dirs - archIncludeDirs = append(archIncludeDirs, flagExporterProperties.Export_include_dirs...) - - // To avoid duplicate includes when base includes + arch includes are combined - // FIXME: This doesn't take conflicts between arch and os includes into account - archIncludeDirs = bazel.SubtractStrings(archIncludeDirs, includeDirs) - + archIncludeDirs := getVariantIncludeDirs(includeDirs, flagExporterProperties) if len(archIncludeDirs) > 0 { includeDirsAttribute.SetValueForArch(arch.Name, archIncludeDirs) } } } - for os, props := range module.GetTargetProperties(ctx, &FlagExporterProperties{}) { - if flagExporterProperties, ok := props.(*FlagExporterProperties); ok { - osIncludeDirs := flagExporterProperties.Export_system_include_dirs - osIncludeDirs = append(osIncludeDirs, flagExporterProperties.Export_include_dirs...) - - // To avoid duplicate includes when base includes + os includes are combined - // FIXME: This doesn't take conflicts between arch and os includes into account - osIncludeDirs = bazel.SubtractStrings(osIncludeDirs, includeDirs) - - if len(osIncludeDirs) > 0 { - includeDirsAttribute.SetValueForOS(os.Name, osIncludeDirs) + for os, targetProperties := range module.GetTargetProperties(ctx, &FlagExporterProperties{}) { + if flagExporterProperties, ok := targetProperties.Properties.(*FlagExporterProperties); ok { + targetIncludeDirs := getVariantIncludeDirs(includeDirs, flagExporterProperties) + if len(targetIncludeDirs) > 0 { + includeDirsAttribute.SetOsValueForTarget(os.Name, targetIncludeDirs) + } + } + for arch, archProperties := range targetProperties.ArchProperties { + if flagExporterProperties, ok := archProperties.(*FlagExporterProperties); ok { + targetIncludeDirs := getVariantIncludeDirs(includeDirs, flagExporterProperties) + if len(targetIncludeDirs) > 0 { + includeDirsAttribute.SetOsArchValueForTarget(os.Name, arch.Name, targetIncludeDirs) + } } } }