From ee9b117038367bfd137846150f9eccf372cb2676 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Tue, 6 Apr 2021 17:40:32 +0900 Subject: [PATCH 1/2] Remove nativeApiLevelFromUserWithDefault ... in favor of proptools.StringDefault Bug: 1663140 Test: m Change-Id: I0b3062921b25179cd1bf53856973fb67fe5cfc05 --- cc/api_level.go | 8 -------- cc/ndk_library.go | 5 +++-- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/cc/api_level.go b/cc/api_level.go index c93d6eda3..fd145a9e2 100644 --- a/cc/api_level.go +++ b/cc/api_level.go @@ -53,14 +53,6 @@ func nativeApiLevelFromUser(ctx android.BaseModuleContext, return value, nil } -func nativeApiLevelFromUserWithDefault(ctx android.BaseModuleContext, - raw string, defaultValue string) (android.ApiLevel, error) { - if raw == "" { - raw = defaultValue - } - return nativeApiLevelFromUser(ctx, raw) -} - func nativeApiLevelOrPanic(ctx android.BaseModuleContext, raw string) android.ApiLevel { value, err := nativeApiLevelFromUser(ctx, raw) diff --git a/cc/ndk_library.go b/cc/ndk_library.go index 10de889d1..a32ef1fc0 100644 --- a/cc/ndk_library.go +++ b/cc/ndk_library.go @@ -20,6 +20,7 @@ import ( "sync" "github.com/google/blueprint" + "github.com/google/blueprint/proptools" "android/soong/android" ) @@ -147,8 +148,8 @@ func (this *stubDecorator) initializeProperties(ctx BaseModuleContext) bool { return false } - this.unversionedUntil, err = nativeApiLevelFromUserWithDefault(ctx, - String(this.properties.Unversioned_until), "minimum") + str := proptools.StringDefault(this.properties.Unversioned_until, "minimum") + this.unversionedUntil, err = nativeApiLevelFromUser(ctx, str) if err != nil { ctx.PropertyErrorf("unversioned_until", err.Error()) return false From 4eab21d5a29d21c86b8b6662150064b455d3cc92 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Thu, 15 Apr 2021 15:17:54 +0900 Subject: [PATCH 2/2] ApexInfo doesn't pass MinSdkVersion as string, but as ApiLevel ApexInfo is not part of the properties struct. It can handle structs having private fields. Bug: 1663140 Test: m Change-Id: Ib07d4410f0ce187c9de347da34b84b814b2eb537 --- android/apex.go | 14 +++----------- android/apex_test.go | 42 +++++++++++++++++++++--------------------- apex/apex.go | 2 +- cc/cc.go | 2 +- 4 files changed, 26 insertions(+), 34 deletions(-) diff --git a/android/apex.go b/android/apex.go index cfda2aa8f..25a07b866 100644 --- a/android/apex.go +++ b/android/apex.go @@ -43,10 +43,8 @@ type ApexInfo struct { // mergeApexVariations. ApexVariationName string - // Serialized ApiLevel that this module has to support at minimum. Should be accessed via - // MinSdkVersion() method. Cannot be stored in its struct form because this is cloned into - // properties structs, and ApiLevel has private members. - MinSdkVersionStr string + // ApiLevel that this module has to support at minimum. + MinSdkVersion ApiLevel // True if this module comes from an updatable apexBundle. Updatable bool @@ -82,19 +80,13 @@ var ApexInfoProvider = blueprint.NewMutatorProvider(ApexInfo{}, "apex") // have to be built twice, but only once. In that case, the two apex variations apex.a and apex.b // are configured to have the same alias variation named apex29. func (i ApexInfo) mergedName(ctx PathContext) string { - name := "apex" + strconv.Itoa(i.MinSdkVersion(ctx).FinalOrFutureInt()) + name := "apex" + strconv.Itoa(i.MinSdkVersion.FinalOrFutureInt()) for _, sdk := range i.RequiredSdks { name += "_" + sdk.Name + "_" + sdk.Version } return name } -// MinSdkVersion gives the api level that this module has to support at minimum. This is from the -// min_sdk_version property of the containing apexBundle. -func (i ApexInfo) MinSdkVersion(ctx PathContext) ApiLevel { - return ApiLevelOrPanic(ctx, i.MinSdkVersionStr) -} - // IsForPlatform tells whether this module is for the platform or not. If false is returned, it // means that this apex variant of the module is built for an APEX. func (i ApexInfo) IsForPlatform() bool { diff --git a/android/apex_test.go b/android/apex_test.go index b5323e8af..109b1c8b2 100644 --- a/android/apex_test.go +++ b/android/apex_test.go @@ -33,10 +33,10 @@ func Test_mergeApexVariations(t *testing.T) { { name: "single", in: []ApexInfo{ - {"foo", "current", false, nil, []string{"foo"}, nil, NotForPrebuiltApex}, + {"foo", FutureApiLevel, false, nil, []string{"foo"}, nil, NotForPrebuiltApex}, }, wantMerged: []ApexInfo{ - {"apex10000", "current", false, nil, []string{"foo"}, nil, NotForPrebuiltApex}, + {"apex10000", FutureApiLevel, false, nil, []string{"foo"}, nil, NotForPrebuiltApex}, }, wantAliases: [][2]string{ {"foo", "apex10000"}, @@ -45,11 +45,11 @@ func Test_mergeApexVariations(t *testing.T) { { name: "merge", in: []ApexInfo{ - {"foo", "current", false, SdkRefs{{"baz", "1"}}, []string{"foo"}, nil, NotForPrebuiltApex}, - {"bar", "current", false, SdkRefs{{"baz", "1"}}, []string{"bar"}, nil, NotForPrebuiltApex}, + {"foo", FutureApiLevel, false, SdkRefs{{"baz", "1"}}, []string{"foo"}, nil, NotForPrebuiltApex}, + {"bar", FutureApiLevel, false, SdkRefs{{"baz", "1"}}, []string{"bar"}, nil, NotForPrebuiltApex}, }, wantMerged: []ApexInfo{ - {"apex10000_baz_1", "current", false, SdkRefs{{"baz", "1"}}, []string{"bar", "foo"}, nil, false}}, + {"apex10000_baz_1", FutureApiLevel, false, SdkRefs{{"baz", "1"}}, []string{"bar", "foo"}, nil, false}}, wantAliases: [][2]string{ {"bar", "apex10000_baz_1"}, {"foo", "apex10000_baz_1"}, @@ -58,12 +58,12 @@ func Test_mergeApexVariations(t *testing.T) { { name: "don't merge version", in: []ApexInfo{ - {"foo", "current", false, nil, []string{"foo"}, nil, NotForPrebuiltApex}, - {"bar", "30", false, nil, []string{"bar"}, nil, NotForPrebuiltApex}, + {"foo", FutureApiLevel, false, nil, []string{"foo"}, nil, NotForPrebuiltApex}, + {"bar", uncheckedFinalApiLevel(30), false, nil, []string{"bar"}, nil, NotForPrebuiltApex}, }, wantMerged: []ApexInfo{ - {"apex30", "30", false, nil, []string{"bar"}, nil, NotForPrebuiltApex}, - {"apex10000", "current", false, nil, []string{"foo"}, nil, NotForPrebuiltApex}, + {"apex30", uncheckedFinalApiLevel(30), false, nil, []string{"bar"}, nil, NotForPrebuiltApex}, + {"apex10000", FutureApiLevel, false, nil, []string{"foo"}, nil, NotForPrebuiltApex}, }, wantAliases: [][2]string{ {"bar", "apex30"}, @@ -73,11 +73,11 @@ func Test_mergeApexVariations(t *testing.T) { { name: "merge updatable", in: []ApexInfo{ - {"foo", "current", false, nil, []string{"foo"}, nil, NotForPrebuiltApex}, - {"bar", "current", true, nil, []string{"bar"}, nil, NotForPrebuiltApex}, + {"foo", FutureApiLevel, false, nil, []string{"foo"}, nil, NotForPrebuiltApex}, + {"bar", FutureApiLevel, true, nil, []string{"bar"}, nil, NotForPrebuiltApex}, }, wantMerged: []ApexInfo{ - {"apex10000", "current", true, nil, []string{"bar", "foo"}, nil, NotForPrebuiltApex}, + {"apex10000", FutureApiLevel, true, nil, []string{"bar", "foo"}, nil, NotForPrebuiltApex}, }, wantAliases: [][2]string{ {"bar", "apex10000"}, @@ -87,12 +87,12 @@ func Test_mergeApexVariations(t *testing.T) { { name: "don't merge sdks", in: []ApexInfo{ - {"foo", "current", false, SdkRefs{{"baz", "1"}}, []string{"foo"}, nil, NotForPrebuiltApex}, - {"bar", "current", false, SdkRefs{{"baz", "2"}}, []string{"bar"}, nil, NotForPrebuiltApex}, + {"foo", FutureApiLevel, false, SdkRefs{{"baz", "1"}}, []string{"foo"}, nil, NotForPrebuiltApex}, + {"bar", FutureApiLevel, false, SdkRefs{{"baz", "2"}}, []string{"bar"}, nil, NotForPrebuiltApex}, }, wantMerged: []ApexInfo{ - {"apex10000_baz_2", "current", false, SdkRefs{{"baz", "2"}}, []string{"bar"}, nil, NotForPrebuiltApex}, - {"apex10000_baz_1", "current", false, SdkRefs{{"baz", "1"}}, []string{"foo"}, nil, NotForPrebuiltApex}, + {"apex10000_baz_2", FutureApiLevel, false, SdkRefs{{"baz", "2"}}, []string{"bar"}, nil, NotForPrebuiltApex}, + {"apex10000_baz_1", FutureApiLevel, false, SdkRefs{{"baz", "1"}}, []string{"foo"}, nil, NotForPrebuiltApex}, }, wantAliases: [][2]string{ {"bar", "apex10000_baz_2"}, @@ -102,15 +102,15 @@ func Test_mergeApexVariations(t *testing.T) { { name: "don't merge when for prebuilt_apex", in: []ApexInfo{ - {"foo", "current", false, nil, []string{"foo"}, nil, NotForPrebuiltApex}, - {"bar", "current", true, nil, []string{"bar"}, nil, NotForPrebuiltApex}, + {"foo", FutureApiLevel, false, nil, []string{"foo"}, nil, NotForPrebuiltApex}, + {"bar", FutureApiLevel, true, nil, []string{"bar"}, nil, NotForPrebuiltApex}, // This one should not be merged in with the others because it is for // a prebuilt_apex. - {"baz", "current", true, nil, []string{"baz"}, nil, ForPrebuiltApex}, + {"baz", FutureApiLevel, true, nil, []string{"baz"}, nil, ForPrebuiltApex}, }, wantMerged: []ApexInfo{ - {"apex10000", "current", true, nil, []string{"bar", "foo"}, nil, NotForPrebuiltApex}, - {"baz", "current", true, nil, []string{"baz"}, nil, ForPrebuiltApex}, + {"apex10000", FutureApiLevel, true, nil, []string{"bar", "foo"}, nil, NotForPrebuiltApex}, + {"baz", FutureApiLevel, true, nil, []string{"baz"}, nil, ForPrebuiltApex}, }, wantAliases: [][2]string{ {"bar", "apex10000"}, diff --git a/apex/apex.go b/apex/apex.go index 0eacf6dcb..f5e6fa944 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -903,7 +903,7 @@ func (a *apexBundle) ApexInfoMutator(mctx android.TopDownMutatorContext) { // be built for this apexBundle. apexInfo := android.ApexInfo{ ApexVariationName: mctx.ModuleName(), - MinSdkVersionStr: minSdkVersion.String(), + MinSdkVersion: minSdkVersion, RequiredSdks: a.RequiredSdks(), Updatable: a.Updatable(), InApexes: []string{mctx.ModuleName()}, diff --git a/cc/cc.go b/cc/cc.go index bef49b855..7f69d56b8 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -2499,7 +2499,7 @@ func (c *Module) depsToPaths(ctx android.ModuleContext) PathDeps { c.apexSdkVersion = android.FutureApiLevel apexInfo := ctx.Provider(android.ApexInfoProvider).(android.ApexInfo) if !apexInfo.IsForPlatform() { - c.apexSdkVersion = apexInfo.MinSdkVersion(ctx) + c.apexSdkVersion = apexInfo.MinSdkVersion } if android.InList("hwaddress", ctx.Config().SanitizeDevice()) {