diff --git a/android/arch.go b/android/arch.go index f719ddcd9..20b4ab07d 100644 --- a/android/arch.go +++ b/android/arch.go @@ -1615,3 +1615,97 @@ func decodeMultilibTargets(multilib string, targets []Target, prefer32 bool) ([] return buildTargets, nil } + +// GetArchProperties returns a map of architectures to the values of the +// properties of the 'dst' struct that are specific to that architecture. +// +// 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 arch specific property value specified by the module if defined. +func (m *ModuleBase) GetArchProperties(dst interface{}) map[ArchType]interface{} { + // Return value of the arch types to the prop values for that arch. + archToProp := map[ArchType]interface{}{} + + // Nothing to do for non-arch-specific modules. + if !m.ArchSpecific() { + return archToProp + } + + // 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 { + // Skip over nil arch props + continue + } + + // Non-nil arch prop, let's see if the props match up. + for _, arch := range ArchTypeList() { + // e.g X86, Arm + field := arch.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: + // + // arch: { arm: { key: value, ... }, x86: { key: value, ... } } + for _, archProperties := range m.archProperties[i] { + archPropValues := reflect.ValueOf(archProperties).Elem() + + // This is the archPropRoot struct. Traverse into the Arch nested struct. + src := archPropValues.FieldByName("Arch").Elem() + + // Step into non-nil pointers to structs in the src value. + if src.Kind() == reflect.Ptr { + if src.IsNil() { + // Ignore nil pointers. + continue + } + src = src.Elem() + } + + // Find the requested field (e.g. x86, x86_64) in the src struct. + src = src.FieldByName(field) + if !src.IsValid() { + continue + } + + // We only care about structs. These are not the droids you are looking for. + if src.Kind() != reflect.Struct { + continue + } + + // If the value of the field is a struct then step into the + // BlueprintEmbed field. The special "BlueprintEmbed" name is + // used by createArchPropTypeDesc to embed the arch properties + // in the parent struct, so the src arch prop should be in this + // field. + // + // See createArchPropTypeDesc for more details on how Arch-specific + // module properties are processed from the nested props and written + // into the module's archProperties. + src = src.FieldByName("BlueprintEmbed") + + // 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 arch, you have. + archToProp[arch] = dstClone + + // Go to the next prop. + break + } + } + } + return archToProp +} diff --git a/android/module.go b/android/module.go index 53422465a..1936d3cfb 100644 --- a/android/module.go +++ b/android/module.go @@ -1123,8 +1123,15 @@ type ModuleBase struct { variableProperties interface{} hostAndDeviceProperties hostAndDeviceProperties generalProperties []interface{} - archProperties [][]interface{} - customizableProperties []interface{} + + // Arch specific versions of structs in generalProperties. The outer index + // has the same order as generalProperties as initialized in + // InitAndroidArchModule, and the inner index chooses the props specific to + // the architecture. The interface{} value is an archPropRoot that is + // filled with arch specific values by the arch mutator. + archProperties [][]interface{} + + customizableProperties []interface{} // Properties specific to the Blueprint to BUILD migration. bazelTargetModuleProperties bazel.BazelTargetModuleProperties diff --git a/bazel/properties.go b/bazel/properties.go index a4df4bc83..a5ffa55cb 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -14,6 +14,8 @@ package bazel +import "fmt" + type bazelModuleProperties struct { // The label of the Bazel target replacing this Soong module. Label string @@ -63,3 +65,72 @@ func (ll *LabelList) Append(other LabelList) { ll.Excludes = append(other.Excludes, other.Excludes...) } } + +// StringListAttribute corresponds to the string_list Bazel attribute type with +// support for additional metadata, like configurations. +type StringListAttribute struct { + // The base value of the string list attribute. + Value []string + + // Optional additive set of list values to the base value. + ArchValues stringListArchValues +} + +// Arch-specific string_list typed Bazel attribute values. This should correspond +// to the types of architectures supported for compilation in arch.go. +type stringListArchValues struct { + X86 []string + X86_64 []string + Arm []string + Arm64 []string + Default []string + // TODO(b/181299724): this is currently missing the "common" arch, which + // doesn't have an equivalent platform() definition yet. +} + +// HasArchSpecificValues returns true if the attribute contains +// architecture-specific string_list values. +func (attrs *StringListAttribute) HasArchSpecificValues() bool { + for _, arch := range []string{"x86", "x86_64", "arm", "arm64", "default"} { + if len(attrs.GetValueForArch(arch)) > 0 { + return true + } + } + return false +} + +// GetValueForArch returns the string_list attribute value for an architecture. +func (attrs *StringListAttribute) GetValueForArch(arch string) []string { + switch arch { + case "x86": + return attrs.ArchValues.X86 + case "x86_64": + return attrs.ArchValues.X86_64 + case "arm": + return attrs.ArchValues.Arm + case "arm64": + return attrs.ArchValues.Arm64 + case "default": + return attrs.ArchValues.Default + default: + panic(fmt.Errorf("Unknown arch: %s", arch)) + } +} + +// SetValueForArch sets the string_list attribute value for an architecture. +func (attrs *StringListAttribute) SetValueForArch(arch string, value []string) { + 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 + case "default": + attrs.ArchValues.Default = value + default: + panic(fmt.Errorf("Unknown arch: %s", arch)) + } +} diff --git a/bp2build/Android.bp b/bp2build/Android.bp index 7d748ec98..8deb5a217 100644 --- a/bp2build/Android.bp +++ b/bp2build/Android.bp @@ -10,6 +10,7 @@ bootstrap_go_package { "bp2build.go", "build_conversion.go", "bzl_conversion.go", + "configurability.go", "conversion.go", "metrics.go", ], diff --git a/bp2build/build_conversion.go b/bp2build/build_conversion.go index a4c4a0dd3..7fa499683 100644 --- a/bp2build/build_conversion.go +++ b/bp2build/build_conversion.go @@ -354,11 +354,42 @@ func prettyPrint(propertyValue reflect.Value, indent int) (string, error) { ret += makeIndent(indent) ret += "]" case reflect.Struct: + // 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.LabelList); ok { // TODO(b/165114590): convert glob syntax return prettyPrint(reflect.ValueOf(labels.Includes), 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) + list, _ := prettyPrint(reflect.ValueOf(stringList.GetValueForArch("default")), indent+1) + ret += fmt.Sprintf("\"%s\": %s,\n", "//conditions:default", list) + + ret += makeIndent(indent) + ret += "})" + return ret, err } ret = "{\n" diff --git a/bp2build/cc_object_conversion_test.go b/bp2build/cc_object_conversion_test.go index f6558422f..1d4e32221 100644 --- a/bp2build/cc_object_conversion_test.go +++ b/bp2build/cc_object_conversion_test.go @@ -99,15 +99,6 @@ func TestCcObjectBp2Build(t *testing.T) { cc_defaults { name: "foo_defaults", defaults: ["foo_bar_defaults"], - // TODO(b/178130668): handle configurable attributes that depend on the platform - arch: { - x86: { - cflags: ["-fPIC"], - }, - x86_64: { - cflags: ["-fPIC"], - }, - }, } cc_defaults { @@ -233,3 +224,137 @@ cc_object { } } } + +func TestCcObjectConfigurableAttributesBp2Build(t *testing.T) { + testCases := []struct { + description string + moduleTypeUnderTest string + moduleTypeUnderTestFactory android.ModuleFactory + moduleTypeUnderTestBp2BuildMutator func(android.TopDownMutatorContext) + blueprint string + expectedBazelTargets []string + filesystem map[string]string + }{ + { + description: "cc_object setting cflags for one arch", + moduleTypeUnderTest: "cc_object", + moduleTypeUnderTestFactory: cc.ObjectFactory, + moduleTypeUnderTestBp2BuildMutator: cc.ObjectBp2Build, + blueprint: `cc_object { + name: "foo", + arch: { + x86: { + cflags: ["-fPIC"], + }, + }, + bazel_module: { bp2build_available: true }, +} +`, + expectedBazelTargets: []string{ + `cc_object( + name = "foo", + copts = [ + "-fno-addrsig", + ] + select({ + "@bazel_tools//platforms:x86_32": [ + "-fPIC", + ], + "//conditions:default": [ + ], + }), +)`, + }, + }, + { + description: "cc_object setting cflags for 4 architectures", + moduleTypeUnderTest: "cc_object", + moduleTypeUnderTestFactory: cc.ObjectFactory, + moduleTypeUnderTestBp2BuildMutator: cc.ObjectBp2Build, + blueprint: `cc_object { + name: "foo", + arch: { + x86: { + cflags: ["-fPIC"], + }, + x86_64: { + cflags: ["-fPIC"], + }, + arm: { + cflags: ["-Wall"], + }, + arm64: { + cflags: ["-Wall"], + }, + }, + bazel_module: { bp2build_available: true }, +} +`, + expectedBazelTargets: []string{ + `cc_object( + name = "foo", + copts = [ + "-fno-addrsig", + ] + select({ + "@bazel_tools//platforms:arm": [ + "-Wall", + ], + "@bazel_tools//platforms:aarch64": [ + "-Wall", + ], + "@bazel_tools//platforms:x86_32": [ + "-fPIC", + ], + "@bazel_tools//platforms:x86_64": [ + "-fPIC", + ], + "//conditions:default": [ + ], + }), +)`, + }, + }, + } + + dir := "." + for _, testCase := range testCases { + filesystem := make(map[string][]byte) + toParse := []string{ + "Android.bp", + } + config := android.TestConfig(buildDir, nil, testCase.blueprint, filesystem) + ctx := android.NewTestContext(config) + // Always register cc_defaults module factory + ctx.RegisterModuleType("cc_defaults", func() android.Module { return cc.DefaultsFactory() }) + + ctx.RegisterModuleType(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestFactory) + ctx.RegisterBp2BuildMutator(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestBp2BuildMutator) + ctx.RegisterForBazelConversion() + + _, errs := ctx.ParseFileList(dir, toParse) + if Errored(t, testCase.description, errs) { + continue + } + _, errs = ctx.ResolveDependencies(config) + if Errored(t, testCase.description, errs) { + continue + } + + codegenCtx := NewCodegenContext(config, *ctx.Context, Bp2Build) + bazelTargets := generateBazelTargetsForDir(codegenCtx, dir) + if actualCount, expectedCount := len(bazelTargets), len(testCase.expectedBazelTargets); actualCount != expectedCount { + fmt.Println(bazelTargets) + t.Errorf("%s: Expected %d bazel target, got %d", testCase.description, expectedCount, actualCount) + } else { + for i, target := range bazelTargets { + if w, g := testCase.expectedBazelTargets[i], target.content; w != g { + t.Errorf( + "%s: Expected generated Bazel target to be '%s', got '%s'", + testCase.description, + w, + g, + ) + } + } + } + } +} diff --git a/bp2build/configurability.go b/bp2build/configurability.go new file mode 100644 index 000000000..47cf3c612 --- /dev/null +++ b/bp2build/configurability.go @@ -0,0 +1,15 @@ +package bp2build + +import "android/soong/android" + +// 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", + } +) diff --git a/cc/object.go b/cc/object.go index 140a066af..32347b81b 100644 --- a/cc/object.go +++ b/cc/object.go @@ -93,7 +93,7 @@ func ObjectFactory() android.Module { type bazelObjectAttributes struct { Srcs bazel.LabelList Deps bazel.LabelList - Copts []string + Copts bazel.StringListAttribute Local_include_dirs []string } @@ -133,13 +133,14 @@ func ObjectBp2Build(ctx android.TopDownMutatorContext) { ctx.ModuleErrorf("compiler must not be nil for a cc_object module") } - var copts []string + // Set arch-specific configurable attributes + var copts bazel.StringListAttribute var srcs []string var excludeSrcs []string var localIncludeDirs []string for _, props := range m.compiler.compilerProps() { if baseCompilerProps, ok := props.(*BaseCompilerProperties); ok { - copts = baseCompilerProps.Cflags + copts.Value = baseCompilerProps.Cflags srcs = baseCompilerProps.Srcs excludeSrcs = baseCompilerProps.Exclude_srcs localIncludeDirs = baseCompilerProps.Local_include_dirs @@ -154,6 +155,13 @@ func ObjectBp2Build(ctx android.TopDownMutatorContext) { } } + for arch, p := range m.GetArchProperties(&BaseCompilerProperties{}) { + if cProps, ok := p.(*BaseCompilerProperties); ok { + copts.SetValueForArch(arch.Name, cProps.Cflags) + } + } + copts.SetValueForArch("default", []string{}) + attrs := &bazelObjectAttributes{ Srcs: android.BazelLabelForModuleSrcExcludes(ctx, srcs, excludeSrcs), Deps: deps,