Merge "Add target/os configurable string_list attrs."

bazel/properties.go

@@ -96,21 +96,10 @@ const (
 )
 
 var (
-	// This is the list of architectures with a Bazel config_setting and
+	// These are the list of OSes and architectures with a Bazel config_setting
-	// constraint value equivalent. is actually android.ArchTypeList, but the
+	// and constraint value equivalent. These exist in arch.go, but the android
-	// android package depends on the bazel package, so a cyclic dependency
+	// package depends on the bazel package, so a cyclic dependency prevents
-	// prevents using that here.
+	// using those variables 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
@@ -133,6 +122,10 @@ var (
 	}
 )
 
+type Attribute interface {
+	HasConfigurableValues() bool
+}
+
 // Arch-specific label_list typed Bazel attribute values. This should correspond
 // to the types of architectures supported for compilation in arch.go.
 type labelListArchValues struct {
@@ -176,14 +169,14 @@ func MakeLabelListAttribute(value LabelList) LabelListAttribute {
 
 // HasArchSpecificValues returns true if the attribute contains
 // architecture-specific label_list values.
-func (attrs *LabelListAttribute) HasConfigurableValues() bool {
+func (attrs LabelListAttribute) HasConfigurableValues() bool {
-	for _, arch := range selectableArchs {
+	for arch := range PlatformArchMap {
 		if len(attrs.GetValueForArch(arch).Includes) > 0 {
 			return true
 		}
 	}
 
-	for _, os := range selectableTargetOs {
+	for os := range PlatformOsMap {
 		if len(attrs.GetValueForOS(os).Includes) > 0 {
 			return true
 		}
@@ -253,8 +246,15 @@ type StringListAttribute struct {
 	// The base value of the string list attribute.
 	Value []string
 
-	// Optional additive set of list values to the base value.
+	// The arch-specific attribute string list values. Optional. If used, these
+	// are generated in a select statement and appended to the non-arch specific
+	// label list Value.
 	ArchValues stringListArchValues
+
+	// 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
 }
 
 // Arch-specific string_list typed Bazel attribute values. This should correspond
@@ -267,14 +267,29 @@ type stringListArchValues struct {
 	Common []string
 }
 
+type stringListOsValues struct {
+	Android     []string
+	Darwin      []string
+	Fuchsia     []string
+	Linux       []string
+	LinuxBionic []string
+	Windows     []string
+}
+
 // HasConfigurableValues returns true if the attribute contains
 // architecture-specific string_list values.
-func (attrs *StringListAttribute) HasConfigurableValues() bool {
+func (attrs StringListAttribute) HasConfigurableValues() bool {
-	for _, arch := range selectableArchs {
+	for arch := range PlatformArchMap {
 		if len(attrs.GetValueForArch(arch)) > 0 {
 			return true
 		}
 	}
+
+	for os := range PlatformOsMap {
+		if len(attrs.GetValueForOS(os)) > 0 {
+			return true
+		}
+	}
 	return false
 }
 
@@ -305,6 +320,35 @@ 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,
+	}
+}
+
+// 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 {
+		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 {
+		panic(fmt.Errorf("Unknown os: %s", os))
+	}
+	*v = value
+}
+
 // TryVariableSubstitution, replace string substitution formatting within each string in slice with
 // Starlark string.format compatible tag for productVariable.
 func TryVariableSubstitutions(slice []string, productVariable string) ([]string, bool) {

bp2build/build_conversion.go

@@ -415,12 +415,10 @@ func prettyPrint(propertyValue reflect.Value, indent int) (string, error) {
 	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.LabelListAttribute); ok {
+		if attr, ok := propertyValue.Interface().(bazel.Attribute); ok {
-			return prettyPrintLabelListAttribute(labels, indent)
+			return prettyPrintAttribute(attr, 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 {
-			return prettyPrintStringListAttribute(stringList, indent)
 		}
 
 		ret = "{\n"

bp2build/cc_object_conversion_test.go

@@ -412,6 +412,54 @@ func TestCcObjectConfigurableAttributesBp2Build(t *testing.T) {
         ],
         "//conditions:default": [],
     }),
+)`,
+			},
+		},
+		{
+			description:                        "cc_object setting cflags for multiple OSes",
+			moduleTypeUnderTest:                "cc_object",
+			moduleTypeUnderTestFactory:         cc.ObjectFactory,
+			moduleTypeUnderTestBp2BuildMutator: cc.ObjectBp2Build,
+			blueprint: `cc_object {
+    name: "foo",
+    srcs: ["base.cpp"],
+    target: {
+        android: {
+            cflags: ["-fPIC"],
+        },
+        windows: {
+            cflags: ["-fPIC"],
+        },
+        darwin: {
+            cflags: ["-Wall"],
+        },
+    },
+    bazel_module: { bp2build_available: true },
+}
+`,
+			expectedBazelTargets: []string{
+				`cc_object(
+    name = "foo",
+    copts = [
+        "-fno-addrsig",
+    ] + select({
+        "//build/bazel/platforms/os:android": [
+            "-fPIC",
+        ],
+        "//build/bazel/platforms/os:darwin": [
+            "-Wall",
+        ],
+        "//build/bazel/platforms/os:windows": [
+            "-fPIC",
+        ],
+        "//conditions:default": [],
+    }),
+    local_include_dirs = [
+        ".",
+    ],
+    srcs = [
+        "base.cpp",
+    ],
 )`,
 			},
 		},

bp2build/configurability.go

@@ -9,48 +9,67 @@ import (
 
 // Configurability support for bp2build.
 
-// prettyPrintStringListAttribute converts a StringListAttribute to its Bazel
+type selects map[string]reflect.Value
-// syntax. May contain a select statement.
+
-func prettyPrintStringListAttribute(stringList bazel.StringListAttribute, indent int) (string, error) {
+func getStringListValues(list bazel.StringListAttribute) (reflect.Value, selects, selects) {
-	ret, err := prettyPrint(reflect.ValueOf(stringList.Value), indent)
+	value := reflect.ValueOf(list.Value)
-	if err != nil {
+	if !list.HasConfigurableValues() {
-		return ret, err
+		return value, nil, nil
 	}
 
-	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)
+		archSelects[selectKey] = reflect.ValueOf(list.GetValueForArch(arch))
 	}
+
+	osSelects := map[string]reflect.Value{}
+	for os, selectKey := range bazel.PlatformOsMap {
+		osSelects[selectKey] = reflect.ValueOf(list.GetValueForOS(os))
+	}
+
+	return value, archSelects, osSelects
+}
+
+func getLabelListValues(list bazel.LabelListAttribute) (reflect.Value, selects, selects) {
+	value := reflect.ValueOf(list.Value.Includes)
+	if !list.HasConfigurableValues() {
+		return value, nil, nil
+	}
+
+	archSelects := map[string]reflect.Value{}
+	for arch, selectKey := range bazel.PlatformArchMap {
+		archSelects[selectKey] = reflect.ValueOf(list.GetValueForArch(arch).Includes)
+	}
+
+	osSelects := map[string]reflect.Value{}
+	for os, selectKey := range bazel.PlatformOsMap {
+		osSelects[selectKey] = reflect.ValueOf(list.GetValueForOS(os).Includes)
+	}
+
+	return value, archSelects, osSelects
+}
+
+// prettyPrintAttribute converts an Attribute to its Bazel syntax. May contain
+// select statements.
+func prettyPrintAttribute(v bazel.Attribute, indent int) (string, error) {
+	var value reflect.Value
+	var archSelects, osSelects selects
+
+	switch list := v.(type) {
+	case bazel.StringListAttribute:
+		value, archSelects, osSelects = getStringListValues(list)
+	case bazel.LabelListAttribute:
+		value, archSelects, osSelects = getLabelListValues(list)
+	default:
+		return "", fmt.Errorf("Not a supported Bazel attribute type: %s", v)
+	}
+
+	ret, err := prettyPrint(value, indent)
+	if err != nil {
+		return ret, err
+	}
+
+	// Create the selects for arch specific values.
 	selectMap, err := prettyPrintSelectMap(archSelects, "[]", indent)
 	if err != nil {
 		return "", err
@@ -58,17 +77,22 @@ func prettyPrintLabelListAttribute(labels bazel.LabelListAttribute, indent int)
 	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
+	if err != nil {
+		return "", err
+	}
+	ret += selectMap
+
+	return ret, 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) {
+	if selectMap == nil {
+		return "", nil
+	}
+
 	var selects string
 	for _, selectKey := range android.SortedStringKeys(selectMap) {
 		value := selectMap[selectKey]

cc/bp2build.go

@@ -58,6 +58,31 @@ func depsBp2BuildMutator(ctx android.BottomUpMutatorContext) {
 	ctx.AddDependency(module, nil, android.SortedUniqueStrings(allDeps)...)
 }
 
+// bp2buildParseCflags creates a label list attribute containing the cflags of a module, including
+func bp2BuildParseCflags(ctx android.TopDownMutatorContext, module *Module) bazel.StringListAttribute {
+	var ret bazel.StringListAttribute
+	for _, props := range module.compiler.compilerProps() {
+		if baseCompilerProps, ok := props.(*BaseCompilerProperties); ok {
+			ret.Value = baseCompilerProps.Cflags
+			break
+		}
+	}
+
+	for arch, props := range module.GetArchProperties(&BaseCompilerProperties{}) {
+		if baseCompilerProps, ok := props.(*BaseCompilerProperties); ok {
+			ret.SetValueForArch(arch.Name, baseCompilerProps.Cflags)
+		}
+	}
+
+	for os, props := range module.GetTargetProperties(&BaseCompilerProperties{}) {
+		if baseCompilerProps, ok := props.(*BaseCompilerProperties); ok {
+			ret.SetValueForOS(os.Name, baseCompilerProps.Cflags)
+		}
+	}
+
+	return ret
+}
+
 // 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 {

cc/library_headers.go

@@ -62,6 +62,7 @@ func prebuiltLibraryHeaderFactory() android.Module {
 }
 
 type bazelCcLibraryHeadersAttributes struct {
+	Copts    bazel.StringListAttribute
 	Hdrs     bazel.LabelListAttribute
 	Includes bazel.LabelListAttribute
 	Deps     bazel.LabelListAttribute
@@ -99,6 +100,7 @@ func CcLibraryHeadersBp2Build(ctx android.TopDownMutatorContext) {
 	headerLibsLabels := bp2BuildParseHeaderLibs(ctx, module)
 
 	attrs := &bazelCcLibraryHeadersAttributes{
+		Copts:    bp2BuildParseCflags(ctx, module),
 		Includes: exportedIncludesLabels,
 		Hdrs:     exportedIncludesHeadersLabels,
 		Deps:     headerLibsLabels,

cc/object.go

@@ -156,13 +156,11 @@ func ObjectBp2Build(ctx android.TopDownMutatorContext) {
 	}
 
 	// Set arch-specific configurable attributes
-	var copts bazel.StringListAttribute
 	var srcs bazel.LabelListAttribute
 	var localIncludeDirs []string
 	var asFlags []string
 	for _, props := range m.compiler.compilerProps() {
 		if baseCompilerProps, ok := props.(*BaseCompilerProperties); ok {
-			copts.Value = baseCompilerProps.Cflags
 			srcs = bazel.MakeLabelListAttribute(
 				android.BazelLabelForModuleSrcExcludes(
 					ctx,
@@ -205,14 +203,13 @@ func ObjectBp2Build(ctx android.TopDownMutatorContext) {
 	for arch, p := range m.GetArchProperties(&BaseCompilerProperties{}) {
 		if cProps, ok := p.(*BaseCompilerProperties); ok {
 			srcs.SetValueForArch(arch.Name, android.BazelLabelForModuleSrcExcludes(ctx, cProps.Srcs, cProps.Exclude_srcs))
-			copts.SetValueForArch(arch.Name, cProps.Cflags)
 		}
 	}
 
 	attrs := &bazelObjectAttributes{
 		Srcs:               srcs,
 		Deps:               deps,
-		Copts:              copts,
+		Copts:              bp2BuildParseCflags(ctx, m),
 		Asflags:            asFlags,
 		Local_include_dirs: localIncludeDirs,
 	}