Merge changes I167b47a1,I7ebd3330,Ifc8116e1
* changes:
Fix snapshot of a host/device cc_library with stubs
Adds support for 'ignored-on-host'
Detect invalid arch specific properties in snapshot
diff --git a/android/sdk.go b/android/sdk.go
index 873e089..e823106 100644
--- a/android/sdk.go
+++ b/android/sdk.go
@@ -342,9 +342,24 @@
//
// * 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 2c8e311..a7a1de2 100644
--- a/cc/library_sdk_member.go
+++ b/cc/library_sdk_member.go
@@ -307,7 +307,7 @@
// 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 @@
// 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 472d3da..9d75c74 100644
--- a/java/java.go
+++ b/java/java.go
@@ -1906,7 +1906,7 @@
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 733f7ac..b77447a 100644
--- a/sdk/cc_sdk_test.go
+++ b/sdk/cc_sdk_test.go
@@ -1805,3 +1805,86 @@
}
`))
}
+
+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 095f836..ae1a492 100644
--- a/sdk/sdk_test.go
+++ b/sdk/sdk_test.go
@@ -226,8 +226,8 @@
}
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 @@
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 @@
}
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 @@
},
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 03a5c03..d43a42d 100644
--- a/sdk/update.go
+++ b/sdk/update.go
@@ -982,6 +982,13 @@
}
}
+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 @@
// 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 @@
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 @@
} 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 @@
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 @@
// 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 @@
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