Merge "bp2build: add configurable attribute (select) support." am: 37bd24a1db am: 1c65191027 am: f28de7434b

Original change: https://android-review.googlesource.com/c/platform/build/soong/+/1603778

MUST ONLY BE SUBMITTED BY AUTOMERGER

Change-Id: Iafe936decfc711746a17c4a6348c141654b60fc0

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
+}

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

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))
+	}
+}

bp2build/Android.bp

@@ -10,6 +10,7 @@ bootstrap_go_package {
         "bp2build.go",
         "build_conversion.go",
         "bzl_conversion.go",
+        "configurability.go",
         "conversion.go",
         "metrics.go",
     ],

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"

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,
+					)
+				}
+			}
+		}
+	}
+}

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",
+	}
+)

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,