Merge changes from topic "revert-1302576-NGVCIVIIJO"

* changes:
  Revert "Detect invalid arch specific properties in snapshot"
  Revert "Adds support for 'ignored-on-host'"
  Revert "Fix snapshot of a host/device cc_library with stubs"
This commit is contained in:
Greg Kaiser 2020-05-08 19:18:40 +00:00 committed by Gerrit Code Review
commit a71fe150fa
6 changed files with 17 additions and 223 deletions

View File

@ -342,24 +342,9 @@ 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.

View File

@ -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 `android:"arch_variant"`
ExportedIncludeDirs android.Paths
// The list of arch specific exported generated include dirs.
//
@ -322,31 +322,27 @@ 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 `android:"arch_variant"`
ExportedSystemIncludeDirs android.Paths
// The list of possibly common exported flags.
//
// This field is exported as its contents may not be arch specific.
ExportedFlags []string `android:"arch_variant"`
ExportedFlags []string
// The set of shared libraries
//
// This field is exported as its contents may not be arch specific.
SharedLibs []string `android:"arch_variant"`
SharedLibs []string
// 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 `android:"arch_variant"`
SystemSharedLibs []string
// The specific stubs version for the lib variant, or empty string if stubs
// are not in use.
//
// 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"`
StubsVersion string
// outputFile is not exported as it is always arch specific.
outputFile android.Path

View File

@ -1906,7 +1906,7 @@ func (mt *librarySdkMemberType) CreateVariantPropertiesStruct() android.SdkMembe
type librarySdkMemberProperties struct {
android.SdkMemberPropertiesBase
JarToExport android.Path `android:"arch_variant"`
JarToExport android.Path
AidlIncludeDirs android.Paths
}

View File

@ -1805,86 +1805,3 @@ 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"],
}
`))
}

View File

@ -226,8 +226,8 @@ func TestSDkInstall(t *testing.T) {
}
type EmbeddedPropertiesStruct struct {
S_Embedded_Common string `android:"arch_variant"`
S_Embedded_Different string `android:"arch_variant"`
S_Embedded_Common string
S_Embedded_Different string
}
type testPropertiesStruct struct {
@ -235,11 +235,11 @@ type testPropertiesStruct struct {
private string
Public_Kept string `sdk:"keep"`
S_Common string
S_Different string `android:"arch_variant"`
S_Different string
A_Common []string
A_Different []string `android:"arch_variant"`
A_Different []string
F_Common *bool
F_Different *bool `android:"arch_variant"`
F_Different *bool
EmbeddedPropertiesStruct
}
@ -289,12 +289,9 @@ 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",
@ -349,26 +346,3 @@ 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)
}

View File

@ -982,13 +982,6 @@ 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)
}
@ -1222,34 +1215,16 @@ 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 {
@ -1295,20 +1270,6 @@ 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
@ -1340,10 +1301,8 @@ 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)
}
@ -1409,38 +1368,17 @@ 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 or
// all the values have been filtered out, 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,
// 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
@ -1449,13 +1387,12 @@ 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 common value then store it in the common struct field
// If the fields all have a 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
@ -1467,21 +1404,6 @@ 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