Merge changes I82d3a384,I3b03f0a3

* changes:
  product_variables srcs in prebuilt_etc
  refactoring build_conversion.go to remove panic

android/variable.go

@@ -592,6 +592,9 @@ type ProductConfigProperty struct {
 	// "acme__board__soc_a", "acme__board__soc_b", and
 	// "acme__board__conditions_default"
 	FullConfig string
+
+	// keeps track of whether this product variable is nested under an arch variant
+	OuterAxis bazel.ConfigurationAxis
 }
 
 func (p *ProductConfigProperty) AlwaysEmit() bool {
@@ -600,11 +603,11 @@ func (p *ProductConfigProperty) AlwaysEmit() bool {
 
 func (p *ProductConfigProperty) ConfigurationAxis() bazel.ConfigurationAxis {
 	if p.Namespace == "" {
-		return bazel.ProductVariableConfigurationAxis(p.FullConfig)
+		return bazel.ProductVariableConfigurationAxis(p.FullConfig, p.OuterAxis)
 	} else {
 		// Soong config variables can be uniquely identified by the namespace
 		// (e.g. acme, android) and the product variable name (e.g. board, size)
-		return bazel.ProductVariableConfigurationAxis(p.Namespace + "__" + p.Name)
+		return bazel.ProductVariableConfigurationAxis(p.Namespace+"__"+p.Name, bazel.NoConfigAxis)
 	}
 }
 
@@ -663,9 +666,11 @@ func ProductVariableProperties(ctx BazelConversionPathContext) ProductConfigProp
 			moduleBase.variableProperties,
 			"",
 			"",
-			&productConfigProperties)
+			&productConfigProperties,
+			bazel.ConfigurationAxis{},
+		)
 
-		for _, configToProps := range moduleBase.GetArchVariantProperties(ctx, moduleBase.variableProperties) {
+		for axis, configToProps := range moduleBase.GetArchVariantProperties(ctx, moduleBase.variableProperties) {
 			for config, props := range configToProps {
 				// GetArchVariantProperties is creating an instance of the requested type
 				// and productVariablesValues expects an interface, so no need to cast
@@ -674,7 +679,8 @@ func ProductVariableProperties(ctx BazelConversionPathContext) ProductConfigProp
 					props,
 					"",
 					config,
-					&productConfigProperties)
+					&productConfigProperties,
+					axis)
 			}
 		}
 	}
@@ -687,7 +693,8 @@ func ProductVariableProperties(ctx BazelConversionPathContext) ProductConfigProp
 					namespacedVariableProp,
 					namespace,
 					"",
-					&productConfigProperties)
+					&productConfigProperties,
+					bazel.NoConfigAxis)
 			}
 		}
 	}
@@ -803,6 +810,7 @@ func (props *ProductConfigProperties) zeroValuesForNamespacedVariables() {
 					p.Name,
 					p.FullConfig,
 					zeroValue,
+					bazel.NoConfigAxis,
 				)
 			}
 		}
