diff options
author | Cole Faust <colefaust@google.com> | 2024-04-17 10:24:51 -0700 |
---|---|---|
committer | Cole Faust <colefaust@google.com> | 2024-04-25 15:31:00 -0700 |
commit | 4560bb086e629254686c6550c8c66c425b8570ed (patch) | |
tree | b44102d1426f421ca35b5364510ff4bdbca556ee | |
parent | 36b63229797b63e76d04b8bdedf1cdf8eb865327 (diff) | |
download | blueprint-4560bb086e629254686c6550c8c66c425b8570ed.tar.gz |
Allow extending configurable propeties with non-configurable properties
Sometimes modules add arch-variant properties in load hooks, to disable
modules by default on certain platforms for example. When changing the
property to a Configurable property, these load hooks would also need
to be changed in order to have a matching type for
ExtendMatchingProperties.
Since this can be kindof a pain to address everywhere, for now,
special case the extension functions to promote non-configurable
properties to configurable ones. We can remove this later when
everything switches to configurable properties.
Bug: 323382414
Test: go tests
Change-Id: Iac96587dbd60ccdd6aa667dd69a71ad252abe589
-rw-r--r-- | proptools/configurable.go | 18 | ||||
-rw-r--r-- | proptools/extend.go | 49 | ||||
-rw-r--r-- | proptools/extend_test.go | 132 |
3 files changed, 197 insertions, 2 deletions
diff --git a/proptools/configurable.go b/proptools/configurable.go index f521ab4..9fe79c3 100644 --- a/proptools/configurable.go +++ b/proptools/configurable.go @@ -257,6 +257,24 @@ func configurableCaseType(configuredType reflect.Type) reflect.Type { panic("unimplemented") } +// for the given T, return the reflect.type of Configurable[T] +func configurableType(configuredType reflect.Type) (reflect.Type, error) { + // I don't think it's possible to do this generically with go's + // current reflection apis unfortunately + switch configuredType.Kind() { + case reflect.String: + return reflect.TypeOf(Configurable[string]{}), nil + case reflect.Bool: + return reflect.TypeOf(Configurable[bool]{}), nil + case reflect.Slice: + switch configuredType.Elem().Kind() { + case reflect.String: + return reflect.TypeOf(Configurable[[]string]{}), nil + } + } + return nil, fmt.Errorf("configurable structs can only contain strings, bools, or string slices, found %s", configuredType.String()) +} + // Configurable can wrap the type of a blueprint property, // in order to allow select statements to be used in bp files // for that property. For example, for the property struct: diff --git a/proptools/extend.go b/proptools/extend.go index 110fb24..68c04a3 100644 --- a/proptools/extend.go +++ b/proptools/extend.go @@ -384,12 +384,16 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value continue } case reflect.Bool, reflect.String, reflect.Slice, reflect.Map: - if srcFieldValue.Type() != dstFieldValue.Type() { + // If the types don't match or srcFieldValue cannot be converted to a Configurable type, it's an error + ct, err := configurableType(srcFieldValue.Type()) + if srcFieldValue.Type() != dstFieldValue.Type() && (err != nil || dstFieldValue.Type() != ct) { return extendPropertyErrorf(propertyName(srcField), "mismatched types %s and %s", dstFieldValue.Type(), srcFieldValue.Type()) } case reflect.Ptr: - if srcFieldValue.Type() != dstFieldValue.Type() { + // If the types don't match or srcFieldValue cannot be converted to a Configurable type, it's an error + ct, err := configurableType(srcFieldValue.Type().Elem()) + if srcFieldValue.Type() != dstFieldValue.Type() && (err != nil || dstFieldValue.Type() != ct) { return extendPropertyErrorf(propertyName(srcField), "mismatched types %s and %s", dstFieldValue.Type(), srcFieldValue.Type()) } @@ -457,6 +461,47 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value func ExtendBasicType(dstFieldValue, srcFieldValue reflect.Value, order Order) { prepend := order == Prepend || order == Prepend_replace + if !srcFieldValue.IsValid() { + return + } + + // If dst is a Configurable and src isn't, promote src to a Configurable. + // This isn't necessary if all property structs are using Configurable values, + // but it's helpful to avoid having to change as many places in the code when + // converting properties to Configurable properties. For example, load hooks + // make their own mini-property structs and append them onto the main property + // structs when they want to change the default values of properties. + srcFieldType := srcFieldValue.Type() + if isConfigurable(dstFieldValue.Type()) && !isConfigurable(srcFieldType) { + var value reflect.Value + if srcFieldType.Kind() == reflect.Pointer { + srcFieldType = srcFieldType.Elem() + if srcFieldValue.IsNil() { + value = srcFieldValue + } else { + // Copy the pointer + value = reflect.New(srcFieldType) + value.Elem().Set(srcFieldValue.Elem()) + } + } else { + value = reflect.New(srcFieldType) + value.Elem().Set(srcFieldValue) + } + caseType := configurableCaseType(srcFieldType) + case_ := reflect.New(caseType) + case_.Interface().(configurableCaseReflection).initialize(nil, value.Interface()) + cases := reflect.MakeSlice(reflect.SliceOf(caseType), 0, 1) + cases = reflect.Append(cases, case_.Elem()) + ct, err := configurableType(srcFieldType) + if err != nil { + // Should be unreachable due to earlier checks + panic(err.Error()) + } + temp := reflect.New(ct) + temp.Interface().(configurablePtrReflection).initialize("", nil, cases.Interface()) + srcFieldValue = temp.Elem() + } + switch srcFieldValue.Kind() { case reflect.Struct: if !isConfigurable(srcFieldValue.Type()) { diff --git a/proptools/extend_test.go b/proptools/extend_test.go index b2920f5..71a0bc2 100644 --- a/proptools/extend_test.go +++ b/proptools/extend_test.go @@ -1869,6 +1869,138 @@ func appendMatchingPropertiesTestCases() []appendMatchingPropertiesTestCase { }, err: extendPropertyErrorf("s", "mismatched types []int and []string"), }, + { + name: "Append *bool to Configurable[bool]", + order: Append, + dst: []interface{}{ + &struct{ S Configurable[bool] }{ + S: Configurable[bool]{ + conditions: []ConfigurableCondition{{ + FunctionName: "soong_config_variable", + Args: []string{ + "my_namespace", + "foo", + }, + }}, + cases: []ConfigurableCase[bool]{{ + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "a", + }}, + value: BoolPtr(true), + }, { + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeDefault, + }}, + value: BoolPtr(false), + }}, + appendWrapper: &appendWrapper[bool]{}, + }, + }, + }, + src: &struct{ S *bool }{ + S: BoolPtr(true), + }, + out: []interface{}{ + &struct{ S Configurable[bool] }{ + S: Configurable[bool]{ + conditions: []ConfigurableCondition{{ + FunctionName: "soong_config_variable", + Args: []string{ + "my_namespace", + "foo", + }, + }}, + cases: []ConfigurableCase[bool]{{ + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "a", + }}, + value: BoolPtr(true), + }, { + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeDefault, + }}, + value: BoolPtr(false), + }}, + appendWrapper: &appendWrapper[bool]{ + append: Configurable[bool]{ + cases: []ConfigurableCase[bool]{{ + value: BoolPtr(true), + }}, + appendWrapper: &appendWrapper[bool]{}, + }, + }, + }, + }, + }, + }, + { + name: "Append bool to Configurable[bool]", + order: Append, + dst: []interface{}{ + &struct{ S Configurable[bool] }{ + S: Configurable[bool]{ + conditions: []ConfigurableCondition{{ + FunctionName: "soong_config_variable", + Args: []string{ + "my_namespace", + "foo", + }, + }}, + cases: []ConfigurableCase[bool]{{ + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "a", + }}, + value: BoolPtr(true), + }, { + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeDefault, + }}, + value: BoolPtr(false), + }}, + appendWrapper: &appendWrapper[bool]{}, + }, + }, + }, + src: &struct{ S bool }{ + S: true, + }, + out: []interface{}{ + &struct{ S Configurable[bool] }{ + S: Configurable[bool]{ + conditions: []ConfigurableCondition{{ + FunctionName: "soong_config_variable", + Args: []string{ + "my_namespace", + "foo", + }, + }}, + cases: []ConfigurableCase[bool]{{ + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "a", + }}, + value: BoolPtr(true), + }, { + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeDefault, + }}, + value: BoolPtr(false), + }}, + appendWrapper: &appendWrapper[bool]{ + append: Configurable[bool]{ + cases: []ConfigurableCase[bool]{{ + value: BoolPtr(true), + }}, + appendWrapper: &appendWrapper[bool]{}, + }, + }, + }, + }, + }, + }, } } |