diff --git a/android/sdk.go b/android/sdk.go index 873e08942..e823106e8 100644 --- a/android/sdk.go +++ b/android/sdk.go @@ -342,9 +342,24 @@ 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. A field annotated with a tag of `sdk:"keep"` will be treated as if it + // 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. // + // A field annotated with a tag of `android:"arch_variant"` will be allowed to have + // values that differ by arch, fields not tagged as such must have common values across + // all variants. + // + // * Additional field tags can be specified on a field that will ignore certain values + // for the purpose of common value optimization. A value that is ignored must have the + // default value for the property type. This is to ensure that significant value are not + // ignored by accident. The purpose of this is to allow the snapshot generation to reflect + // the behavior of the runtime. e.g. if a property is ignored on the host then a property + // that is common for android can be treated as if it was common for android and host as + // the setting for host is ignored anyway. + // * `sdk:"ignored-on-host" - this indicates the property is ignored on the host variant. + // // * The sdk module type populates the BpModule structure, creating the arch specific // structure and calls AddToPropertySet(...) on the properties struct to add the member // specific properties in the correct place in the structure. diff --git a/cc/library_sdk_member.go b/cc/library_sdk_member.go index 2c8e31158..a7a1de251 100644 --- a/cc/library_sdk_member.go +++ b/cc/library_sdk_member.go @@ -307,7 +307,7 @@ type nativeLibInfoProperties struct { // The list of possibly common exported include dirs. // // This field is exported as its contents may not be arch specific. - ExportedIncludeDirs android.Paths + ExportedIncludeDirs android.Paths `android:"arch_variant"` // The list of arch specific exported generated include dirs. // @@ -322,27 +322,31 @@ type nativeLibInfoProperties struct { // The list of possibly common exported system include dirs. // // This field is exported as its contents may not be arch specific. - ExportedSystemIncludeDirs android.Paths + ExportedSystemIncludeDirs android.Paths `android:"arch_variant"` // The list of possibly common exported flags. // // This field is exported as its contents may not be arch specific. - ExportedFlags []string + ExportedFlags []string `android:"arch_variant"` // The set of shared libraries // // This field is exported as its contents may not be arch specific. - SharedLibs []string + SharedLibs []string `android:"arch_variant"` // The set of system shared libraries. Note nil and [] are semantically // distinct - see BaseLinkerProperties.System_shared_libs. // // This field is exported as its contents may not be arch specific. - SystemSharedLibs []string + SystemSharedLibs []string `android:"arch_variant"` // The specific stubs version for the lib variant, or empty string if stubs // are not in use. - StubsVersion string + // + // Marked 'ignored-on-host' as the StubsVersion() from which this is initialized is + // not set on host and the stubs.versions property which this is written to is does + // not vary by arch so cannot be android specific. + StubsVersion string `sdk:"ignored-on-host"` // outputFile is not exported as it is always arch specific. outputFile android.Path diff --git a/java/java.go b/java/java.go index 472d3dac4..9d75c74c7 100644 --- a/java/java.go +++ b/java/java.go @@ -1906,7 +1906,7 @@ func (mt *librarySdkMemberType) CreateVariantPropertiesStruct() android.SdkMembe type librarySdkMemberProperties struct { android.SdkMemberPropertiesBase - JarToExport android.Path + JarToExport android.Path `android:"arch_variant"` AidlIncludeDirs android.Paths } diff --git a/sdk/cc_sdk_test.go b/sdk/cc_sdk_test.go index 733f7ac22..b77447aea 100644 --- a/sdk/cc_sdk_test.go +++ b/sdk/cc_sdk_test.go @@ -1805,3 +1805,86 @@ sdk_snapshot { } `)) } + +func TestDeviceAndHostSnapshotWithStubsLibrary(t *testing.T) { + result := testSdkWithCc(t, ` + sdk { + name: "mysdk", + host_supported: true, + native_shared_libs: ["stubslib"], + } + + cc_library { + name: "internaldep", + host_supported: true, + } + + cc_library { + name: "stubslib", + host_supported: true, + shared_libs: ["internaldep"], + stubs: { + symbol_file: "some/where/stubslib.map.txt", + versions: ["1", "2", "3"], + }, + } + `) + + result.CheckSnapshot("mysdk", "", + checkAndroidBpContents(` +// This is auto-generated. DO NOT EDIT. + +cc_prebuilt_library_shared { + name: "mysdk_stubslib@current", + sdk_member_name: "stubslib", + host_supported: true, + installable: false, + stubs: { + versions: ["3"], + }, + target: { + android_arm64: { + srcs: ["android/arm64/lib/stubslib.so"], + }, + android_arm: { + srcs: ["android/arm/lib/stubslib.so"], + }, + linux_glibc_x86_64: { + srcs: ["linux_glibc/x86_64/lib/stubslib.so"], + }, + linux_glibc_x86: { + srcs: ["linux_glibc/x86/lib/stubslib.so"], + }, + }, +} + +cc_prebuilt_library_shared { + name: "stubslib", + prefer: false, + host_supported: true, + stubs: { + versions: ["3"], + }, + target: { + android_arm64: { + srcs: ["android/arm64/lib/stubslib.so"], + }, + android_arm: { + srcs: ["android/arm/lib/stubslib.so"], + }, + linux_glibc_x86_64: { + srcs: ["linux_glibc/x86_64/lib/stubslib.so"], + }, + linux_glibc_x86: { + srcs: ["linux_glibc/x86/lib/stubslib.so"], + }, + }, +} + +sdk_snapshot { + name: "mysdk@current", + host_supported: true, + native_shared_libs: ["mysdk_stubslib@current"], +} +`)) +} diff --git a/sdk/sdk_test.go b/sdk/sdk_test.go index 095f83607..ae1a4923a 100644 --- a/sdk/sdk_test.go +++ b/sdk/sdk_test.go @@ -226,8 +226,8 @@ func TestSDkInstall(t *testing.T) { } type EmbeddedPropertiesStruct struct { - S_Embedded_Common string - S_Embedded_Different string + S_Embedded_Common string `android:"arch_variant"` + S_Embedded_Different string `android:"arch_variant"` } type testPropertiesStruct struct { @@ -235,11 +235,11 @@ type testPropertiesStruct struct { private string Public_Kept string `sdk:"keep"` S_Common string - S_Different string + S_Different string `android:"arch_variant"` A_Common []string - A_Different []string + A_Different []string `android:"arch_variant"` F_Common *bool - F_Different *bool + F_Different *bool `android:"arch_variant"` EmbeddedPropertiesStruct } @@ -289,9 +289,12 @@ func TestCommonValueOptimization(t *testing.T) { } extractor := newCommonValueExtractor(common) - extractor.extractCommonProperties(common, structs) h := TestHelper{t} + + err := extractor.extractCommonProperties(common, structs) + h.AssertDeepEquals("unexpected error", nil, err) + h.AssertDeepEquals("common properties not correct", &testPropertiesStruct{ name: "common", @@ -346,3 +349,26 @@ func TestCommonValueOptimization(t *testing.T) { }, structs[1]) } + +func TestCommonValueOptimization_InvalidArchSpecificVariants(t *testing.T) { + common := &testPropertiesStruct{name: "common"} + structs := []propertiesContainer{ + &testPropertiesStruct{ + name: "struct-0", + S_Common: "should-be-but-is-not-common0", + }, + &testPropertiesStruct{ + name: "struct-1", + S_Common: "should-be-but-is-not-common1", + }, + } + + extractor := newCommonValueExtractor(common) + + h := TestHelper{t} + + err := extractor.extractCommonProperties(common, structs) + h.AssertErrorMessageEquals("unexpected error", `field "S_Common" is not tagged as "arch_variant" but has arch specific properties: + "struct-0" has value "should-be-but-is-not-common0" + "struct-1" has value "should-be-but-is-not-common1"`, err) +} diff --git a/sdk/update.go b/sdk/update.go index 03a5c0379..d43a42d6e 100644 --- a/sdk/update.go +++ b/sdk/update.go @@ -982,6 +982,13 @@ func (osInfo *osTypeSpecificInfo) addToPropertySet(ctx *memberContext, bpModule } } +func (osInfo *osTypeSpecificInfo) isHostVariant() bool { + osClass := osInfo.osType.Class + return osClass == android.Host || osClass == android.HostCross +} + +var _ isHostVariant = (*osTypeSpecificInfo)(nil) + func (osInfo *osTypeSpecificInfo) String() string { return fmt.Sprintf("OsType{%s}", osInfo.osType) } @@ -1215,16 +1222,34 @@ func (s *sdk) getPossibleOsTypes() []android.OsType { // struct (or one of its embedded structs). type fieldAccessorFunc func(structValue reflect.Value) reflect.Value +// Checks the metadata to determine whether the property should be ignored for the +// purposes of common value extraction or not. +type extractorMetadataPredicate func(metadata propertiesContainer) bool + +// Indicates whether optimizable properties are provided by a host variant or +// not. +type isHostVariant interface { + isHostVariant() bool +} + // A property that can be optimized by the commonValueExtractor. type extractorProperty struct { // The name of the field for this property. name string + // Filter that can use metadata associated with the properties being optimized + // to determine whether the field should be ignored during common value + // optimization. + filter extractorMetadataPredicate + // Retrieves the value on which common value optimization will be performed. getter fieldAccessorFunc // The empty value for the field. emptyValue reflect.Value + + // True if the property can support arch variants false otherwise. + archVariant bool } func (p extractorProperty) String() string { @@ -1270,6 +1295,20 @@ func (e *commonValueExtractor) gatherFields(structType reflect.Type, containingS continue } + var filter extractorMetadataPredicate + + // Add a filter + if proptools.HasTag(field, "sdk", "ignored-on-host") { + filter = func(metadata propertiesContainer) bool { + if m, ok := metadata.(isHostVariant); ok { + if m.isHostVariant() { + return false + } + } + return true + } + } + // Save a copy of the field index for use in the function. fieldIndex := f @@ -1301,8 +1340,10 @@ func (e *commonValueExtractor) gatherFields(structType reflect.Type, containingS } else { property := extractorProperty{ name, + filter, fieldGetter, reflect.Zero(field.Type), + proptools.HasTag(field, "android", "arch_variant"), } e.properties = append(e.properties, property) } @@ -1368,17 +1409,38 @@ func (e *commonValueExtractor) extractCommonProperties(commonProperties interfac for _, property := range e.properties { fieldGetter := property.getter + filter := property.filter + if filter == nil { + filter = func(metadata propertiesContainer) bool { + return true + } + } // Check to see if all the structures have the same value for the field. The commonValue - // is nil on entry to the loop and if it is nil on exit then there is no common value, - // otherwise it points to the common value. + // is nil on entry to the loop and if it is nil on exit then there is no common value or + // all the values have been filtered out, otherwise it points to the common value. var commonValue *reflect.Value + // Assume that all the values will be the same. + // + // While similar to this is not quite the same as commonValue == nil. If all the values + // have been filtered out then this will be false but commonValue == nil will be true. + valuesDiffer := false + for i := 0; i < sliceValue.Len(); i++ { container := sliceValue.Index(i).Interface().(propertiesContainer) itemValue := reflect.ValueOf(container.optimizableProperties()) fieldValue := fieldGetter(itemValue) + if !filter(container) { + expectedValue := property.emptyValue.Interface() + actualValue := fieldValue.Interface() + if !reflect.DeepEqual(expectedValue, actualValue) { + return fmt.Errorf("field %q is supposed to be ignored for %q but is set to %#v instead of %#v", property, container, actualValue, expectedValue) + } + continue + } + if commonValue == nil { // Use the first value as the commonProperties value. commonValue = &fieldValue @@ -1387,12 +1449,13 @@ func (e *commonValueExtractor) extractCommonProperties(commonProperties interfac // no value in common so break out. if !reflect.DeepEqual(fieldValue.Interface(), commonValue.Interface()) { commonValue = nil + valuesDiffer = true break } } } - // If the fields all have a common value then store it in the common struct field + // If the fields all have common value then store it in the common struct field // and set the input struct's field to the empty value. if commonValue != nil { emptyValue := property.emptyValue @@ -1404,6 +1467,21 @@ func (e *commonValueExtractor) extractCommonProperties(commonProperties interfac fieldValue.Set(emptyValue) } } + + if valuesDiffer && !property.archVariant { + // The values differ but the property does not support arch variants so it + // is an error. + var details strings.Builder + for i := 0; i < sliceValue.Len(); i++ { + container := sliceValue.Index(i).Interface().(propertiesContainer) + itemValue := reflect.ValueOf(container.optimizableProperties()) + fieldValue := fieldGetter(itemValue) + + _, _ = fmt.Fprintf(&details, "\n %q has value %q", container.String(), fieldValue.Interface()) + } + + return fmt.Errorf("field %q is not tagged as \"arch_variant\" but has arch specific properties:%s", property.String(), details.String()) + } } return nil