From d4a0063d3effc56392d6cc39fa2145adf53c5caa Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Tue, 12 Apr 2022 12:23:20 +0900 Subject: [PATCH] Remove apex10000_private variants When a library is included in two APEXes whose platform_apis settings are different, two apex variants of the library is created: apex1000 and apex1000_private. This change was introduced with ag/15061306, especially by the commit [1]. However, that part should be reverted because it actually creates unnecessary variants. It's unnecessary because the two variants of the library are compiled (excluding the linking) exactly the same. If a private symbol of its dependency was actually used when compiling the apex1000_private variant, then the other apex1000 variant wouldn't have been built because that private symbol must have caused a linkage error. [1] https://googleplex-android-review.git.corp.google.com/c/platform/build/soong/+/15061306/2..4/android/apex.go#b527). Bug: 228785792 Test: m Change-Id: Id58d3e98a51de5e628ca72ef86e9cd11b0ee8971 --- android/apex.go | 10 +++------- android/apex_test.go | 21 +++++++++++++++++---- apex/apex_test.go | 4 ++-- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/android/apex.go b/android/apex.go index cf1bcfef4..b127f7410 100644 --- a/android/apex.go +++ b/android/apex.go @@ -113,9 +113,6 @@ func (i ApexInfo) mergedName(ctx PathContext) string { for _, sdk := range i.RequiredSdks { name += "_" + sdk.Name + "_" + sdk.Version } - if i.UsePlatformApis { - name += "_private" - } return name } @@ -546,10 +543,9 @@ func mergeApexVariations(ctx PathContext, apexInfos []ApexInfo) (merged []ApexIn merged[index].InApexModules = append(merged[index].InApexModules, apexInfo.InApexModules...) merged[index].ApexContents = append(merged[index].ApexContents, apexInfo.ApexContents...) merged[index].Updatable = merged[index].Updatable || apexInfo.Updatable - if merged[index].UsePlatformApis != apexInfo.UsePlatformApis { - panic(fmt.Errorf("variants having different UsePlatformApis can't be merged")) - } - merged[index].UsePlatformApis = apexInfo.UsePlatformApis + // Platform APIs is allowed for this module only when all APEXes containing + // the module are with `use_platform_apis: true`. + merged[index].UsePlatformApis = merged[index].UsePlatformApis && apexInfo.UsePlatformApis } else { seen[mergedName] = len(merged) apexInfo.ApexVariationName = mergedName diff --git a/android/apex_test.go b/android/apex_test.go index 60a639b4b..1e2f3bde4 100644 --- a/android/apex_test.go +++ b/android/apex_test.go @@ -118,17 +118,30 @@ func Test_mergeApexVariations(t *testing.T) { }, }, { - name: "don't merge different UsePlatformApis", + name: "merge different UsePlatformApis but don't allow using platform api", in: []ApexInfo{ {"foo", FutureApiLevel, false, false, nil, []string{"foo"}, []string{"foo"}, nil, NotForPrebuiltApex}, {"bar", FutureApiLevel, false, true, nil, []string{"bar"}, []string{"bar"}, nil, NotForPrebuiltApex}, }, wantMerged: []ApexInfo{ - {"apex10000_private", FutureApiLevel, false, true, nil, []string{"bar"}, []string{"bar"}, nil, NotForPrebuiltApex}, - {"apex10000", FutureApiLevel, false, false, nil, []string{"foo"}, []string{"foo"}, nil, NotForPrebuiltApex}, + {"apex10000", FutureApiLevel, false, false, nil, []string{"bar", "foo"}, []string{"bar", "foo"}, nil, NotForPrebuiltApex}, }, wantAliases: [][2]string{ - {"bar", "apex10000_private"}, + {"bar", "apex10000"}, + {"foo", "apex10000"}, + }, + }, + { + name: "merge same UsePlatformApis and allow using platform api", + in: []ApexInfo{ + {"foo", FutureApiLevel, false, true, nil, []string{"foo"}, []string{"foo"}, nil, NotForPrebuiltApex}, + {"bar", FutureApiLevel, false, true, nil, []string{"bar"}, []string{"bar"}, nil, NotForPrebuiltApex}, + }, + wantMerged: []ApexInfo{ + {"apex10000", FutureApiLevel, false, true, nil, []string{"bar", "foo"}, []string{"bar", "foo"}, nil, NotForPrebuiltApex}, + }, + wantAliases: [][2]string{ + {"bar", "apex10000"}, {"foo", "apex10000"}, }, }, diff --git a/apex/apex_test.go b/apex/apex_test.go index 5021f275d..9c440939d 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -1038,10 +1038,10 @@ func TestApexCanUsePrivateApis(t *testing.T) { // Ensure that we are using non-stub variants of mylib2 and libfoo.shared_from_rust (because // of the platform_apis: true) - mylibLdFlags := ctx.ModuleForTests("mylib", "android_arm64_armv8-a_shared_apex10000_private").Rule("ld").Args["libFlags"] + mylibLdFlags := ctx.ModuleForTests("mylib", "android_arm64_armv8-a_shared_apex10000").Rule("ld").Args["libFlags"] ensureNotContains(t, mylibLdFlags, "mylib2/android_arm64_armv8-a_shared_current/mylib2.so") ensureContains(t, mylibLdFlags, "mylib2/android_arm64_armv8-a_shared/mylib2.so") - rustDeps := ctx.ModuleForTests("foo.rust", "android_arm64_armv8-a_apex10000_private").Rule("rustc").Args["linkFlags"] + rustDeps := ctx.ModuleForTests("foo.rust", "android_arm64_armv8-a_apex10000").Rule("rustc").Args["linkFlags"] ensureNotContains(t, rustDeps, "libfoo.shared_from_rust/android_arm64_armv8-a_shared_current/libfoo.shared_from_rust.so") ensureContains(t, rustDeps, "libfoo.shared_from_rust/android_arm64_armv8-a_shared/libfoo.shared_from_rust.so") }