From 106a3a4bec44f8d1120f72dc52f6d2c797f3a80e Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Thu, 27 Jan 2022 16:39:06 +0000 Subject: [PATCH 1/2] Allow pruning of unsupported fields in structs in maps Adds support for traversing into a field that is of type: map[...]*struct{...} This is needed to allow java_sdk_library to mark scope specific properties, e.g. public.annotations as being target build release specific. It was necessary to change the Scope field from: Scope map[*apiScope]scopeProperties to: Scope map[*apiScope]*scopeProperties That is because there is no way in go to change the field of a struct value of a map. i.e. you cannot do the following, not even using reflection: Scope[apiScopePublic].AnnotationsZip = nil Bug: 204763318 Test: m nothing Change-Id: Id103f70f55d4202971321ef4925cbec4b55f8136 --- java/sdk_library.go | 6 ++--- sdk/build_release.go | 52 +++++++++++++++++++++++++++++++++++++++ sdk/build_release_test.go | 22 +++++++++++++++++ 3 files changed, 77 insertions(+), 3 deletions(-) diff --git a/java/sdk_library.go b/java/sdk_library.go index 57ab2686f..98f29990d 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -2755,7 +2755,7 @@ type sdkLibrarySdkMemberProperties struct { android.SdkMemberPropertiesBase // Scope to per scope properties. - Scopes map[*apiScope]scopeProperties + Scopes map[*apiScope]*scopeProperties // The Java stubs source files. Stub_srcs []string @@ -2815,7 +2815,7 @@ type scopeProperties struct { func (s *sdkLibrarySdkMemberProperties) PopulateFromVariant(ctx android.SdkMemberContext, variant android.Module) { sdk := variant.(*SdkLibrary) - s.Scopes = make(map[*apiScope]scopeProperties) + s.Scopes = make(map[*apiScope]*scopeProperties) for _, apiScope := range allApiScopes { paths := sdk.findScopePaths(apiScope) if paths == nil { @@ -2838,7 +2838,7 @@ func (s *sdkLibrarySdkMemberProperties) PopulateFromVariant(ctx android.SdkMembe if paths.annotationsZip.Valid() { properties.AnnotationsZip = paths.annotationsZip.Path() } - s.Scopes[apiScope] = properties + s.Scopes[apiScope] = &properties } } diff --git a/sdk/build_release.go b/sdk/build_release.go index 212582ac6..2bcdc6f57 100644 --- a/sdk/build_release.go +++ b/sdk/build_release.go @@ -269,6 +269,51 @@ func (p *propertyPruner) gatherFields(structType reflect.Type, containingStructA subNamePrefix = name + "." } p.gatherFields(fieldType, fieldGetter, subNamePrefix, selector) + + case reflect.Map: + // Get the type of the values stored in the map. + valueType := fieldType.Elem() + // Skip over * types. + if valueType.Kind() == reflect.Ptr { + valueType = valueType.Elem() + } + if valueType.Kind() == reflect.Struct { + // If this is not referenced by a pointer then it is an error as it is impossible to + // modify a struct that is stored directly as a value in a map. + if fieldType.Elem().Kind() != reflect.Ptr { + panic(fmt.Errorf("Cannot prune struct %s stored by value in map %s, map values must"+ + " be pointers to structs", + fieldType.Elem(), name)) + } + + // Create a new pruner for the values of the map. + valuePruner := newPropertyPrunerForStructType(valueType, selector) + + // Create a new fieldPruner that will iterate over all the items in the map and call the + // pruner on them. + fieldPruner := func(container reflect.Value) { + mapValue := fieldGetter(container) + + for _, keyValue := range mapValue.MapKeys() { + itemValue := mapValue.MapIndex(keyValue) + + defer func() { + if r := recover(); r != nil { + panic(fmt.Errorf("%s\n\tfor key %q", r, keyValue)) + } + }() + + valuePruner.pruneProperties(itemValue.Interface()) + } + } + + // Add the map field pruner to the list of property pruners. + property := prunerProperty{ + name + "[*]", + fieldPruner, + } + p.properties = append(p.properties, property) + } } } } @@ -304,6 +349,13 @@ type fieldSelectorFunc func(name string, field reflect.StructField) bool // of properties. func newPropertyPruner(propertiesStruct interface{}, selector fieldSelectorFunc) *propertyPruner { structType := getStructValue(reflect.ValueOf(propertiesStruct)).Type() + return newPropertyPrunerForStructType(structType, selector) +} + +// newPropertyPruner creates a new property pruner for the supplied properties struct type. +// +// The returned pruner can be used on any properties structure of the supplied type. +func newPropertyPrunerForStructType(structType reflect.Type, selector fieldSelectorFunc) *propertyPruner { pruner := &propertyPruner{} pruner.gatherFields(structType, nil, "", selector) return pruner diff --git a/sdk/build_release_test.go b/sdk/build_release_test.go index 0ec10409f..6608be4f8 100644 --- a/sdk/build_release_test.go +++ b/sdk/build_release_test.go @@ -126,11 +126,17 @@ func TestPropertyPrunerByBuildRelease(t *testing.T) { F1_only string `supported_build_releases:"F1"` } + type mapped struct { + Default string + T_only string `supported_build_releases:"T"` + } + type testBuildReleasePruner struct { Default string S_and_T_only string `supported_build_releases:"S-T"` T_later string `supported_build_releases:"T+"` Nested nested + Mapped map[string]*mapped } inputFactory := func() testBuildReleasePruner { @@ -141,6 +147,16 @@ func TestPropertyPrunerByBuildRelease(t *testing.T) { Nested: nested{ F1_only: "F1_only", }, + Mapped: map[string]*mapped{ + "one": { + Default: "one-default", + T_only: "one-t-only", + }, + "two": { + Default: "two-default", + T_only: "two-t-only", + }, + }, } } @@ -169,6 +185,8 @@ func TestPropertyPrunerByBuildRelease(t *testing.T) { expected := inputFactory() expected.T_later = "" expected.Nested.F1_only = "" + expected.Mapped["one"].T_only = "" + expected.Mapped["two"].T_only = "" assertJsonEquals(t, expected, testStruct) }) @@ -189,6 +207,8 @@ func TestPropertyPrunerByBuildRelease(t *testing.T) { expected := inputFactory() expected.S_and_T_only = "" + expected.Mapped["one"].T_only = "" + expected.Mapped["two"].T_only = "" assertJsonEquals(t, expected, testStruct) }) @@ -200,6 +220,8 @@ func TestPropertyPrunerByBuildRelease(t *testing.T) { expected := inputFactory() expected.S_and_T_only = "" expected.Nested.F1_only = "" + expected.Mapped["one"].T_only = "" + expected.Mapped["two"].T_only = "" assertJsonEquals(t, expected, testStruct) }) } From a54016c3ba5db8efd69d098e0bb4cb8b318d2efa Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Thu, 27 Jan 2022 16:39:47 +0000 Subject: [PATCH 2/2] Only output annotations properties in snapshots for T+ Marks the `scopeProperties.AnnotationsZip` property as only being supported on T builds and above. Bug: 204763318 Test: m nothing Change-Id: Ie59396287c08da77a6a1f15d4be202429e011e17 --- java/sdk_library.go | 2 +- sdk/java_sdk_test.go | 52 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/java/sdk_library.go b/java/sdk_library.go index 98f29990d..6a2a7a845 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -2808,7 +2808,7 @@ type scopeProperties struct { StubsSrcJar android.Path CurrentApiFile android.Path RemovedApiFile android.Path - AnnotationsZip android.Path + AnnotationsZip android.Path `supported_build_releases:"T+"` SdkVersion string } diff --git a/sdk/java_sdk_test.go b/sdk/java_sdk_test.go index 0d9b4a063..f0d3b35d7 100644 --- a/sdk/java_sdk_test.go +++ b/sdk/java_sdk_test.go @@ -1319,6 +1319,58 @@ java_sdk_library_import { ) } +func TestSnapshotWithJavaSdkLibrary_AnnotationsZip_PreT(t *testing.T) { + result := android.GroupFixturePreparers( + prepareForSdkTestWithJavaSdkLibrary, + android.FixtureMergeEnv(map[string]string{ + "SOONG_SDK_SNAPSHOT_TARGET_BUILD_RELEASE": "S", + }), + ).RunTestWithBp(t, ` + sdk { + name: "mysdk", + java_sdk_libs: ["myjavalib"], + } + + java_sdk_library { + name: "myjavalib", + srcs: ["Test.java"], + sdk_version: "current", + shared_library: false, + annotations_enabled: true, + public: { + enabled: true, + }, + } + `) + + CheckSnapshot(t, result, "mysdk", "", + checkUnversionedAndroidBpContents(` +// This is auto-generated. DO NOT EDIT. + +java_sdk_library_import { + name: "myjavalib", + prefer: false, + visibility: ["//visibility:public"], + apex_available: ["//apex_available:platform"], + shared_library: false, + public: { + jars: ["sdk_library/public/myjavalib-stubs.jar"], + stub_srcs: ["sdk_library/public/myjavalib_stub_sources"], + current_api: "sdk_library/public/myjavalib.txt", + removed_api: "sdk_library/public/myjavalib-removed.txt", + sdk_version: "current", + }, +} + `), + checkAllCopyRules(` +.intermediates/myjavalib.stubs/android_common/javac/myjavalib.stubs.jar -> sdk_library/public/myjavalib-stubs.jar +.intermediates/myjavalib.stubs.source/android_common/metalava/myjavalib.stubs.source_api.txt -> sdk_library/public/myjavalib.txt +.intermediates/myjavalib.stubs.source/android_common/metalava/myjavalib.stubs.source_removed.txt -> sdk_library/public/myjavalib-removed.txt + `), + checkMergeZips(".intermediates/mysdk/common_os/tmp/sdk_library/public/myjavalib_stub_sources.zip"), + ) +} + func TestSnapshotWithJavaSdkLibrary_CompileDex(t *testing.T) { result := android.GroupFixturePreparers(prepareForSdkTestWithJavaSdkLibrary).RunTestWithBp(t, ` sdk {