From 22cd2eb3d142f4e52e88da0ab384bb62518ae7aa Mon Sep 17 00:00:00 2001 From: Rupert Shuttleworth Date: Thu, 27 May 2021 02:15:54 -0400 Subject: [PATCH] Add e.g. Target: { Android_arm: { ...} } support to LabelAttribute. LabelListAttribute support was already added, but LabelAttribute support is needed for cc_import rules. Test: Added unit test for version_script, which is the only supported LabelAttribute so far. Change-Id: I4e86e7391586e0780623d06b794e7399f0ccd50e --- bazel/properties.go | 181 +++++++++++++++++++++---- bp2build/cc_library_conversion_test.go | 43 ++++++ bp2build/configurability.go | 70 ++++++++-- cc/bp2build.go | 8 ++ 4 files changed, 263 insertions(+), 39 deletions(-) diff --git a/bazel/properties.go b/bazel/properties.go index 8adc9d07f..491a15455 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -332,49 +332,174 @@ type Attribute interface { HasConfigurableValues() bool } -// Represents an attribute whose value is a single label -type LabelAttribute struct { - Value Label +type labelArchValues struct { X86 Label X86_64 Label Arm Label Arm64 Label + + ConditionsDefault Label +} + +type labelTargetValue struct { + // E.g. for android + OsValue Label + + // E.g. for android_arm, android_arm64, ... + ArchValues labelArchValues +} + +type labelTargetValues struct { + Android labelTargetValue + Darwin labelTargetValue + Fuchsia labelTargetValue + Linux labelTargetValue + LinuxBionic labelTargetValue + Windows labelTargetValue + + ConditionsDefault labelTargetValue +} + +// Represents an attribute whose value is a single label +type LabelAttribute struct { + Value Label + + ArchValues labelArchValues + + TargetValues labelTargetValues } func (attr *LabelAttribute) GetValueForArch(arch string) Label { - switch arch { - case ARCH_ARM: - return attr.Arm - case ARCH_ARM64: - return attr.Arm64 - case ARCH_X86: - return attr.X86 - case ARCH_X86_64: - return attr.X86_64 - case CONDITIONS_DEFAULT: - return attr.Value - default: - panic("Invalid arch type") + var v *Label + if v = attr.archValuePtrs()[arch]; v == nil { + panic(fmt.Errorf("Unknown arch: %s", arch)) } + return *v } func (attr *LabelAttribute) SetValueForArch(arch string, value Label) { - switch arch { - case ARCH_ARM: - attr.Arm = value - case ARCH_ARM64: - attr.Arm64 = value - case ARCH_X86: - attr.X86 = value - case ARCH_X86_64: - attr.X86_64 = value - default: - panic("Invalid arch type") + var v *Label + if v = attr.archValuePtrs()[arch]; v == nil { + panic(fmt.Errorf("Unknown arch: %s", arch)) + } + *v = value +} + +func (attr *LabelAttribute) archValuePtrs() map[string]*Label { + return map[string]*Label{ + ARCH_X86: &attr.ArchValues.X86, + ARCH_X86_64: &attr.ArchValues.X86_64, + ARCH_ARM: &attr.ArchValues.Arm, + ARCH_ARM64: &attr.ArchValues.Arm64, + CONDITIONS_DEFAULT: &attr.ArchValues.ConditionsDefault, } } func (attr LabelAttribute) HasConfigurableValues() bool { - return attr.Arm.Label != "" || attr.Arm64.Label != "" || attr.X86.Label != "" || attr.X86_64.Label != "" + for arch := range PlatformArchMap { + if attr.GetValueForArch(arch).Label != "" { + return true + } + } + + for os := range PlatformOsMap { + if attr.GetOsValueForTarget(os).Label != "" { + return true + } + // TODO(b/187530594): Should we also check arch=CONDITIONS_DEFAULT (not in AllArches) + for _, arch := range AllArches { + if attr.GetOsArchValueForTarget(os, arch).Label != "" { + return true + } + } + } + return false +} + +func (attr *LabelAttribute) getValueForTarget(os string) labelTargetValue { + var v *labelTargetValue + if v = attr.targetValuePtrs()[os]; v == nil { + panic(fmt.Errorf("Unknown os: %s", os)) + } + return *v +} + +func (attr *LabelAttribute) GetOsValueForTarget(os string) Label { + var v *labelTargetValue + if v = attr.targetValuePtrs()[os]; v == nil { + panic(fmt.Errorf("Unknown os: %s", os)) + } + return v.OsValue +} + +func (attr *LabelAttribute) GetOsArchValueForTarget(os string, arch string) Label { + var v *labelTargetValue + if v = attr.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 (attr *LabelAttribute) setValueForTarget(os string, value labelTargetValue) { + var v *labelTargetValue + if v = attr.targetValuePtrs()[os]; v == nil { + panic(fmt.Errorf("Unknown os: %s", os)) + } + *v = value +} + +func (attr *LabelAttribute) SetOsValueForTarget(os string, value Label) { + var v *labelTargetValue + if v = attr.targetValuePtrs()[os]; v == nil { + panic(fmt.Errorf("Unknown os: %s", os)) + } + v.OsValue = value +} + +func (attr *LabelAttribute) SetOsArchValueForTarget(os string, arch string, value Label) { + var v *labelTargetValue + if v = attr.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)) + } +} + +func (attr *LabelAttribute) targetValuePtrs() map[string]*labelTargetValue { + return map[string]*labelTargetValue{ + OS_ANDROID: &attr.TargetValues.Android, + OS_DARWIN: &attr.TargetValues.Darwin, + OS_FUCHSIA: &attr.TargetValues.Fuchsia, + OS_LINUX: &attr.TargetValues.Linux, + OS_LINUX_BIONIC: &attr.TargetValues.LinuxBionic, + OS_WINDOWS: &attr.TargetValues.Windows, + CONDITIONS_DEFAULT: &attr.TargetValues.ConditionsDefault, + } } // Arch-specific label_list typed Bazel attribute values. This should correspond diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index 6aa148ec2..49f1f42ba 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/bp2build/cc_library_conversion_test.go @@ -808,3 +808,46 @@ func TestCcLibraryCppFlagsGoesIntoCopts(t *testing.T) { )`}, }) } + +func TestCcLibraryLabelAttributeGetTargetProperties(t *testing.T) { + runCcLibraryTestCase(t, bp2buildTestCase{ + description: "cc_library GetTargetProperties on a LabelAttribute", + moduleTypeUnderTest: "cc_library", + moduleTypeUnderTestFactory: cc.LibraryFactory, + moduleTypeUnderTestBp2BuildMutator: cc.CcLibraryBp2Build, + depsMutators: []android.RegisterMutatorFunc{cc.RegisterDepsBp2Build}, + dir: "foo/bar", + filesystem: map[string]string{ + "foo/bar/Android.bp": ` + cc_library { + name: "a", + srcs: ["a.cpp"], + target: { + android_arm: { + version_script: "android_arm.map", + }, + linux_bionic_arm64: { + version_script: "linux_bionic_arm64.map", + }, + }, + + bazel_module: { bp2build_available: true }, + } + `, + }, + blueprint: soongCcLibraryPreamble, + expectedBazelTargets: []string{`cc_library( + name = "a", + copts = [ + "-Ifoo/bar", + "-I$(BINDIR)/foo/bar", + ], + srcs = ["a.cpp"], + version_script = select({ + "//build/bazel/platforms:android_arm": "android_arm.map", + "//build/bazel/platforms:linux_bionic_arm64": "linux_bionic_arm64.map", + "//conditions:default": None, + }), +)`}, + }) +} diff --git a/bp2build/configurability.go b/bp2build/configurability.go index b5070b9c9..c13e737c0 100644 --- a/bp2build/configurability.go +++ b/bp2build/configurability.go @@ -60,19 +60,67 @@ func getStringListValues(list bazel.StringListAttribute) (reflect.Value, []selec } func getLabelValue(label bazel.LabelAttribute) (reflect.Value, []selects) { - var value reflect.Value - var archSelects selects - - if label.HasConfigurableValues() { - archSelects = map[string]reflect.Value{} - for arch, selectKey := range bazel.PlatformArchMap { - archSelects[selectKey] = reflect.ValueOf(label.GetValueForArch(arch)) - } - } else { - value = reflect.ValueOf(label.Value) + value := reflect.ValueOf(label.Value) + if !label.HasConfigurableValues() { + return value, []selects{} } - return value, []selects{archSelects} + // Keep track of which arches and oses have been used in case we need to raise a warning + usedArches := make(map[string]bool) + usedOses := make(map[string]bool) + + archSelects := map[string]reflect.Value{} + for arch, selectKey := range bazel.PlatformArchMap { + archSelects[selectKey] = reflect.ValueOf(label.GetValueForArch(arch)) + if archSelects[selectKey].IsValid() && !isZero(archSelects[selectKey]) { + usedArches[arch] = true + } + } + + osSelects := map[string]reflect.Value{} + for _, os := range android.SortedStringKeys(bazel.PlatformOsMap) { + selectKey := bazel.PlatformOsMap[os] + osSelects[selectKey] = reflect.ValueOf(label.GetOsValueForTarget(os)) + if osSelects[selectKey].IsValid() && !isZero(osSelects[selectKey]) { + usedOses[os] = true + } + } + + osArchSelects := make([]selects, 0) + for _, os := range android.SortedStringKeys(bazel.PlatformOsMap) { + 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(label.GetOsArchValueForTarget(os, arch)) + if archSelects[selectKey].IsValid() && !isZero(archSelects[selectKey]) { + if _, ok := usedArches[arch]; ok { + fmt.Printf("WARNING: Same arch used twice in LabelAttribute select: arch '%s'\n", arch) + } + if _, ok := usedOses[os]; ok { + fmt.Printf("WARNING: Same os used twice in LabelAttribute select: os '%s'\n", os) + } + } + } + osArchSelects = append(osArchSelects, archSelects) + } + + // Because we have to return a single Label, we can only use one select statement + combinedSelects := map[string]reflect.Value{} + for k, v := range archSelects { + combinedSelects[k] = v + } + for k, v := range osSelects { + combinedSelects[k] = v + } + for _, osArchSelect := range osArchSelects { + for k, v := range osArchSelect { + combinedSelects[k] = v + } + } + + return value, []selects{combinedSelects} } func getLabelListValues(list bazel.LabelListAttribute) (reflect.Value, []selects) { diff --git a/cc/bp2build.go b/cc/bp2build.go index 339a489d0..711410c39 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -568,6 +568,10 @@ func bp2BuildParseLinkerProps(ctx android.TopDownMutatorContext, module *Module) linkopts.SetOsValueForTarget(os.Name, getBp2BuildLinkerFlags(baseLinkerProps)) + if baseLinkerProps.Version_script != nil { + versionScript.SetOsValueForTarget(os.Name, android.BazelLabelForModuleSrcSingle(ctx, *baseLinkerProps.Version_script)) + } + sharedLibs := baseLinkerProps.Shared_libs dynamicDeps.SetOsValueForTarget(os.Name, android.BazelLabelForModuleDeps(ctx, sharedLibs)) } @@ -582,6 +586,10 @@ func bp2BuildParseLinkerProps(ctx android.TopDownMutatorContext, module *Module) linkopts.SetOsArchValueForTarget(os.Name, arch.Name, getBp2BuildLinkerFlags(baseLinkerProps)) + if baseLinkerProps.Version_script != nil { + versionScript.SetOsArchValueForTarget(os.Name, arch.Name, android.BazelLabelForModuleSrcSingle(ctx, *baseLinkerProps.Version_script)) + } + sharedLibs := baseLinkerProps.Shared_libs dynamicDeps.SetOsArchValueForTarget(os.Name, arch.Name, android.BazelLabelForModuleDeps(ctx, sharedLibs)) }