From 094faa5b26bd4d181c48c25b84d786d2a0f0e743 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 6 Aug 2020 17:38:25 -0700 Subject: [PATCH 1/2] Use local variations for versionsMutator Use local variations and an alias for the "" variation for versionMutator. Local variations are used here because the variation of one module doesn't affect the variation it should depend on - a module with variation version:29 will manually link against dependencies with the correct version variation, and don't need it to be automatically resolved. The alias allows other modules to depend on a module mutated by the version mutator without specifying the version: "" variation. This allows removing the variation on genrule modules. The motiviation here is to make sure versionMutator calls AliasVariation, which will allow the previous linkageMutator to also use AliasVariation. Bug: 162437057 Test: no change to build.ninja, make_vars-aosp_crosshatch.mk, Android-aosp_crosshatch.mk or late-aosp_crosshatch.mk Change-Id: Ibb030cc4334e47511b8ab5bc3eb8a5ae39ad0023 --- cc/library.go | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/cc/library.go b/cc/library.go index 2a329ac3c..ad152d526 100644 --- a/cc/library.go +++ b/cc/library.go @@ -28,7 +28,6 @@ import ( "android/soong/android" "android/soong/cc/config" - "android/soong/genrule" ) type LibraryProperties struct { @@ -1559,13 +1558,14 @@ func createVersionVariations(mctx android.BottomUpMutatorContext, versions []str // "" is for the non-stubs variant versions = append([]string{""}, versions...) - modules := mctx.CreateVariations(versions...) + modules := mctx.CreateLocalVariations(versions...) for i, m := range modules { if versions[i] != "" { m.(LinkableInterface).SetBuildStubs() m.(LinkableInterface).SetStubsVersions(versions[i]) } } + mctx.AliasVariation("") } func VersionVariantAvailable(module interface { @@ -1600,7 +1600,7 @@ func VersionMutator(mctx android.BottomUpMutatorContext) { if c, ok := library.(*Module); ok && c.IsStubs() { stubsVersionsLock.Lock() defer stubsVersionsLock.Unlock() - // For LLNDK llndk_library, we borrow vstubs.ersions from its implementation library. + // For LLNDK llndk_library, we borrow stubs.versions from its implementation library. // Since llndk_library has dependency to its implementation library, // we can safely access stubsVersionsFor() with its baseModuleName. versions := stubsVersionsFor(mctx.Config())[c.BaseModuleName()] @@ -1611,17 +1611,10 @@ func VersionMutator(mctx android.BottomUpMutatorContext) { return } - mctx.CreateVariations("") + mctx.CreateLocalVariations("") + mctx.AliasVariation("") return } - if genrule, ok := mctx.Module().(*genrule.Module); ok { - if _, ok := genrule.Extra.(*GenruleExtraProperties); ok { - if VersionVariantAvailable(genrule) { - mctx.CreateVariations("") - return - } - } - } } // maybeInjectBoringSSLHash adds a rule to run bssl_inject_hash on the output file if the module has the From 81ca6cd4072add2b910ca68cc1f945ea77bf7660 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 6 Aug 2020 17:46:48 -0700 Subject: [PATCH 2/2] Add alias variations to linkageMutator Alias the shared variation if it exists, otherwise the static variation. This allows modules that are not aware of shared library variations (like cc_genrule) to depend on shared libraries by depending on just the normal image, os and arch variations. Bug: 162437057 Test: TestLibraryGenruleCmd Change-Id: Icec57d43538e01ab05cc50d4e3f9a11cc55f0162 --- cc/genrule_test.go | 39 +++++++++++++++++++++++++++++++++++++++ cc/library.go | 10 ++++++++++ 2 files changed, 49 insertions(+) diff --git a/cc/genrule_test.go b/cc/genrule_test.go index d38cf27f0..a366f765c 100644 --- a/cc/genrule_test.go +++ b/cc/genrule_test.go @@ -76,3 +76,42 @@ func TestArchGenruleCmd(t *testing.T) { t.Errorf(`want arm64 inputs %v, got %v`, expected, gen.Inputs.Strings()) } } + +func TestLibraryGenruleCmd(t *testing.T) { + bp := ` + cc_library { + name: "libboth", + } + + cc_library_shared { + name: "libshared", + } + + cc_library_static { + name: "libstatic", + } + + cc_genrule { + name: "gen", + tool_files: ["tool"], + srcs: [ + ":libboth", + ":libshared", + ":libstatic", + ], + cmd: "$(location tool) $(in) $(out)", + out: ["out"], + } + ` + ctx := testCc(t, bp) + + gen := ctx.ModuleForTests("gen", "android_arm_armv7-a-neon").Output("out") + expected := []string{"libboth.so", "libshared.so", "libstatic.a"} + var got []string + for _, input := range gen.Inputs { + got = append(got, input.Base()) + } + if !reflect.DeepEqual(expected, got) { + t.Errorf(`want inputs %v, got %v`, expected, got) + } +} diff --git a/cc/library.go b/cc/library.go index ad152d526..9e6297420 100644 --- a/cc/library.go +++ b/cc/library.go @@ -1469,6 +1469,12 @@ func LinkageMutator(mctx android.BottomUpMutatorContext) { static.linker.(prebuiltLibraryInterface).setStatic() shared.linker.(prebuiltLibraryInterface).setShared() + if library.buildShared() { + mctx.AliasVariation("shared") + } else if library.buildStatic() { + mctx.AliasVariation("static") + } + if !library.buildStatic() { static.linker.(prebuiltLibraryInterface).disablePrebuilt() } @@ -1500,18 +1506,22 @@ func LinkageMutator(mctx android.BottomUpMutatorContext) { if _, ok := library.(*Module); ok { reuseStaticLibrary(mctx, static.(*Module), shared.(*Module)) } + mctx.AliasVariation("shared") } else if library.BuildStaticVariant() { variations := append([]string{"static"}, variations...) modules := mctx.CreateLocalVariations(variations...) modules[0].(LinkableInterface).SetStatic() + mctx.AliasVariation("static") } else if library.BuildSharedVariant() { variations := append([]string{"shared"}, variations...) modules := mctx.CreateLocalVariations(variations...) modules[0].(LinkableInterface).SetShared() + mctx.AliasVariation("shared") } else if len(variations) > 0 { mctx.CreateLocalVariations(variations...) + mctx.AliasVariation(variations[0]) } } }