From 9f720ce52a14a52e064f5a889140be1571f739f6 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 2 Oct 2020 10:26:04 -0700 Subject: [PATCH 1/5] Fix apex_test.go and add it to Android.bp apex_test.go wasn't listed in the Android.bp file, which allowed it to bitrot. Make the API level methods take a PathContext so that they can be called from a test using configErrorWrapper. Also fix an int that was converted to a string. Test: apex_test.go Change-Id: I1ff87134c837bd5d344d22550baabde10d1b0b2e --- android/Android.bp | 1 + android/apex.go | 6 +++--- android/apex_test.go | 36 +++++++++++++++++++----------------- android/api_levels.go | 6 +++--- 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/android/Android.bp b/android/Android.bp index a1b515940..4ba524141 100644 --- a/android/Android.bp +++ b/android/Android.bp @@ -63,6 +63,7 @@ bootstrap_go_package { testSrcs: [ "android_test.go", "androidmk_test.go", + "apex_test.go", "arch_test.go", "config_test.go", "csuite_config_test.go", diff --git a/android/apex.go b/android/apex.go index c88696284..1589a1718 100644 --- a/android/apex.go +++ b/android/apex.go @@ -42,7 +42,7 @@ type ApexInfo struct { InApexes []string } -func (i ApexInfo) mergedName(ctx EarlyModuleContext) string { +func (i ApexInfo) mergedName(ctx PathContext) string { name := "apex" + strconv.Itoa(i.MinSdkVersion(ctx).FinalOrFutureInt()) for _, sdk := range i.RequiredSdks { name += "_" + sdk.Name + "_" + sdk.Version @@ -50,7 +50,7 @@ func (i ApexInfo) mergedName(ctx EarlyModuleContext) string { return name } -func (this *ApexInfo) MinSdkVersion(ctx EarlyModuleContext) ApiLevel { +func (this *ApexInfo) MinSdkVersion(ctx PathContext) ApiLevel { return ApiLevelOrPanic(ctx, this.MinSdkVersionStr) } @@ -358,7 +358,7 @@ func (a byApexName) Less(i, j int) bool { return a[i].ApexVariationName < a[j].A // mergeApexVariations deduplicates APEX variations that would build identically into a common // variation. It returns the reduced list of variations and a list of aliases from the original // variation names to the new variation names. -func mergeApexVariations(ctx EarlyModuleContext, apexVariations []ApexInfo) (merged []ApexInfo, aliases [][2]string) { +func mergeApexVariations(ctx PathContext, apexVariations []ApexInfo) (merged []ApexInfo, aliases [][2]string) { sort.Sort(byApexName(apexVariations)) seen := make(map[string]int) for _, apexInfo := range apexVariations { diff --git a/android/apex_test.go b/android/apex_test.go index db02833e7..dd372f792 100644 --- a/android/apex_test.go +++ b/android/apex_test.go @@ -29,10 +29,10 @@ func Test_mergeApexVariations(t *testing.T) { { name: "single", in: []ApexInfo{ - {"foo", 10000, false, nil, []string{"foo"}}, + {"foo", "current", false, nil, []string{"foo"}}, }, wantMerged: []ApexInfo{ - {"apex10000", 10000, false, nil, []string{"foo"}}, + {"apex10000", "current", false, nil, []string{"foo"}}, }, wantAliases: [][2]string{ {"foo", "apex10000"}, @@ -41,11 +41,11 @@ func Test_mergeApexVariations(t *testing.T) { { name: "merge", in: []ApexInfo{ - {"foo", 10000, false, SdkRefs{{"baz", "1"}}, []string{"foo"}}, - {"bar", 10000, false, SdkRefs{{"baz", "1"}}, []string{"bar"}}, + {"foo", "current", false, SdkRefs{{"baz", "1"}}, []string{"foo"}}, + {"bar", "current", false, SdkRefs{{"baz", "1"}}, []string{"bar"}}, }, wantMerged: []ApexInfo{ - {"apex10000_baz_1", 10000, false, SdkRefs{{"baz", "1"}}, []string{"bar", "foo"}}, + {"apex10000_baz_1", "current", false, SdkRefs{{"baz", "1"}}, []string{"bar", "foo"}}, }, wantAliases: [][2]string{ {"bar", "apex10000_baz_1"}, @@ -55,12 +55,12 @@ func Test_mergeApexVariations(t *testing.T) { { name: "don't merge version", in: []ApexInfo{ - {"foo", 10000, false, nil, []string{"foo"}}, - {"bar", 30, false, nil, []string{"bar"}}, + {"foo", "current", false, nil, []string{"foo"}}, + {"bar", "30", false, nil, []string{"bar"}}, }, wantMerged: []ApexInfo{ - {"apex30", 30, false, nil, []string{"bar"}}, - {"apex10000", 10000, false, nil, []string{"foo"}}, + {"apex30", "30", false, nil, []string{"bar"}}, + {"apex10000", "current", false, nil, []string{"foo"}}, }, wantAliases: [][2]string{ {"bar", "apex30"}, @@ -70,11 +70,11 @@ func Test_mergeApexVariations(t *testing.T) { { name: "merge updatable", in: []ApexInfo{ - {"foo", 10000, false, nil, []string{"foo"}}, - {"bar", 10000, true, nil, []string{"bar"}}, + {"foo", "current", false, nil, []string{"foo"}}, + {"bar", "current", true, nil, []string{"bar"}}, }, wantMerged: []ApexInfo{ - {"apex10000", 10000, true, nil, []string{"bar", "foo"}}, + {"apex10000", "current", true, nil, []string{"bar", "foo"}}, }, wantAliases: [][2]string{ {"bar", "apex10000"}, @@ -84,12 +84,12 @@ func Test_mergeApexVariations(t *testing.T) { { name: "don't merge sdks", in: []ApexInfo{ - {"foo", 10000, false, SdkRefs{{"baz", "1"}}, []string{"foo"}}, - {"bar", 10000, false, SdkRefs{{"baz", "2"}}, []string{"bar"}}, + {"foo", "current", false, SdkRefs{{"baz", "1"}}, []string{"foo"}}, + {"bar", "current", false, SdkRefs{{"baz", "2"}}, []string{"bar"}}, }, wantMerged: []ApexInfo{ - {"apex10000_baz_2", 10000, false, SdkRefs{{"baz", "2"}}, []string{"bar"}}, - {"apex10000_baz_1", 10000, false, SdkRefs{{"baz", "1"}}, []string{"foo"}}, + {"apex10000_baz_2", "current", false, SdkRefs{{"baz", "2"}}, []string{"bar"}}, + {"apex10000_baz_1", "current", false, SdkRefs{{"baz", "1"}}, []string{"foo"}}, }, wantAliases: [][2]string{ {"bar", "apex10000_baz_2"}, @@ -99,7 +99,9 @@ func Test_mergeApexVariations(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gotMerged, gotAliases := mergeApexVariations(tt.in) + config := TestConfig(buildDir, nil, "", nil) + ctx := &configErrorWrapper{config: config} + gotMerged, gotAliases := mergeApexVariations(ctx, tt.in) if !reflect.DeepEqual(gotMerged, tt.wantMerged) { t.Errorf("mergeApexVariations() gotMerged = %v, want %v", gotMerged, tt.wantMerged) } diff --git a/android/api_levels.go b/android/api_levels.go index 97683404e..bace3d455 100644 --- a/android/api_levels.go +++ b/android/api_levels.go @@ -152,7 +152,7 @@ var FirstNonLibAndroidSupportVersion = uncheckedFinalApiLevel(21) // * "30" -> "30" // * "R" -> "30" // * "S" -> "S" -func ReplaceFinalizedCodenames(ctx EarlyModuleContext, raw string) string { +func ReplaceFinalizedCodenames(ctx PathContext, raw string) string { num, ok := getFinalCodenamesMap(ctx.Config())[raw] if !ok { return raw @@ -175,7 +175,7 @@ func ReplaceFinalizedCodenames(ctx EarlyModuleContext, raw string) string { // // Inputs that are not "current", known previews, or convertible to an integer // will return an error. -func ApiLevelFromUser(ctx EarlyModuleContext, raw string) (ApiLevel, error) { +func ApiLevelFromUser(ctx PathContext, raw string) (ApiLevel, error) { if raw == "" { panic("API level string must be non-empty") } @@ -203,7 +203,7 @@ func ApiLevelFromUser(ctx EarlyModuleContext, raw string) (ApiLevel, error) { // Converts an API level string `raw` into an ApiLevel in the same method as // `ApiLevelFromUser`, but the input is assumed to have no errors and any errors // will panic instead of returning an error. -func ApiLevelOrPanic(ctx EarlyModuleContext, raw string) ApiLevel { +func ApiLevelOrPanic(ctx PathContext, raw string) ApiLevel { value, err := ApiLevelFromUser(ctx, raw) if err != nil { panic(err.Error()) From b6135218a4e9788c3f7a40a82775216a64100652 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 1 Oct 2020 11:13:27 -0700 Subject: [PATCH 2/5] Remove vendor crt special case It doesn't seem to be necessary, crt objects can get headers via local_include_dirs. Test: m checkbuild Change-Id: Id357adc054c85576aa9935d0acbee5f1ae3dcbff --- cc/object.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/cc/object.go b/cc/object.go index 778d131ba..ab2672b3c 100644 --- a/cc/object.go +++ b/cc/object.go @@ -96,11 +96,6 @@ func (object *objectLinker) linkerProps() []interface{} { func (*objectLinker) linkerInit(ctx BaseModuleContext) {} func (object *objectLinker) linkerDeps(ctx DepsContext, deps Deps) Deps { - if ctx.useVndk() && ctx.toolchain().Bionic() { - // Needed for VNDK builds where bionic headers aren't automatically added. - deps.LateSharedLibs = append(deps.LateSharedLibs, "libc") - } - deps.HeaderLibs = append(deps.HeaderLibs, object.Properties.Header_libs...) deps.ObjFiles = append(deps.ObjFiles, object.Properties.Objs...) return deps From 1348ce3f1399ce7cf51dec139e1292846975e953 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 1 Oct 2020 13:37:16 -0700 Subject: [PATCH 3/5] Don't make SplitPerApiLevel imply UseSdk UseSdk was returning true when SplitPerApiLevel returned true, which was causing the platform variant of SplitPerApiLevel module to compile against the SDK. Check SplitPerApiLevel separately in the sdkMutator instead. Test: m checkbuild Change-Id: I0ae667d48a3b7b96709a6cad8e8ea9701659fc2a --- cc/cc.go | 5 ++++- cc/linkable.go | 2 ++ cc/sdk.go | 4 ++-- rust/rust.go | 4 ++++ 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/cc/cc.go b/cc/cc.go index caefea4f6..006a4a596 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -697,6 +697,9 @@ func (c *Module) MinSdkVersion() string { } func (c *Module) SplitPerApiLevel() bool { + if !c.canUseSdk() { + return false + } if linker, ok := c.linker.(*objectLinker); ok { return linker.isCrt() } @@ -1026,7 +1029,7 @@ func (c *Module) canUseSdk() bool { func (c *Module) UseSdk() bool { if c.canUseSdk() { - return String(c.Properties.Sdk_version) != "" || c.SplitPerApiLevel() + return String(c.Properties.Sdk_version) != "" } return false } diff --git a/cc/linkable.go b/cc/linkable.go index 6d8a4b71e..a67cd4e4c 100644 --- a/cc/linkable.go +++ b/cc/linkable.go @@ -63,6 +63,8 @@ type LinkableInterface interface { ToolchainLibrary() bool NdkPrebuiltStl() bool StubDecorator() bool + + SplitPerApiLevel() bool } var ( diff --git a/cc/sdk.go b/cc/sdk.go index b68baad01..ec57f06f4 100644 --- a/cc/sdk.go +++ b/cc/sdk.go @@ -32,11 +32,11 @@ func sdkMutator(ctx android.BottomUpMutatorContext) { switch m := ctx.Module().(type) { case LinkableInterface: if m.AlwaysSdk() { - if !m.UseSdk() { + if !m.UseSdk() && !m.SplitPerApiLevel() { ctx.ModuleErrorf("UseSdk() must return true when AlwaysSdk is set, did the factory forget to set Sdk_version?") } ctx.CreateVariations("sdk") - } else if m.UseSdk() { + } else if m.UseSdk() || m.SplitPerApiLevel() { modules := ctx.CreateVariations("", "sdk") modules[0].(*Module).Properties.Sdk_version = nil modules[1].(*Module).Properties.IsSdkVariant = true diff --git a/rust/rust.go b/rust/rust.go index 68e5a5be8..6004c6825 100644 --- a/rust/rust.go +++ b/rust/rust.go @@ -221,6 +221,10 @@ func (mod *Module) IsSdkVariant() bool { return false } +func (mod *Module) SplitPerApiLevel() bool { + return false +} + func (mod *Module) ToolchainLibrary() bool { return false } From 3146c5cd67f728375aa9bdfa3b0643895a32411e Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 30 Sep 2020 15:34:40 -0700 Subject: [PATCH 4/5] Create fewer empty version variants Don't create empty version variants for binaries, objects, rust rlibs or rust dylibs. Test: no change to build.ninja Change-Id: I62d4d43da476eafdb258a08b5ada758bb2971a1a --- cc/binary_sdk_member.go | 11 +++-------- cc/cc.go | 6 +++--- cc/library.go | 19 ++++++++++++++++--- cc/library_sdk_member.go | 7 +++++-- rust/rust.go | 6 +----- 5 files changed, 28 insertions(+), 21 deletions(-) diff --git a/cc/binary_sdk_member.go b/cc/binary_sdk_member.go index 55e400e8e..ebf89eaec 100644 --- a/cc/binary_sdk_member.go +++ b/cc/binary_sdk_member.go @@ -40,19 +40,14 @@ type binarySdkMemberType struct { func (mt *binarySdkMemberType) AddDependencies(mctx android.BottomUpMutatorContext, dependencyTag blueprint.DependencyTag, names []string) { targets := mctx.MultiTargets() - for _, lib := range names { + for _, bin := range names { for _, target := range targets { - name, version := StubsLibNameAndVersion(lib) - if version == "" { - version = "latest" - } variations := target.Variations() if mctx.Device() { variations = append(variations, - blueprint.Variation{Mutator: "image", Variation: android.CoreVariation}, - blueprint.Variation{Mutator: "version", Variation: version}) + blueprint.Variation{Mutator: "image", Variation: android.CoreVariation}) } - mctx.AddFarVariationDependencies(variations, dependencyTag, name) + mctx.AddFarVariationDependencies(variations, dependencyTag, bin) } } } diff --git a/cc/cc.go b/cc/cc.go index 006a4a596..e7aa1accd 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -1871,7 +1871,7 @@ func (c *Module) addSharedLibDependenciesWithVersions(ctx android.BottomUpMutato variations = append([]blueprint.Variation(nil), variations...) - if version != "" && VersionVariantAvailable(c) { + if version != "" && CanBeOrLinkAgainstVersionVariants(c) { // Version is explicitly specified. i.e. libFoo#30 variations = append(variations, blueprint.Variation{Mutator: "version", Variation: version}) depTag.explicitlyVersioned = true @@ -1886,7 +1886,7 @@ func (c *Module) addSharedLibDependenciesWithVersions(ctx android.BottomUpMutato // If the version is not specified, add dependency to all stubs libraries. // The stubs library will be used when the depending module is built for APEX and // the dependent module is not in the same APEX. - if version == "" && VersionVariantAvailable(c) { + if version == "" && CanBeOrLinkAgainstVersionVariants(c) { if dep, ok := deps[0].(*Module); ok { for _, ver := range dep.AllStubsVersions() { // Note that depTag.ExplicitlyVersioned is false in this case. @@ -2492,7 +2492,7 @@ func (c *Module) depsToPaths(ctx android.ModuleContext) PathDeps { if ccDep.CcLibrary() && !libDepTag.static() { depIsStubs := ccDep.BuildStubs() - depHasStubs := VersionVariantAvailable(c) && ccDep.HasStubsVariants() + depHasStubs := CanBeOrLinkAgainstVersionVariants(c) && ccDep.HasStubsVariants() depInSameApexes := android.DirectlyInAllApexes(c.InApexes(), depName) depInPlatform := !android.DirectlyInAnyApex(ctx, depName) diff --git a/cc/library.go b/cc/library.go index bf7868ff7..059e29b9c 100644 --- a/cc/library.go +++ b/cc/library.go @@ -1541,7 +1541,7 @@ func createVersionVariations(mctx android.BottomUpMutatorContext, versions []str mctx.CreateAliasVariation("latest", latestVersion) } -func VersionVariantAvailable(module interface { +func CanBeOrLinkAgainstVersionVariants(module interface { Host() bool InRamdisk() bool InRecovery() bool @@ -1549,10 +1549,23 @@ func VersionVariantAvailable(module interface { return !module.Host() && !module.InRamdisk() && !module.InRecovery() } +func CanBeVersionVariant(module interface { + Host() bool + InRamdisk() bool + InRecovery() bool + CcLibraryInterface() bool + Shared() bool + Static() bool +}) bool { + return CanBeOrLinkAgainstVersionVariants(module) && + module.CcLibraryInterface() && (module.Shared() || module.Static()) +} + // versionSelector normalizes the versions in the Stubs.Versions property into MutatedProperties.AllStubsVersions, // and propagates the value from implementation libraries to llndk libraries with the same name. func versionSelectorMutator(mctx android.BottomUpMutatorContext) { - if library, ok := mctx.Module().(LinkableInterface); ok && VersionVariantAvailable(library) { + if library, ok := mctx.Module().(LinkableInterface); ok && CanBeVersionVariant(library) { + if library.CcLibrary() && library.BuildSharedVariant() && len(library.StubsVersions()) > 0 && !library.IsSdkVariant() { @@ -1582,7 +1595,7 @@ func versionSelectorMutator(mctx android.BottomUpMutatorContext) { // versionMutator splits a module into the mandatory non-stubs variant // (which is unnamed) and zero or more stubs variants. func versionMutator(mctx android.BottomUpMutatorContext) { - if library, ok := mctx.Module().(LinkableInterface); ok && VersionVariantAvailable(library) { + if library, ok := mctx.Module().(LinkableInterface); ok && CanBeVersionVariant(library) { createVersionVariations(mctx, library.AllStubsVersions()) } } diff --git a/cc/library_sdk_member.go b/cc/library_sdk_member.go index 41ce29422..765fe710f 100644 --- a/cc/library_sdk_member.go +++ b/cc/library_sdk_member.go @@ -85,8 +85,11 @@ func (mt *librarySdkMemberType) AddDependencies(mctx android.BottomUpMutatorCont variations := target.Variations() if mctx.Device() { variations = append(variations, - blueprint.Variation{Mutator: "image", Variation: android.CoreVariation}, - blueprint.Variation{Mutator: "version", Variation: version}) + blueprint.Variation{Mutator: "image", Variation: android.CoreVariation}) + if mt.linkTypes != nil { + variations = append(variations, + blueprint.Variation{Mutator: "version", Variation: version}) + } } if mt.linkTypes == nil { mctx.AddFarVariationDependencies(variations, dependencyTag, name) diff --git a/rust/rust.go b/rust/rust.go index 6004c6825..1fc3b0e2c 100644 --- a/rust/rust.go +++ b/rust/rust.go @@ -997,11 +997,7 @@ func (mod *Module) DepsMutator(actx android.BottomUpMutatorContext) { } deps := mod.deps(ctx) - commonDepVariations := []blueprint.Variation{} - if cc.VersionVariantAvailable(mod) { - commonDepVariations = append(commonDepVariations, - blueprint.Variation{Mutator: "version", Variation: ""}) - } + var commonDepVariations []blueprint.Variation if !mod.Host() { commonDepVariations = append(commonDepVariations, blueprint.Variation{Mutator: "image", Variation: android.CoreVariation}) From 565cafdcb1178c0d6804583546ecd8069439de0c Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 25 Sep 2020 18:47:38 -0700 Subject: [PATCH 5/5] Don't create version variations of sdk modules They are never used, skip creating them. Test: m checkbuild Change-Id: I4c8cd544327ae79b781f704be5a9064efdbdf2af --- cc/library.go | 4 +++- rust/rust.go | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/cc/library.go b/cc/library.go index 059e29b9c..b5bec952a 100644 --- a/cc/library.go +++ b/cc/library.go @@ -1545,14 +1545,16 @@ func CanBeOrLinkAgainstVersionVariants(module interface { Host() bool InRamdisk() bool InRecovery() bool + UseSdk() bool }) bool { - return !module.Host() && !module.InRamdisk() && !module.InRecovery() + return !module.Host() && !module.InRamdisk() && !module.InRecovery() && !module.UseSdk() } func CanBeVersionVariant(module interface { Host() bool InRamdisk() bool InRecovery() bool + UseSdk() bool CcLibraryInterface() bool Shared() bool Static() bool diff --git a/rust/rust.go b/rust/rust.go index 1fc3b0e2c..9b18c8c76 100644 --- a/rust/rust.go +++ b/rust/rust.go @@ -1053,7 +1053,7 @@ func (mod *Module) DepsMutator(actx android.BottomUpMutatorContext) { blueprint.Variation{Mutator: "link", Variation: "static"}), cc.StaticDepTag(), deps.StaticLibs...) - crtVariations := append(cc.GetCrtVariations(ctx, mod), commonDepVariations...) + crtVariations := cc.GetCrtVariations(ctx, mod) if deps.CrtBegin != "" { actx.AddVariationDependencies(crtVariations, cc.CrtBeginDepTag, deps.CrtBegin) }