diff --git a/android/sdk.go b/android/sdk.go index 969e21adb..ad67bbe6f 100644 --- a/android/sdk.go +++ b/android/sdk.go @@ -325,7 +325,8 @@ type SdkMemberType interface { // // * The variant property structs are analysed to find exported (capitalized) fields which // have common values. Those fields are cleared and the common value added to the common - // properties. + // properties. A field annotated with a tag of `sdk:"keep"` will be treated as if it + // was not capitalized, i.e. not optimized for common values. // // * The sdk module type populates the BpModule structure, creating the arch specific // structure and calls AddToPropertySet(...) on the properties struct to add the member @@ -452,13 +453,13 @@ func RegisterSdkMemberType(memberType SdkMemberType) { // are not affected by the optimization to extract common values. type SdkMemberPropertiesBase struct { // The setting to use for the compile_multilib property. - Compile_multilib string + Compile_multilib string `sdk:"keep"` // The number of unique os types supported by the member variants. - Os_count int + Os_count int `sdk:"keep"` // The os type for which these properties refer. - Os OsType + Os OsType `sdk:"keep"` } // The os prefix to use for any file paths in the sdk. diff --git a/sdk/sdk_test.go b/sdk/sdk_test.go index 96837e3dd..fde92307f 100644 --- a/sdk/sdk_test.go +++ b/sdk/sdk_test.go @@ -16,6 +16,8 @@ package sdk import ( "testing" + + "github.com/google/blueprint/proptools" ) // Needed in an _test.go file in this package to ensure tests run correctly, particularly in IDE. @@ -222,3 +224,106 @@ func TestSDkInstall(t *testing.T) { checkAllOtherCopyRules(`.intermediates/mysdk/common_os/mysdk-current.zip -> mysdk-current.zip`), ) } + +type EmbeddedPropertiesStruct struct { + S_Embedded_Common string + S_Embedded_Different string +} + +type testPropertiesStruct struct { + private string + Public_Kept string `sdk:"keep"` + S_Common string + S_Different string + A_Common []string + A_Different []string + F_Common *bool + F_Different *bool + EmbeddedPropertiesStruct +} + +func TestCommonValueOptimization(t *testing.T) { + common := &testPropertiesStruct{} + structs := []*testPropertiesStruct{ + &testPropertiesStruct{ + private: "common", + Public_Kept: "common", + S_Common: "common", + S_Different: "upper", + A_Common: []string{"first", "second"}, + A_Different: []string{"alpha", "beta"}, + F_Common: proptools.BoolPtr(false), + F_Different: proptools.BoolPtr(false), + EmbeddedPropertiesStruct: EmbeddedPropertiesStruct{ + S_Embedded_Common: "embedded_common", + S_Embedded_Different: "embedded_upper", + }, + }, + &testPropertiesStruct{ + private: "common", + Public_Kept: "common", + S_Common: "common", + S_Different: "lower", + A_Common: []string{"first", "second"}, + A_Different: []string{"alpha", "delta"}, + F_Common: proptools.BoolPtr(false), + F_Different: proptools.BoolPtr(true), + EmbeddedPropertiesStruct: EmbeddedPropertiesStruct{ + S_Embedded_Common: "embedded_common", + S_Embedded_Different: "embedded_lower", + }, + }, + } + + extractor := newCommonValueExtractor(common) + extractor.extractCommonProperties(common, structs) + + h := TestHelper{t} + h.AssertDeepEquals("common properties not correct", common, + &testPropertiesStruct{ + private: "", + Public_Kept: "", + S_Common: "common", + S_Different: "", + A_Common: []string{"first", "second"}, + A_Different: []string(nil), + F_Common: proptools.BoolPtr(false), + F_Different: nil, + EmbeddedPropertiesStruct: EmbeddedPropertiesStruct{ + S_Embedded_Common: "embedded_common", + S_Embedded_Different: "", + }, + }) + + h.AssertDeepEquals("updated properties[0] not correct", structs[0], + &testPropertiesStruct{ + private: "common", + Public_Kept: "common", + S_Common: "", + S_Different: "upper", + A_Common: nil, + A_Different: []string{"alpha", "beta"}, + F_Common: nil, + F_Different: proptools.BoolPtr(false), + EmbeddedPropertiesStruct: EmbeddedPropertiesStruct{ + S_Embedded_Common: "", + S_Embedded_Different: "embedded_upper", + }, + }) + + h.AssertDeepEquals("updated properties[1] not correct", structs[1], + &testPropertiesStruct{ + private: "common", + Public_Kept: "common", + S_Common: "", + S_Different: "lower", + A_Common: nil, + A_Different: []string{"alpha", "delta"}, + F_Common: nil, + F_Different: proptools.BoolPtr(true), + EmbeddedPropertiesStruct: EmbeddedPropertiesStruct{ + S_Embedded_Common: "", + S_Embedded_Different: "embedded_lower", + }, + }) +} diff --git a/sdk/testing.go b/sdk/testing.go index 464c3ca9a..00245cef4 100644 --- a/sdk/testing.go +++ b/sdk/testing.go @@ -19,6 +19,7 @@ import ( "io/ioutil" "os" "path/filepath" + "reflect" "strings" "testing" @@ -176,6 +177,13 @@ func (h *TestHelper) AssertTrimmedStringEquals(message string, expected string, h.AssertStringEquals(message, strings.TrimSpace(expected), strings.TrimSpace(actual)) } +func (h *TestHelper) AssertDeepEquals(message string, expected interface{}, actual interface{}) { + h.t.Helper() + if !reflect.DeepEqual(actual, expected) { + h.t.Errorf("%s: expected %#v, actual %#v", message, expected, actual) + } +} + // Encapsulates result of processing an SDK definition. Provides support for // checking the state of the build structures. type testSdkResult struct { diff --git a/sdk/update.go b/sdk/update.go index 1a631dd7b..54d4c7925 100644 --- a/sdk/update.go +++ b/sdk/update.go @@ -951,7 +951,8 @@ func (s *sdk) getPossibleOsTypes() []android.OsType { return osTypes } -// Given a struct value, access a field within that struct. +// Given a struct value, access a field within that struct (or one of its embedded +// structs). type fieldAccessorFunc func(structValue reflect.Value) reflect.Value // Supports extracting common values from a number of instances of a properties @@ -969,13 +970,18 @@ type commonValueExtractor struct { func newCommonValueExtractor(propertiesStruct interface{}) *commonValueExtractor { structType := getStructValue(reflect.ValueOf(propertiesStruct)).Type() extractor := &commonValueExtractor{} - extractor.gatherFields(structType) + extractor.gatherFields(structType, nil) return extractor } // Gather the fields from the supplied structure type from which common values will // be extracted. -func (e *commonValueExtractor) gatherFields(structType reflect.Type) { +// +// This is recursive function. If it encounters an embedded field (no field name) +// that is a struct then it will recurse into that struct passing in the accessor +// for the field. That will then be used in the accessors for the fields in the +// embedded struct. +func (e *commonValueExtractor) gatherFields(structType reflect.Type, containingStructAccessor fieldAccessorFunc) { for f := 0; f < structType.NumField(); f++ { field := structType.Field(f) if field.PkgPath != "" { @@ -983,14 +989,20 @@ func (e *commonValueExtractor) gatherFields(structType reflect.Type) { continue } - // Ignore embedded structures. - if field.Type.Kind() == reflect.Struct && field.Anonymous { + // Ignore fields whose value should be kept. + if proptools.HasTag(field, "sdk", "keep") { continue } // Save a copy of the field index for use in the function. fieldIndex := f fieldGetter := func(value reflect.Value) reflect.Value { + if containingStructAccessor != nil { + // This is an embedded structure so first access the field for the embedded + // structure. + value = containingStructAccessor(value) + } + // Skip through interface and pointer values to find the structure. value = getStructValue(value) @@ -998,7 +1010,12 @@ func (e *commonValueExtractor) gatherFields(structType reflect.Type) { return value.Field(fieldIndex) } - e.fieldGetters = append(e.fieldGetters, fieldGetter) + if field.Type.Kind() == reflect.Struct && field.Anonymous { + // Gather fields from the embedded structure. + e.gatherFields(field.Type, fieldGetter) + } else { + e.fieldGetters = append(e.fieldGetters, fieldGetter) + } } }