@@ -810,7 +818,7 @@ func (props *ProductConfigProperties) zeroValuesForNamespacedVariables() {
 }
 
 func (p *ProductConfigProperties) AddProductConfigProperty(
-	propertyName, namespace, productVariableName, config string, property interface{}) {
+	propertyName, namespace, productVariableName, config string, property interface{}, outerAxis bazel.ConfigurationAxis) {
 	if (*p)[propertyName] == nil {
 		(*p)[propertyName] = make(map[ProductConfigProperty]interface{})
 	}
@@ -819,6 +827,7 @@ func (p *ProductConfigProperties) AddProductConfigProperty(
 		Namespace:  namespace,           // e.g. acme, android
 		Name:       productVariableName, // e.g. size, feature1, feature2, FEATURE3, board
 		FullConfig: config,              // e.g. size, feature1-x86, size__conditions_default
+		OuterAxis:  outerAxis,
 	}
 
 	if existing, ok := (*p)[propertyName][productConfigProp]; ok && namespace != "" {
@@ -869,7 +878,7 @@ func maybeExtractConfigVarProp(v reflect.Value) (reflect.Value, bool) {
 	return v, true
 }
 
-func (productConfigProperties *ProductConfigProperties) AddProductConfigProperties(namespace, suffix string, variableValues reflect.Value) {
+func (productConfigProperties *ProductConfigProperties) AddProductConfigProperties(namespace, suffix string, variableValues reflect.Value, outerAxis bazel.ConfigurationAxis) {
 	// variableValues can either be a product_variables or
 	// soong_config_variables struct.
 	//
@@ -974,7 +983,8 @@ func (productConfigProperties *ProductConfigProperties) AddProductConfigProperti
 						namespace,           // e.g. acme, android
 						productVariableName, // e.g. size, feature1, FEATURE2, board
 						config,
-						field.Field(k).Interface(), // e.g. ["-DDEFAULT"], ["foo", "bar"]
+						field.Field(k).Interface(), // e.g. ["-DDEFAULT"], ["foo", "bar"],
+						outerAxis,
 					)
 				}
 			} else if property.Kind() != reflect.Interface {
@@ -988,6 +998,7 @@ func (productConfigProperties *ProductConfigProperties) AddProductConfigProperti
 					productVariableName,
 					config,
 					property.Interface(),
+					outerAxis,
 				)
 			}
 		}
