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

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.

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

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
 }
 

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"],
+}
+`))
+}

sdk/sdk_test.go

@@ -226,8 +226,8 @@ func TestSDkInstall(t *testing.T) {
 }
 
 type EmbeddedPropertiesStruct struct {
-	S_Embedded_Common    string
+	S_Embedded_Common    string `android:"arch_variant"`
-	S_Embedded_Different string
+	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)
+}

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,
+		// is nil on entry to the loop and if it is nil on exit then there is no common value or
-		// otherwise it points to the common value.
+		// 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