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
This commit is contained in:
Cole Faust 2024-04-17 10:24:51 -07:00
parent 36b6322979
commit 4560bb086e
3 changed files with 197 additions and 2 deletions

View file

@ -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:

View file

@ -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()) {

View file

@ -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]{},
},
},
},
},
},
},
}
}