From 9e2a5a7d6d311902803997582b67b8dc1495b865 Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Tue, 19 Sep 2023 08:41:14 -0400 Subject: [PATCH] Don't panic for unhandled product vars Instead, we return an error. This allows us to access some product variable information earlier when it will not be used as an attribute without panicing Test: m nothing Change-Id: Id094b2b9e1364a8d174d99b3824fa149fb235b3e --- android/module.go | 5 ++++- android/variable.go | 17 +++++++++++------ apex/apex.go | 5 ++++- bp2build/testing.go | 5 ++++- cc/bp2build.go | 5 ++++- etc/prebuilt_etc.go | 7 +++++-- genrule/genrule.go | 5 ++++- 7 files changed, 36 insertions(+), 13 deletions(-) diff --git a/android/module.go b/android/module.go index e6e5918a5..f4b51ea6d 100644 --- a/android/module.go +++ b/android/module.go @@ -1458,7 +1458,10 @@ func addCompatibilityConstraintForCompileMultilib(ctx *topDownMutatorContext, en // Returns a list of the constraint_value targets who enable this override. func productVariableConfigEnableAttribute(ctx *topDownMutatorContext) bazel.LabelListAttribute { result := bazel.LabelListAttribute{} - productVariableProps := ProductVariableProperties(ctx, ctx.Module()) + productVariableProps, errs := ProductVariableProperties(ctx, ctx.Module()) + for _, err := range errs { + ctx.ModuleErrorf("ProductVariableProperties error: %s", err) + } if productConfigProps, exists := productVariableProps["Enabled"]; exists { for productConfigProp, prop := range productConfigProps { flag, ok := prop.(*bool) diff --git a/android/variable.go b/android/variable.go index 6d9cc4b8d..e521ca21c 100644 --- a/android/variable.go +++ b/android/variable.go @@ -669,7 +669,8 @@ type ProductConfigProperties map[string]map[ProductConfigOrSoongConfigProperty]i // ProductVariableProperties returns a ProductConfigProperties containing only the properties which // have been set for the given module. -func ProductVariableProperties(ctx ArchVariantContext, module Module) ProductConfigProperties { +func ProductVariableProperties(ctx ArchVariantContext, module Module) (ProductConfigProperties, []error) { + var errs []error moduleBase := module.base() productConfigProperties := ProductConfigProperties{} @@ -693,12 +694,15 @@ func ProductVariableProperties(ctx ArchVariantContext, module Module) ProductCon for namespace, namespacedVariableProps := range m.namespacedVariableProps() { for _, namespacedVariableProp := range namespacedVariableProps { variableValues := reflect.ValueOf(namespacedVariableProp).Elem().FieldByName(soongconfig.SoongConfigProperty) - productConfigProperties.AddSoongConfigProperties(namespace, variableValues) + err := productConfigProperties.AddSoongConfigProperties(namespace, variableValues) + if err != nil { + errs = append(errs, err) + } } } } - return productConfigProperties + return productConfigProperties, errs } func (p *ProductConfigProperties) AddProductConfigProperty( @@ -820,7 +824,7 @@ func (productConfigProperties *ProductConfigProperties) AddProductConfigProperti } -func (productConfigProperties *ProductConfigProperties) AddSoongConfigProperties(namespace string, soongConfigVariablesStruct reflect.Value) { +func (productConfigProperties *ProductConfigProperties) AddSoongConfigProperties(namespace string, soongConfigVariablesStruct reflect.Value) error { // // Example of soong_config_variables: // @@ -917,7 +921,7 @@ func (productConfigProperties *ProductConfigProperties) AddSoongConfigProperties if propertyName == "Target" { productConfigProperties.AddSoongConfigPropertiesFromTargetStruct(namespace, variableName, proptools.PropertyNameForField(propertyOrValueName), field.Field(k)) } else if propertyName == "Arch" || propertyName == "Multilib" { - panic("Arch/Multilib are not currently supported in soong config variable structs") + return fmt.Errorf("Arch/Multilib are not currently supported in soong config variable structs") } else { productConfigProperties.AddSoongConfigProperty(propertyName, namespace, variableName, proptools.PropertyNameForField(propertyOrValueName), "", field.Field(k).Interface()) } @@ -928,13 +932,14 @@ func (productConfigProperties *ProductConfigProperties) AddSoongConfigProperties if propertyOrValueName == "Target" { productConfigProperties.AddSoongConfigPropertiesFromTargetStruct(namespace, variableName, "", propertyOrStruct) } else if propertyOrValueName == "Arch" || propertyOrValueName == "Multilib" { - panic("Arch/Multilib are not currently supported in soong config variable structs") + return fmt.Errorf("Arch/Multilib are not currently supported in soong config variable structs") } else { productConfigProperties.AddSoongConfigProperty(propertyOrValueName, namespace, variableName, "", "", propertyOrStruct.Interface()) } } } } + return nil } func (productConfigProperties *ProductConfigProperties) AddSoongConfigPropertiesFromTargetStruct(namespace, soongConfigVariableName string, soongConfigVariableValue string, targetStruct reflect.Value) { diff --git a/apex/apex.go b/apex/apex.go index a116b85ee..aaa55bb3e 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -3298,7 +3298,10 @@ func convertWithBp2build(a *apexBundle, ctx android.TopDownMutatorContext) (baze cannedFsConfigAttribute.SetValue(android.BazelLabelForModuleSrcSingle(ctx, *a.properties.Canned_fs_config)) } - productVariableProps := android.ProductVariableProperties(ctx, a) + productVariableProps, errs := android.ProductVariableProperties(ctx, a) + for _, err := range errs { + ctx.ModuleErrorf("ProductVariableProperties error: %s", err) + } // TODO(b/219503907) this would need to be set to a.MinSdkVersionValue(ctx) but // given it's coming via config, we probably don't want to put it in here. var minSdkVersion bazel.StringAttribute diff --git a/bp2build/testing.go b/bp2build/testing.go index dbabc067e..eca602279 100644 --- a/bp2build/testing.go +++ b/bp2build/testing.go @@ -517,7 +517,10 @@ func (m *customModule) ConvertWithBp2build(ctx android.TopDownMutatorContext) { } } } - productVariableProps := android.ProductVariableProperties(ctx, ctx.Module()) + productVariableProps, errs := android.ProductVariableProperties(ctx, ctx.Module()) + for _, err := range errs { + ctx.ModuleErrorf("ProductVariableProperties error: %s", err) + } if props, ok := productVariableProps["String_literal_prop"]; ok { for c, p := range props { if val, ok := p.(*string); ok { diff --git a/cc/bp2build.go b/cc/bp2build.go index aacc0884b..6a49915aa 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -944,7 +944,10 @@ func bp2BuildParseBaseProps(ctx android.Bp2buildMutatorContext, module *Module) nativeCoverage = BoolPtr(false) } - productVariableProps := android.ProductVariableProperties(ctx, ctx.Module()) + productVariableProps, errs := android.ProductVariableProperties(ctx, ctx.Module()) + for _, err := range errs { + ctx.ModuleErrorf("ProductVariableProperties error: %s", err) + } (&compilerAttrs).convertProductVariables(ctx, productVariableProps) (&linkerAttrs).convertProductVariables(ctx, productVariableProps) diff --git a/etc/prebuilt_etc.go b/etc/prebuilt_etc.go index c48bafa26..c6318d68d 100644 --- a/etc/prebuilt_etc.go +++ b/etc/prebuilt_etc.go @@ -730,8 +730,11 @@ func (module *PrebuiltEtc) Bp2buildHelper(ctx android.TopDownMutatorContext) (*b src.SetSelectValue(axis, config, label) } } - - for propName, productConfigProps := range android.ProductVariableProperties(ctx, ctx.Module()) { + productVarProperties, errs := android.ProductVariableProperties(ctx, ctx.Module()) + for _, err := range errs { + ctx.ModuleErrorf("ProductVariableProperties error: %s", err) + } + for propName, productConfigProps := range productVarProperties { for configProp, propVal := range productConfigProps { if propName == "Src" { props, ok := propVal.(*string) diff --git a/genrule/genrule.go b/genrule/genrule.go index d1c2f1378..0d484c4e7 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -993,7 +993,10 @@ func (m *Module) ConvertWithBp2build(ctx android.TopDownMutatorContext) { var cmdProp bazel.StringAttribute cmdProp.SetValue(replaceVariables(proptools.String(m.properties.Cmd))) - allProductVariableProps := android.ProductVariableProperties(ctx, m) + allProductVariableProps, errs := android.ProductVariableProperties(ctx, m) + for _, err := range errs { + ctx.ModuleErrorf("ProductVariableProperties error: %s", err) + } if productVariableProps, ok := allProductVariableProps["Cmd"]; ok { for productVariable, value := range productVariableProps { var cmd string