Merge "Retry: Detect invalid arch specific properties in snapshot" (385378bf) · Commits · e / os / android_build_soong

android/sdk.go

+7 −1

Original line number	Diff line number	Diff line
		@@ -342,9 +342,15 @@ 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.
		//
		// * 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

+5 −5

Original line number	Diff line number	Diff line
		@@ -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,23 +322,23 @@ 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.

java/java.go

+1 −1

Original line number	Diff line number	Diff line
		@@ -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/sdk_test.go

+28 −5

Original line number	Diff line number	Diff line
		@@ -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
		}

		@@ -346,3 +346,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

+29 −3

Original line number	Diff line number	Diff line
		@@ -1225,6 +1225,9 @@ type extractorProperty struct {

		// 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 {
		@@ -1303,6 +1306,7 @@ func (e *commonValueExtractor) gatherFields(structType reflect.Type, containingS
		name,
		fieldGetter,
		reflect.Zero(field.Type),
		proptools.HasTag(field, "android", "arch_variant"),
		}
		e.properties = append(e.properties, property)
		}
		@@ -1370,10 +1374,16 @@ func (e *commonValueExtractor) extractCommonProperties(commonProperties interfac
		fieldGetter := property.getter

		// 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())
		@@ -1387,12 +1397,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 +1415,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