Allow extractCommonProperties to return an error
Refactoring in preparation for follow up changes.
Also:
* Adds a new AssertErrorMessageEquals() helper method.
* Improved error reporting in the accessor and added name to
extractorProperty to ensure meaningful errors are reported.
* Added String() string method to propertiesContainer.
* Reports errors using the field name as the errors are not really
fixable by developers and it is more meaningful to the build team.
Bug: 155628860
Test: m nothing
Change-Id: I5c5b8436bcbc39e4e7cd35df2577b2dac53e702a
diff --git a/sdk/sdk_test.go b/sdk/sdk_test.go
index 7ae3a03..095f836 100644
--- a/sdk/sdk_test.go
+++ b/sdk/sdk_test.go
@@ -231,6 +231,7 @@
}
type testPropertiesStruct struct {
+ name string
private string
Public_Kept string `sdk:"keep"`
S_Common string
@@ -246,10 +247,17 @@
return p
}
+func (p *testPropertiesStruct) String() string {
+ return p.name
+}
+
+var _ propertiesContainer = (*testPropertiesStruct)(nil)
+
func TestCommonValueOptimization(t *testing.T) {
- common := &testPropertiesStruct{}
+ common := &testPropertiesStruct{name: "common"}
structs := []propertiesContainer{
&testPropertiesStruct{
+ name: "struct-0",
private: "common",
Public_Kept: "common",
S_Common: "common",
@@ -264,6 +272,7 @@
},
},
&testPropertiesStruct{
+ name: "struct-1",
private: "common",
Public_Kept: "common",
S_Common: "common",
@@ -285,6 +294,7 @@
h := TestHelper{t}
h.AssertDeepEquals("common properties not correct",
&testPropertiesStruct{
+ name: "common",
private: "",
Public_Kept: "",
S_Common: "common",
@@ -302,6 +312,7 @@
h.AssertDeepEquals("updated properties[0] not correct",
&testPropertiesStruct{
+ name: "struct-0",
private: "common",
Public_Kept: "common",
S_Common: "",
@@ -319,6 +330,7 @@
h.AssertDeepEquals("updated properties[1] not correct",
&testPropertiesStruct{
+ name: "struct-1",
private: "common",
Public_Kept: "common",
S_Common: "",
diff --git a/sdk/testing.go b/sdk/testing.go
index 9e27201..14a397c 100644
--- a/sdk/testing.go
+++ b/sdk/testing.go
@@ -173,6 +173,15 @@
}
}
+func (h *TestHelper) AssertErrorMessageEquals(message string, expected string, actual error) {
+ h.t.Helper()
+ if actual == nil {
+ h.t.Errorf("Expected error but was nil")
+ } else if actual.Error() != expected {
+ h.t.Errorf("%s: expected %s, actual %s", message, expected, actual.Error())
+ }
+}
+
func (h *TestHelper) AssertTrimmedStringEquals(message string, expected string, actual string) {
h.t.Helper()
h.AssertStringEquals(message, strings.TrimSpace(expected), strings.TrimSpace(actual))
diff --git a/sdk/update.go b/sdk/update.go
index ae74b9d..03a5c03 100644
--- a/sdk/update.go
+++ b/sdk/update.go
@@ -306,13 +306,13 @@
for _, sdkVariant := range sdkVariants {
properties := sdkVariant.dynamicMemberTypeListProperties
osTypeToMemberProperties[sdkVariant.Target().Os] = sdkVariant
- dynamicMemberPropertiesContainers = append(dynamicMemberPropertiesContainers, &dynamicMemberPropertiesContainer{properties})
+ dynamicMemberPropertiesContainers = append(dynamicMemberPropertiesContainers, &dynamicMemberPropertiesContainer{sdkVariant, properties})
}
// Extract the common lists of members into a separate struct.
commonDynamicMemberProperties := s.dynamicSdkMemberTypes.createMemberListProperties()
extractor := newCommonValueExtractor(commonDynamicMemberProperties)
- extractor.extractCommonProperties(commonDynamicMemberProperties, dynamicMemberPropertiesContainers)
+ extractCommonProperties(ctx, extractor, commonDynamicMemberProperties, dynamicMemberPropertiesContainers)
// Add properties common to all os types.
s.addMemberPropertiesToPropertySet(builder, snapshotModule, commonDynamicMemberProperties)
@@ -389,6 +389,13 @@
return outputZipFile
}
+func extractCommonProperties(ctx android.ModuleContext, extractor *commonValueExtractor, commonProperties interface{}, inputPropertiesSlice interface{}) {
+ err := extractor.extractCommonProperties(commonProperties, inputPropertiesSlice)
+ if err != nil {
+ ctx.ModuleErrorf("error extracting common properties: %s", err)
+ }
+}
+
func (s *sdk) addMemberPropertiesToPropertySet(builder *snapshotBuilder, propertySet android.BpPropertySet, dynamicMemberTypeListProperties interface{}) {
for _, memberListProperty := range s.memberListProperties() {
names := memberListProperty.getter(dynamicMemberTypeListProperties)
@@ -829,6 +836,8 @@
archInfos []*archTypeSpecificInfo
}
+var _ propertiesContainer = (*osTypeSpecificInfo)(nil)
+
type variantPropertiesFactoryFunc func() android.SdkMemberProperties
// Create a new osTypeSpecificInfo for the specified os type and its properties
@@ -886,7 +895,7 @@
// Optimize the properties by extracting common properties from arch type specific
// properties into os type specific properties.
-func (osInfo *osTypeSpecificInfo) optimizeProperties(commonValueExtractor *commonValueExtractor) {
+func (osInfo *osTypeSpecificInfo) optimizeProperties(ctx *memberContext, commonValueExtractor *commonValueExtractor) {
// Nothing to do if there is only a single common architecture.
if len(osInfo.archInfos) == 0 {
return
@@ -897,10 +906,10 @@
multilib = multilib.addArchType(archInfo.archType)
// Optimize the arch properties first.
- archInfo.optimizeProperties(commonValueExtractor)
+ archInfo.optimizeProperties(ctx, commonValueExtractor)
}
- commonValueExtractor.extractCommonProperties(osInfo.Properties, osInfo.archInfos)
+ extractCommonProperties(ctx.sdkMemberContext, commonValueExtractor, osInfo.Properties, osInfo.archInfos)
// Choose setting for compile_multilib that is appropriate for the arch variants supplied.
osInfo.Properties.Base().Compile_multilib = multilib.String()
@@ -973,6 +982,10 @@
}
}
+func (osInfo *osTypeSpecificInfo) String() string {
+ return fmt.Sprintf("OsType{%s}", osInfo.osType)
+}
+
type archTypeSpecificInfo struct {
baseInfo
@@ -981,6 +994,8 @@
linkInfos []*linkTypeSpecificInfo
}
+var _ propertiesContainer = (*archTypeSpecificInfo)(nil)
+
// Create a new archTypeSpecificInfo for the specified arch type and its properties
// structures populated with information from the variants.
func newArchSpecificInfo(ctx android.SdkMemberContext, archType android.ArchType, variantPropertiesFactory variantPropertiesFactoryFunc, archVariants []android.Module) *archTypeSpecificInfo {
@@ -1038,12 +1053,12 @@
// Optimize the properties by extracting common properties from link type specific
// properties into arch type specific properties.
-func (archInfo *archTypeSpecificInfo) optimizeProperties(commonValueExtractor *commonValueExtractor) {
+func (archInfo *archTypeSpecificInfo) optimizeProperties(ctx *memberContext, commonValueExtractor *commonValueExtractor) {
if len(archInfo.linkInfos) == 0 {
return
}
- commonValueExtractor.extractCommonProperties(archInfo.Properties, archInfo.linkInfos)
+ extractCommonProperties(ctx.sdkMemberContext, commonValueExtractor, archInfo.Properties, archInfo.linkInfos)
}
// Add the properties for an arch type to a property set.
@@ -1058,12 +1073,18 @@
}
}
+func (archInfo *archTypeSpecificInfo) String() string {
+ return fmt.Sprintf("ArchType{%s}", archInfo.archType)
+}
+
type linkTypeSpecificInfo struct {
baseInfo
linkType string
}
+var _ propertiesContainer = (*linkTypeSpecificInfo)(nil)
+
// Create a new linkTypeSpecificInfo for the specified link type and its properties
// structures populated with information from the variant.
func newLinkSpecificInfo(ctx android.SdkMemberContext, linkType string, variantPropertiesFactory variantPropertiesFactoryFunc, linkVariant android.Module) *linkTypeSpecificInfo {
@@ -1079,6 +1100,10 @@
return linkInfo
}
+func (l *linkTypeSpecificInfo) String() string {
+ return fmt.Sprintf("LinkType{%s}", l.linkType)
+}
+
type memberContext struct {
sdkMemberContext android.ModuleContext
builder *snapshotBuilder
@@ -1143,11 +1168,11 @@
osSpecificPropertiesContainers = append(osSpecificPropertiesContainers, osInfo)
// Optimize the properties across all the variants for a specific os type.
- osInfo.optimizeProperties(commonValueExtractor)
+ osInfo.optimizeProperties(ctx, commonValueExtractor)
}
// Extract properties which are common across all architectures and os types.
- commonValueExtractor.extractCommonProperties(commonProperties, osSpecificPropertiesContainers)
+ extractCommonProperties(ctx.sdkMemberContext, commonValueExtractor, commonProperties, osSpecificPropertiesContainers)
// Add the common properties to the module.
commonProperties.AddToPropertySet(ctx, bpModule)
@@ -1192,6 +1217,9 @@
// A property that can be optimized by the commonValueExtractor.
type extractorProperty struct {
+ // The name of the field for this property.
+ name string
+
// Retrieves the value on which common value optimization will be performed.
getter fieldAccessorFunc
@@ -1199,6 +1227,10 @@
emptyValue reflect.Value
}
+func (p extractorProperty) String() string {
+ return p.name
+}
+
// Supports extracting common values from a number of instances of a properties
// structure into a separate common set of properties.
type commonValueExtractor struct {
@@ -1240,6 +1272,9 @@
// Save a copy of the field index for use in the function.
fieldIndex := f
+
+ name := field.Name
+
fieldGetter := func(value reflect.Value) reflect.Value {
if containingStructAccessor != nil {
// This is an embedded structure so first access the field for the embedded
@@ -1250,6 +1285,12 @@
// Skip through interface and pointer values to find the structure.
value = getStructValue(value)
+ defer func() {
+ if r := recover(); r != nil {
+ panic(fmt.Errorf("%s for fieldIndex %d of field %s of value %#v", r, fieldIndex, name, value.Interface()))
+ }
+ }()
+
// Return the field.
return value.Field(fieldIndex)
}
@@ -1259,6 +1300,7 @@
e.gatherFields(field.Type, fieldGetter)
} else {
property := extractorProperty{
+ name,
fieldGetter,
reflect.Zero(field.Type),
}
@@ -1288,12 +1330,15 @@
// Allows additional information to be associated with the properties, e.g. for
// filtering.
type propertiesContainer interface {
+ fmt.Stringer
+
// Get the properties that need optimizing.
optimizableProperties() interface{}
}
// A wrapper for dynamic member properties to allow them to be optimized.
type dynamicMemberPropertiesContainer struct {
+ sdkVariant *sdk
dynamicMemberProperties interface{}
}
@@ -1301,6 +1346,10 @@
return c.dynamicMemberProperties
}
+func (c dynamicMemberPropertiesContainer) String() string {
+ return c.sdkVariant.String()
+}
+
// Extract common properties from a slice of property structures of the same type.
//
// All the property structures must be of the same type.
@@ -1311,7 +1360,7 @@
// have the same value (using DeepEquals) across all the input properties. If it does not then no
// change is made. Otherwise, the common value is stored in the field in the commonProperties
// and the field in each of the input properties structure is set to its default value.
-func (e *commonValueExtractor) extractCommonProperties(commonProperties interface{}, inputPropertiesSlice interface{}) {
+func (e *commonValueExtractor) extractCommonProperties(commonProperties interface{}, inputPropertiesSlice interface{}) error {
commonPropertiesValue := reflect.ValueOf(commonProperties)
commonStructValue := commonPropertiesValue.Elem()
@@ -1356,4 +1405,6 @@
}
}
}
+
+ return nil
}