@@ -998,14 +1009,14 @@ func (productConfigProperties *ProductConfigProperties) AddProductConfigProperti
 // product_variables and soong_config_variables to structs that can be generated
 // as select statements.
 func productVariableValues(
-	fieldName string, variableProps interface{}, namespace, suffix string, productConfigProperties *ProductConfigProperties) {
+	fieldName string, variableProps interface{}, namespace, suffix string, productConfigProperties *ProductConfigProperties, outerAxis bazel.ConfigurationAxis) {
 	if suffix != "" {
 		suffix = "-" + suffix
 	}
 
 	// variableValues represent the product_variables or soong_config_variables struct.
 	variableValues := reflect.ValueOf(variableProps).Elem().FieldByName(fieldName)
-	productConfigProperties.AddProductConfigProperties(namespace, suffix, variableValues)
+	productConfigProperties.AddProductConfigProperties(namespace, suffix, variableValues, outerAxis)
 }
 
 func VariableMutator(mctx BottomUpMutatorContext) {

bazel/configurability.go

@@ -295,10 +295,11 @@ var (
 )
 
 // ProductVariableConfigurationAxis returns an axis for the given product variable
-func ProductVariableConfigurationAxis(variable string) ConfigurationAxis {
+func ProductVariableConfigurationAxis(variable string, outerAxis ConfigurationAxis) ConfigurationAxis {
 	return ConfigurationAxis{
 		configurationType: productVariables,
 		subType:           variable,
+		outerAxisType:     outerAxis.configurationType,
 	}
 }
 
@@ -309,6 +310,8 @@ type ConfigurationAxis struct {
 	// some configuration types (e.g. productVariables) have multiple independent axes, subType helps
 	// distinguish between them without needing to list all 17 product variables.
 	subType string
+	// used to keep track of which product variables are arch variant
+	outerAxisType configurationType
 }
 
 func (ca *ConfigurationAxis) less(other ConfigurationAxis) bool {

bazel/properties.go

@@ -309,7 +309,19 @@ func (la *LabelAttribute) Collapse() error {
 	_, containsProductVariables := axisTypes[productVariables]
 	if containsProductVariables {
 		if containsOs || containsArch || containsOsArch {
-			return fmt.Errorf("label attribute could not be collapsed as it has two or more unrelated axes")
+			if containsArch {
+				allProductVariablesAreArchVariant := true
+				for k := range la.ConfigurableValues {
+					if k.configurationType == productVariables && k.outerAxisType != arch {
+						allProductVariablesAreArchVariant = false
+					}
+				}
+				if !allProductVariablesAreArchVariant {
+					return fmt.Errorf("label attribute could not be collapsed as it has two or more unrelated axes")
+				}
+			} else {
+				return fmt.Errorf("label attribute could not be collapsed as it has two or more unrelated axes")
+			}
 		}
 	}
 	if (containsOs && containsArch) || (containsOsArch && (containsOs || containsArch)) {

bazel/properties_test.go

@@ -247,13 +247,13 @@ func TestResolveExcludes(t *testing.T) {
 			OsArchConfigurationAxis: labelListSelectValues{
 				"linux_x86": makeLabelList([]string{"linux_x86_include"}, []string{}),
 			},
-			ProductVariableConfigurationAxis("product_with_defaults"): labelListSelectValues{
+			ProductVariableConfigurationAxis("product_with_defaults", NoConfigAxis): labelListSelectValues{
 				"a":                        makeLabelList([]string{}, []string{"not_in_value"}),
 				"b":                        makeLabelList([]string{"b_val"}, []string{}),
 				"c":                        makeLabelList([]string{"c_val"}, []string{}),
 				ConditionsDefaultConfigKey: makeLabelList([]string{"c_val", "default", "default2"}, []string{}),
 			},
-			ProductVariableConfigurationAxis("product_only_with_excludes"): labelListSelectValues{
+			ProductVariableConfigurationAxis("product_only_with_excludes", NoConfigAxis): labelListSelectValues{
 				"a": makeLabelList([]string{}, []string{"not_in_value"}),
 			},
 		},
@@ -281,7 +281,7 @@ func TestResolveExcludes(t *testing.T) {
 			"linux_x86":                makeLabels("linux_x86_include"),
 			ConditionsDefaultConfigKey: nilLabels,
 		},
-		ProductVariableConfigurationAxis("product_with_defaults"): {
+		ProductVariableConfigurationAxis("product_with_defaults", NoConfigAxis): {
 			"a":                        nilLabels,
 			"b":                        makeLabels("b_val"),
 			"c":                        makeLabels("c_val"),
@@ -674,7 +674,7 @@ func TestDeduplicateAxesFromBase(t *testing.T) {
 			OsArchConfigurationAxis: stringListSelectValues{
 				"linux_x86": {"linux_x86_include"},
 			},
-			ProductVariableConfigurationAxis("a"): stringListSelectValues{
+			ProductVariableConfigurationAxis("a", NoConfigAxis): stringListSelectValues{
 				"a": []string{"not_in_value"},
 			},
 		},
@@ -699,7 +699,7 @@ func TestDeduplicateAxesFromBase(t *testing.T) {
 			"linux": []string{"linux_include"},
 		},
 		OsArchConfigurationAxis: stringListSelectValues{},
-		ProductVariableConfigurationAxis("a"): stringListSelectValues{
+		ProductVariableConfigurationAxis("a", NoConfigAxis): stringListSelectValues{
 			"a": []string{"not_in_value"},
 		},
 	}

bp2build/build_conversion.go

@@ -289,7 +289,9 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers
 						return
 					}
 				}
-				targets = generateBazelTargets(bpCtx, aModule)
+				var targetErrs []error
+				targets, targetErrs = generateBazelTargets(bpCtx, aModule)
+				errs = append(errs, targetErrs...)
 				for _, t := range targets {
 					// A module can potentially generate more than 1 Bazel
 					// target, each of a different rule class.
@@ -306,7 +308,10 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers
 				// be mapped cleanly to a bazel label.
 				return
 			}
-			t := generateSoongModuleTarget(bpCtx, m)
+			t, err := generateSoongModuleTarget(bpCtx, m)
+			if err != nil {
+				errs = append(errs, err)
+			}
 			targets = append(targets, t)
 		default:
 			errs = append(errs, fmt.Errorf("Unknown code-generation mode: %s", ctx.Mode()))
@@ -347,12 +352,18 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers
 	}, errs
 }
 
-func generateBazelTargets(ctx bpToBuildContext, m android.Module) []BazelTarget {
+func generateBazelTargets(ctx bpToBuildContext, m android.Module) ([]BazelTarget, []error) {
 	var targets []BazelTarget
+	var errs []error
 	for _, m := range m.Bp2buildTargets() {
-		targets = append(targets, generateBazelTarget(ctx, m))
+		target, err := generateBazelTarget(ctx, m)
+		if err != nil {
+			errs = append(errs, err)
+			return targets, errs
+		}
+		targets = append(targets, target)
 	}
-	return targets
+	return targets, errs
 }
 
 type bp2buildModule interface {
@@ -363,13 +374,16 @@ type bp2buildModule interface {
 	BazelAttributes() []interface{}
 }
 
-func generateBazelTarget(ctx bpToBuildContext, m bp2buildModule) BazelTarget {
+func generateBazelTarget(ctx bpToBuildContext, m bp2buildModule) (BazelTarget, error) {
 	ruleClass := m.BazelRuleClass()
 	bzlLoadLocation := m.BazelRuleLoadLocation()
 
 	// extract the bazel attributes from the module.
 	attrs := m.BazelAttributes()
-	props := extractModuleProperties(attrs, true)
+	props, err := extractModuleProperties(attrs, true)
+	if err != nil {
+		return BazelTarget{}, err
+	}
 
 	// name is handled in a special manner
 	delete(props.Attrs, "name")
@@ -389,13 +403,13 @@ func generateBazelTarget(ctx bpToBuildContext, m bp2buildModule) BazelTarget {
 			targetName,
 			attributes,
 		),
-	}
+	}, nil
 }
 
 // Convert a module and its deps and props into a Bazel macro/rule
 // representation in the BUILD file.
-func generateSoongModuleTarget(ctx bpToBuildContext, m blueprint.Module) BazelTarget {
+func generateSoongModuleTarget(ctx bpToBuildContext, m blueprint.Module) (BazelTarget, error) {
-	props := getBuildProperties(ctx, m)
+	props, err := getBuildProperties(ctx, m)
 
 	// TODO(b/163018919): DirectDeps can have duplicate (module, variant)
 	// items, if the modules are added using different DependencyTag. Figure
@@ -429,21 +443,21 @@ func generateSoongModuleTarget(ctx bpToBuildContext, m blueprint.Module) BazelTa
 			ctx.ModuleSubDir(m),
 			depLabelList,
 			attributes),
-	}
+	}, err
 }
 
-func getBuildProperties(ctx bpToBuildContext, m blueprint.Module) BazelAttributes {
+func getBuildProperties(ctx bpToBuildContext, m blueprint.Module) (BazelAttributes, error) {
 	// TODO: this omits properties for blueprint modules (blueprint_go_binary,
 	// bootstrap_go_binary, bootstrap_go_package), which will have to be handled separately.
 	if aModule, ok := m.(android.Module); ok {
 		return extractModuleProperties(aModule.GetProperties(), false)
 	}
 
-	return BazelAttributes{}
+	return BazelAttributes{}, nil
 }
 
 // Generically extract module properties and types into a map, keyed by the module property name.
-func extractModuleProperties(props []interface{}, checkForDuplicateProperties bool) BazelAttributes {
+func extractModuleProperties(props []interface{}, checkForDuplicateProperties bool) (BazelAttributes, error) {
 	ret := map[string]string{}
 
 	// Iterate over this android.Module's property structs.
@@ -456,24 +470,29 @@ func extractModuleProperties(props []interface{}, checkForDuplicateProperties bo
 		// manipulate internal props, if needed.
 		if isStructPtr(propertiesValue.Type()) {
 			structValue := propertiesValue.Elem()
-			for k, v := range extractStructProperties(structValue, 0) {
+			ok, err := extractStructProperties(structValue, 0)
+			if err != nil {
+				return BazelAttributes{}, err
+			}
+			for k, v := range ok {
 				if existing, exists := ret[k]; checkForDuplicateProperties && exists {
-					panic(fmt.Errorf(
+					return BazelAttributes{}, fmt.Errorf(
 						"%s (%v) is present in properties whereas it should be consolidated into a commonAttributes",
-						k, existing))
+						k, existing)
 				}
 				ret[k] = v
 			}
 		} else {
-			panic(fmt.Errorf(
+			return BazelAttributes{},
-				"properties must be a pointer to a struct, got %T",
+				fmt.Errorf(
-				propertiesValue.Interface()))
+					"properties must be a pointer to a struct, got %T",
+					propertiesValue.Interface())
 		}
 	}
 
 	return BazelAttributes{
 		Attrs: ret,
-	}
+	}, nil
 }
 
 func isStructPtr(t reflect.Type) bool {
@@ -531,7 +550,12 @@ func prettyPrint(propertyValue reflect.Value, indent int, emitZeroValues bool) (
 		}
 
 		// Sort and print the struct props by the key.
-		structProps := extractStructProperties(propertyValue, indent)
+		structProps, err := extractStructProperties(propertyValue, indent)
+
+		if err != nil {
+			return "", err
+		}
+
 		if len(structProps) == 0 {
 			return "", nil
 		}
@@ -550,11 +574,13 @@ func prettyPrint(propertyValue reflect.Value, indent int, emitZeroValues bool) (
 // which each property value correctly pretty-printed and indented at the right nest level,
 // since property structs can be nested. In Starlark, nested structs are represented as nested
 // dicts: https://docs.bazel.build/skylark/lib/dict.html
-func extractStructProperties(structValue reflect.Value, indent int) map[string]string {
+func extractStructProperties(structValue reflect.Value, indent int) (map[string]string, error) {
 	if structValue.Kind() != reflect.Struct {
-		panic(fmt.Errorf("Expected a reflect.Struct type, but got %s", structValue.Kind()))
+		return map[string]string{}, fmt.Errorf("Expected a reflect.Struct type, but got %s", structValue.Kind())
 	}
 
+	var err error
+
 	ret := map[string]string{}
 	structType := structValue.Type()
 	for i := 0; i < structValue.NumField(); i++ {
@@ -575,7 +601,10 @@ func extractStructProperties(structValue reflect.Value, indent int) map[string]s
 				fieldValue = fieldValue.Elem()
 			}
 			if fieldValue.Type().Kind() == reflect.Struct {
-				propsToMerge := extractStructProperties(fieldValue, indent)
+				propsToMerge, err := extractStructProperties(fieldValue, indent)
+				if err != nil {
+					return map[string]string{}, err
+				}
 				for prop, value := range propsToMerge {
 					ret[prop] = value
 				}
@@ -584,20 +613,20 @@ func extractStructProperties(structValue reflect.Value, indent int) map[string]s
 		}
 
 		propertyName := proptools.PropertyNameForField(field.Name)
-		prettyPrintedValue, err := prettyPrint(fieldValue, indent+1, false)
+		var prettyPrintedValue string
+		prettyPrintedValue, err = prettyPrint(fieldValue, indent+1, false)
 		if err != nil {
-			panic(
+			return map[string]string{}, fmt.Errorf(
-				fmt.Errorf(
+				"Error while parsing property: %q. %s",
-					"Error while parsing property: %q. %s",
+				propertyName,
-					propertyName,
+				err)
-					err))
 		}
 		if prettyPrintedValue != "" {
 			ret[propertyName] = prettyPrintedValue
 		}
 	}
 
-	return ret
+	return ret, nil
 }
 
 func isZero(value reflect.Value) bool {

bp2build/prebuilt_etc_conversion_test.go

@@ -15,10 +15,11 @@
 package bp2build
 
 import (
+	"fmt"
+	"testing"
+
 	"android/soong/android"
 	"android/soong/etc"
-
-	"testing"
 )
 
 func runPrebuiltEtcTestCase(t *testing.T, tc Bp2buildTestCase) {
@@ -128,6 +129,32 @@ prebuilt_etc {
 				"dir": `"etc/tz"`,
 			})}})
 }
+func TestPrebuiltEtcProductVariables(t *testing.T) {
+	runPrebuiltEtcTestCase(t, Bp2buildTestCase{
+		Description: "prebuilt etc - product variables",
+		Filesystem:  map[string]string{},
+		Blueprint: `
+prebuilt_etc {
+    name: "apex_tz_version",
+    src: "version/tz_version",
+    filename: "tz_version",
+    product_variables: {
+      native_coverage: {
+        src: "src1",
+      },
+    },
+}
+`,
+		ExpectedBazelTargets: []string{
+			MakeBazelTarget("prebuilt_file", "apex_tz_version", AttrNameToString{
+				"filename": `"tz_version"`,
+				"src": `select({
+        "//build/bazel/product_variables:native_coverage": "src1",
+        "//conditions:default": "version/tz_version",
+    })`,
+				"dir": `"etc"`,
+			})}})
+}
 
 func runPrebuiltUsrShareTestCase(t *testing.T, tc Bp2buildTestCase) {
 	t.Helper()
@@ -265,3 +292,57 @@ prebuilt_etc {
 				"dir":      `"etc"`,
 			})}})
 }
+
+func TestPrebuiltEtcProductVariableArchSrcs(t *testing.T) {
+	runPrebuiltEtcTestCase(t, Bp2buildTestCase{
+		Description: "prebuilt etc- SRcs from arch variant product variables",
+		Filesystem:  map[string]string{},
+		Blueprint: `
+prebuilt_etc {
+    name: "foo",
+    filename: "fooFilename",
+    arch: {
+      arm: {
+        src: "armSrc",
+        product_variables: {
+          native_coverage: {
+            src: "nativeCoverageArmSrc",
+          },
+        },
+      },
+    },
+}`,
+		ExpectedBazelTargets: []string{
+			MakeBazelTarget("prebuilt_file", "foo", AttrNameToString{
+				"filename": `"fooFilename"`,
+				"dir":      `"etc"`,
+				"src": `select({
+        "//build/bazel/platforms/arch:arm": "armSrc",
+        "//build/bazel/product_variables:native_coverage-arm": "nativeCoverageArmSrc",
+        "//conditions:default": None,
+    })`,
+			})}})
+}
+
+func TestPrebuiltEtcProductVariableError(t *testing.T) {
+	runPrebuiltEtcTestCase(t, Bp2buildTestCase{
+		Description: "",
+		Filesystem:  map[string]string{},
+		Blueprint: `
+prebuilt_etc {
+    name: "foo",
+    filename: "fooFilename",
+    arch: {
+      arm: {
+        src: "armSrc",
+      },
+    },
+    product_variables: {
+      native_coverage: {
+        src: "nativeCoverageArmSrc",
+      },
+    },
+}`,
+		ExpectedErr: fmt.Errorf("label attribute could not be collapsed"),
+	})
+}

bp2build/testing.go

@@ -384,6 +384,9 @@ func customBp2buildOneToMany(ctx android.TopDownMutatorContext, m *customModule)
 func generateBazelTargetsForDir(codegenCtx *CodegenContext, dir string) (BazelTargets, []error) {
 	// TODO: Set generateFilegroups to true and/or remove the generateFilegroups argument completely
 	res, err := GenerateBazelTargets(codegenCtx, false)
+	if err != nil {
+		return BazelTargets{}, err
+	}
 	return res.buildFileToTargets[dir], err
 }
 

etc/prebuilt_etc.go

@@ -31,6 +31,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"path/filepath"
+	"reflect"
 	"strings"
 
 	"github.com/google/blueprint/proptools"
@@ -692,6 +693,22 @@ func (module *PrebuiltEtc) ConvertWithBp2build(ctx android.TopDownMutatorContext
 				src.SetSelectValue(axis, config, label)
 			}
 		}
+
+		for propName, productConfigProps := range android.ProductVariableProperties(ctx) {
+			for configProp, propVal := range productConfigProps {
+				if propName == "Src" {
+					props, ok := propVal.(*string)
+					if !ok {
+						ctx.PropertyErrorf(" Expected Property to have type string, but was %s\n", reflect.TypeOf(propVal).String())
+						continue
+					}
+					if props != nil {
+						label := android.BazelLabelForModuleSrcSingle(ctx, *props)
+						src.SetSelectValue(configProp.ConfigurationAxis(), configProp.SelectKey(), label)
+					}
+				}
+			}
+		}
 	}
 
 	var filename string