From d1f898e70abf0e750ae351689a6291700ff8739c Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 18 Aug 2020 18:35:15 -0700 Subject: [PATCH] Remove global state from version mutator A per-context variable is used to store the list of modules that contain stubs and their available versions. Stores the list of the stubs versions on the implementation module, and then use the new return values from AddVariationDependencies to expand dependencies on implementation libraries to also depend on the stubs libraries. Adds a new mutator pass to propagate list of stub versions to llndk libraries. Also creates an alias version variation called "latest" to allow depending on the latest version without having to know what it is. Test: all Soong tests Test: no change to build.ninja, Android-${TARGET_PRODUCT}.mk, make_vars-${TARGET_PRODUCT}.mk or late-${TARGET_PRODUCT}.mk Change-Id: If19659e2e5828c860fd4d679ef79a414b7ea2efc --- cc/binary_sdk_member.go | 2 +- cc/cc.go | 46 ++++++++++++++++------ cc/cc_test.go | 1 + cc/library.go | 82 ++++++++++++++++------------------------ cc/library_sdk_member.go | 2 +- cc/linkable.go | 4 +- rust/rust.go | 14 +++++-- 7 files changed, 85 insertions(+), 66 deletions(-) diff --git a/cc/binary_sdk_member.go b/cc/binary_sdk_member.go index a1abc728a..55e400e8e 100644 --- a/cc/binary_sdk_member.go +++ b/cc/binary_sdk_member.go @@ -44,7 +44,7 @@ func (mt *binarySdkMemberType) AddDependencies(mctx android.BottomUpMutatorConte for _, target := range targets { name, version := StubsLibNameAndVersion(lib) if version == "" { - version = LatestStubsVersionFor(mctx.Config(), name) + version = "latest" } variations := target.Variations() if mctx.Device() { diff --git a/cc/cc.go b/cc/cc.go index 70229be5a..fb78b2073 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -47,7 +47,8 @@ func RegisterCCBuildComponents(ctx android.RegistrationContext) { ctx.BottomUp("link", LinkageMutator).Parallel() ctx.BottomUp("ndk_api", NdkApiMutator).Parallel() ctx.BottomUp("test_per_src", TestPerSrcMutator).Parallel() - ctx.BottomUp("version", VersionMutator).Parallel() + ctx.BottomUp("version_selector", versionSelectorMutator).Parallel() + ctx.BottomUp("version", versionMutator).Parallel() ctx.BottomUp("begin", BeginMutator).Parallel() ctx.BottomUp("sysprop_cc", SyspropMutator).Parallel() ctx.BottomUp("vendor_snapshot", VendorSnapshotMutator).Parallel() @@ -784,7 +785,28 @@ func (c *Module) BuildStubs() bool { panic(fmt.Errorf("BuildStubs called on non-library module: %q", c.BaseModuleName())) } -func (c *Module) SetStubsVersions(version string) { +func (c *Module) SetAllStubsVersions(versions []string) { + if library, ok := c.linker.(*libraryDecorator); ok { + library.MutatedProperties.AllStubsVersions = versions + return + } + if llndk, ok := c.linker.(*llndkStubDecorator); ok { + llndk.libraryDecorator.MutatedProperties.AllStubsVersions = versions + return + } +} + +func (c *Module) AllStubsVersions() []string { + if library, ok := c.linker.(*libraryDecorator); ok { + return library.MutatedProperties.AllStubsVersions + } + if llndk, ok := c.linker.(*llndkStubDecorator); ok { + return llndk.libraryDecorator.MutatedProperties.AllStubsVersions + } + return nil +} + +func (c *Module) SetStubsVersion(version string) { if c.linker != nil { if library, ok := c.linker.(*libraryDecorator); ok { library.MutatedProperties.StubsVersion = version @@ -795,7 +817,7 @@ func (c *Module) SetStubsVersions(version string) { return } } - panic(fmt.Errorf("SetStubsVersions called on non-library module: %q", c.BaseModuleName())) + panic(fmt.Errorf("SetStubsVersion called on non-library module: %q", c.BaseModuleName())) } func (c *Module) StubsVersion() string { @@ -1994,18 +2016,20 @@ func (c *Module) DepsMutator(actx android.BottomUpMutatorContext) { variations = append(variations, blueprint.Variation{Mutator: "version", Variation: version}) depTag.explicitlyVersioned = true } - actx.AddVariationDependencies(variations, depTag, name) + deps := actx.AddVariationDependencies(variations, depTag, name) // 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) { - for _, ver := range stubsVersionsFor(actx.Config())[name] { - // Note that depTag.ExplicitlyVersioned is false in this case. - actx.AddVariationDependencies([]blueprint.Variation{ - {Mutator: "link", Variation: "shared"}, - {Mutator: "version", Variation: ver}, - }, depTag, name) + if dep, ok := deps[0].(*Module); ok { + for _, ver := range dep.AllStubsVersions() { + // Note that depTag.ExplicitlyVersioned is false in this case. + ctx.AddVariationDependencies([]blueprint.Variation{ + {Mutator: "link", Variation: "shared"}, + {Mutator: "version", Variation: ver}, + }, depTag, name) + } } } } @@ -2457,7 +2481,7 @@ func (c *Module) depsToPaths(ctx android.ModuleContext) PathDeps { if m, ok := ccDep.(*Module); ok && m.IsStubs() { // LLNDK // by default, use current version of LLNDK versionToUse := "" - versions := stubsVersionsFor(ctx.Config())[depName] + versions := m.AllStubsVersions() if c.ApexVariationName() != "" && len(versions) > 0 { // if this is for use_vendor apex && dep has stubsVersions // apply the same rule of apex sdk enforcement to choose right version diff --git a/cc/cc_test.go b/cc/cc_test.go index a4c067772..132d09136 100644 --- a/cc/cc_test.go +++ b/cc/cc_test.go @@ -3025,6 +3025,7 @@ func TestStaticLibDepReorderingWithShared(t *testing.T) { } func checkEquals(t *testing.T, message string, expected, actual interface{}) { + t.Helper() if !reflect.DeepEqual(actual, expected) { t.Errorf(message+ "\nactual: %v"+ diff --git a/cc/library.go b/cc/library.go index 92853b5d3..7b09b1145 100644 --- a/cc/library.go +++ b/cc/library.go @@ -152,6 +152,8 @@ type LibraryMutatedProperties struct { BuildStubs bool `blueprint:"mutated"` // Version of the stubs lib StubsVersion string `blueprint:"mutated"` + // List of all stubs versions associated with an implementation lib + AllStubsVersions []string `blueprint:"mutated"` } type FlagExporterProperties struct { @@ -1517,26 +1519,6 @@ func LinkageMutator(mctx android.BottomUpMutatorContext) { } } -var stubVersionsKey = android.NewOnceKey("stubVersions") - -// maps a module name to the list of stubs versions available for the module -func stubsVersionsFor(config android.Config) map[string][]string { - return config.Once(stubVersionsKey, func() interface{} { - return make(map[string][]string) - }).(map[string][]string) -} - -var stubsVersionsLock sync.Mutex - -func LatestStubsVersionFor(config android.Config, name string) string { - versions, ok := stubsVersionsFor(config)[name] - if ok && len(versions) > 0 { - // the versions are alreay sorted in ascending order - return versions[len(versions)-1] - } - return "" -} - func normalizeVersions(ctx android.BaseModuleContext, versions []string) { numVersions := make([]int, len(versions)) for i, v := range versions { @@ -1556,17 +1538,22 @@ func normalizeVersions(ctx android.BaseModuleContext, versions []string) { } func createVersionVariations(mctx android.BottomUpMutatorContext, versions []string) { - // "" is for the non-stubs variant - versions = append([]string{""}, versions...) + // "" is for the non-stubs (implementation) variant. + variants := append([]string{""}, versions...) - modules := mctx.CreateLocalVariations(versions...) + modules := mctx.CreateLocalVariations(variants...) for i, m := range modules { - if versions[i] != "" { + if variants[i] != "" { m.(LinkableInterface).SetBuildStubs() - m.(LinkableInterface).SetStubsVersions(versions[i]) + m.(LinkableInterface).SetStubsVersion(variants[i]) } } mctx.AliasVariation("") + latestVersion := "" + if len(versions) > 0 { + latestVersion = versions[len(versions)-1] + } + mctx.CreateAliasVariation("latest", latestVersion) } func VersionVariantAvailable(module interface { @@ -1577,44 +1564,41 @@ func VersionVariantAvailable(module interface { return !module.Host() && !module.InRamdisk() && !module.InRecovery() } -// VersionMutator splits a module into the mandatory non-stubs variant -// (which is unnamed) and zero or more stubs variants. -func VersionMutator(mctx android.BottomUpMutatorContext) { +// 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.CcLibrary() && library.BuildSharedVariant() && len(library.StubsVersions()) > 0 && !library.IsSdkVariant() { + versions := library.StubsVersions() normalizeVersions(mctx, versions) if mctx.Failed() { return } - - stubsVersionsLock.Lock() - defer stubsVersionsLock.Unlock() - // save the list of versions for later use - stubsVersionsFor(mctx.Config())[mctx.ModuleName()] = versions - - createVersionVariations(mctx, versions) + // Set the versions on the pre-mutated module so they can be read by any llndk modules that + // depend on the implementation library and haven't been mutated yet. + library.SetAllStubsVersions(versions) return } if c, ok := library.(*Module); ok && c.IsStubs() { - stubsVersionsLock.Lock() - defer stubsVersionsLock.Unlock() - // 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()] - // save the list of versions for later use - stubsVersionsFor(mctx.Config())[mctx.ModuleName()] = versions - - createVersionVariations(mctx, versions) - return + // Get the versions from the implementation module. + impls := mctx.GetDirectDepsWithTag(llndkImplDep) + if len(impls) > 1 { + panic(fmt.Errorf("Expected single implmenetation library, got %d", len(impls))) + } else if len(impls) == 1 { + c.SetAllStubsVersions(impls[0].(*Module).AllStubsVersions()) + } } + } +} - mctx.CreateLocalVariations("") - mctx.AliasVariation("") - return +// 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) { + createVersionVariations(mctx, library.AllStubsVersions()) } } diff --git a/cc/library_sdk_member.go b/cc/library_sdk_member.go index ecfdc9977..2f1554427 100644 --- a/cc/library_sdk_member.go +++ b/cc/library_sdk_member.go @@ -80,7 +80,7 @@ func (mt *librarySdkMemberType) AddDependencies(mctx android.BottomUpMutatorCont for _, target := range targets { name, version := StubsLibNameAndVersion(lib) if version == "" { - version = LatestStubsVersionFor(mctx.Config(), name) + version = "latest" } variations := target.Variations() if mctx.Device() { diff --git a/cc/linkable.go b/cc/linkable.go index 4c8416347..6d8a4b71e 100644 --- a/cc/linkable.go +++ b/cc/linkable.go @@ -26,8 +26,10 @@ type LinkableInterface interface { StubsVersions() []string BuildStubs() bool SetBuildStubs() - SetStubsVersions(string) + SetStubsVersion(string) StubsVersion() string + SetAllStubsVersions([]string) + AllStubsVersions() []string HasStubsVariants() bool SelectedStl() string ApiLevel() string diff --git a/rust/rust.go b/rust/rust.go index 4cba6d6b1..92be0f3ab 100644 --- a/rust/rust.go +++ b/rust/rust.go @@ -458,12 +458,20 @@ func (mod *Module) SetBuildStubs() { panic("SetBuildStubs not yet implemented for rust modules") } -func (mod *Module) SetStubsVersions(string) { - panic("SetStubsVersions not yet implemented for rust modules") +func (mod *Module) SetStubsVersion(string) { + panic("SetStubsVersion not yet implemented for rust modules") } func (mod *Module) StubsVersion() string { - panic("SetStubsVersions not yet implemented for rust modules") + panic("StubsVersion not yet implemented for rust modules") +} + +func (mod *Module) SetAllStubsVersions([]string) { + panic("SetAllStubsVersions not yet implemented for rust modules") +} + +func (mod *Module) AllStubsVersions() []string { + return nil } func (mod *Module) BuildStaticVariant() bool {