From 1ec1255476c46623a6973780793a2497b163ab1a Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 24 Aug 2020 18:04:09 -0700 Subject: [PATCH 1/2] Create os and arch variants for GoBinaryTool modules AddFarVariationDependencies was broken, which allowed modules to add dependencies on GoBinaryTool modules using os and arch variations even though GoBinaryTool modules did not have any variations. Mutate GoBinaryTool modules in the os and arch mutators so that they have the expected os and arch variations. Test: all soong tests Change-Id: Ida7bc75a51ab1d2d38a6be11f574399097380cc9 --- android/arch.go | 29 ++++++++++++++++++++++++----- android/mutator.go | 28 ++++++++++++++++++++-------- 2 files changed, 44 insertions(+), 13 deletions(-) diff --git a/android/arch.go b/android/arch.go index 9a5461474..2ddb3f9fd 100644 --- a/android/arch.go +++ b/android/arch.go @@ -23,6 +23,7 @@ import ( "strings" "github.com/google/blueprint" + "github.com/google/blueprint/bootstrap" "github.com/google/blueprint/proptools" ) @@ -689,15 +690,24 @@ func (target Target) Variations() []blueprint.Variation { } } -func osMutator(mctx BottomUpMutatorContext) { +func osMutator(bpctx blueprint.BottomUpMutatorContext) { var module Module var ok bool - if module, ok = mctx.Module().(Module); !ok { + if module, ok = bpctx.Module().(Module); !ok { + if _, ok := bpctx.Module().(bootstrap.GoBinaryTool); ok { + // Go tools are always build OS tools. + bpctx.CreateVariations(bpctx.Config().(Config).BuildOSTarget.OsVariation()) + } return } base := module.base() + // GoBinaryTool support above requires this mutator to be a blueprint.BottomUpMutatorContext + // because android.BottomUpMutatorContext filters out non-Soong modules. Now that we've + // handled them, create a normal android.BottomUpMutatorContext. + mctx := bottomUpMutatorContextFactory(bpctx, module, false) + if !base.ArchSpecific() { return } @@ -819,15 +829,24 @@ func GetOsSpecificVariantsOfCommonOSVariant(mctx BaseModuleContext) []Module { // // Modules can be initialized with InitAndroidMultiTargetsArchModule, in which case they will be split by OsClass, // but will have a common Target that is expected to handle all other selected Targets via ctx.MultiTargets(). -func archMutator(mctx BottomUpMutatorContext) { +func archMutator(bpctx blueprint.BottomUpMutatorContext) { var module Module var ok bool - if module, ok = mctx.Module().(Module); !ok { + if module, ok = bpctx.Module().(Module); !ok { + if _, ok := bpctx.Module().(bootstrap.GoBinaryTool); ok { + // Go tools are always build OS tools. + bpctx.CreateVariations(bpctx.Config().(Config).BuildOSTarget.ArchVariation()) + } return } base := module.base() + // GoBinaryTool support above requires this mutator to be a blueprint.BottomUpMutatorContext + // because android.BottomUpMutatorContext filters out non-Soong modules. Now that we've + // handled them, create a normal android.BottomUpMutatorContext. + mctx := bottomUpMutatorContextFactory(bpctx, module, false) + if !base.ArchSpecific() { return } @@ -903,7 +922,7 @@ func archMutator(mctx BottomUpMutatorContext) { modules := mctx.CreateVariations(targetNames...) for i, m := range modules { addTargetProperties(m, targets[i], multiTargets, i == 0) - m.(Module).base().setArchProperties(mctx) + m.base().setArchProperties(mctx) } } diff --git a/android/mutator.go b/android/mutator.go index 40e61deb2..521255328 100644 --- a/android/mutator.go +++ b/android/mutator.go @@ -75,6 +75,7 @@ type registerMutatorsContext struct { type RegisterMutatorsContext interface { TopDown(name string, m TopDownMutator) MutatorHandle BottomUp(name string, m BottomUpMutator) MutatorHandle + BottomUpBlueprint(name string, m blueprint.BottomUpMutator) MutatorHandle } type RegisterMutatorFunc func(RegisterMutatorsContext) @@ -143,9 +144,9 @@ var preArch = []RegisterMutatorFunc{ } func registerArchMutator(ctx RegisterMutatorsContext) { - ctx.BottomUp("os", osMutator).Parallel() + ctx.BottomUpBlueprint("os", osMutator).Parallel() ctx.BottomUp("image", imageMutator).Parallel() - ctx.BottomUp("arch", archMutator).Parallel() + ctx.BottomUpBlueprint("arch", archMutator).Parallel() } var preDeps = []RegisterMutatorFunc{ @@ -225,16 +226,21 @@ type bottomUpMutatorContext struct { finalPhase bool } +func bottomUpMutatorContextFactory(ctx blueprint.BottomUpMutatorContext, a Module, + finalPhase bool) BottomUpMutatorContext { + + return &bottomUpMutatorContext{ + bp: ctx, + baseModuleContext: a.base().baseModuleContextFactory(ctx), + finalPhase: finalPhase, + } +} + func (x *registerMutatorsContext) BottomUp(name string, m BottomUpMutator) MutatorHandle { finalPhase := x.finalPhase f := func(ctx blueprint.BottomUpMutatorContext) { if a, ok := ctx.Module().(Module); ok { - actx := &bottomUpMutatorContext{ - bp: ctx, - baseModuleContext: a.base().baseModuleContextFactory(ctx), - finalPhase: finalPhase, - } - m(actx) + m(bottomUpMutatorContextFactory(ctx, a, finalPhase)) } } mutator := &mutator{name: name, bottomUpMutator: f} @@ -242,6 +248,12 @@ func (x *registerMutatorsContext) BottomUp(name string, m BottomUpMutator) Mutat return mutator } +func (x *registerMutatorsContext) BottomUpBlueprint(name string, m blueprint.BottomUpMutator) MutatorHandle { + mutator := &mutator{name: name, bottomUpMutator: m} + x.mutators = append(x.mutators, mutator) + return mutator +} + func (x *registerMutatorsContext) TopDown(name string, m TopDownMutator) MutatorHandle { f := func(ctx blueprint.TopDownMutatorContext) { if a, ok := ctx.Module().(Module); ok { From 42507337e52b14dc3fd50b20b61af32248273d73 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 21 Aug 2020 16:15:23 -0700 Subject: [PATCH 2/2] Only request image and version variations for device SDK dependencies AddFarVariationDependencies was broken, which allowed sdk modules to request dependencies using image and version variations, even for host modules that do not have image or version variations. Make the image and version variations conditional on device sdk modules. Test: go test ./sdk Change-Id: I59b7a32a3782254fd5feb828a5258ee13d4db812 --- apex/apex.go | 38 +++++++++++++++++++++----------------- cc/binary_sdk_member.go | 10 +++++++--- cc/library_sdk_member.go | 19 ++++++++++--------- cc/sanitize.go | 20 ++++++++++++-------- 4 files changed, 50 insertions(+), 37 deletions(-) diff --git a/apex/apex.go b/apex/apex.go index 84a1e7526..3c4ad1958 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -1321,26 +1321,30 @@ func addDependenciesForNativeModules(ctx android.BottomUpMutatorContext, // conflicting variations with this module. This is required since // arch variant of an APEX bundle is 'common' but it is 'arm' or 'arm64' // for native shared libs. - ctx.AddFarVariationDependencies(append(target.Variations(), []blueprint.Variation{ - {Mutator: "image", Variation: imageVariation}, - {Mutator: "link", Variation: "shared"}, - {Mutator: "version", Variation: ""}, // "" is the non-stub variant - }...), sharedLibTag, nativeModules.Native_shared_libs...) - ctx.AddFarVariationDependencies(append(target.Variations(), []blueprint.Variation{ - {Mutator: "image", Variation: imageVariation}, - {Mutator: "link", Variation: "shared"}, - {Mutator: "version", Variation: ""}, // "" is the non-stub variant - }...), jniLibTag, nativeModules.Jni_libs...) + binVariations := target.Variations() + libVariations := append(target.Variations(), + blueprint.Variation{Mutator: "link", Variation: "shared"}) + testVariations := append(target.Variations(), + blueprint.Variation{Mutator: "test_per_src", Variation: ""}) // "" is the all-tests variant - ctx.AddFarVariationDependencies(append(target.Variations(), - blueprint.Variation{Mutator: "image", Variation: imageVariation}), - executableTag, nativeModules.Binaries...) + if ctx.Device() { + binVariations = append(binVariations, + blueprint.Variation{Mutator: "image", Variation: imageVariation}) + libVariations = append(libVariations, + blueprint.Variation{Mutator: "image", Variation: imageVariation}, + blueprint.Variation{Mutator: "version", Variation: ""}) // "" is the non-stub variant + testVariations = append(testVariations, + blueprint.Variation{Mutator: "image", Variation: imageVariation}) + } - ctx.AddFarVariationDependencies(append(target.Variations(), []blueprint.Variation{ - {Mutator: "image", Variation: imageVariation}, - {Mutator: "test_per_src", Variation: ""}, // "" is the all-tests variant - }...), testTag, nativeModules.Tests...) + ctx.AddFarVariationDependencies(libVariations, sharedLibTag, nativeModules.Native_shared_libs...) + + ctx.AddFarVariationDependencies(libVariations, jniLibTag, nativeModules.Jni_libs...) + + ctx.AddFarVariationDependencies(binVariations, executableTag, nativeModules.Binaries...) + + ctx.AddFarVariationDependencies(testVariations, testTag, nativeModules.Tests...) } func (a *apexBundle) combineProperties(ctx android.BottomUpMutatorContext) { diff --git a/cc/binary_sdk_member.go b/cc/binary_sdk_member.go index 337de55b3..a1abc728a 100644 --- a/cc/binary_sdk_member.go +++ b/cc/binary_sdk_member.go @@ -46,9 +46,13 @@ func (mt *binarySdkMemberType) AddDependencies(mctx android.BottomUpMutatorConte if version == "" { version = LatestStubsVersionFor(mctx.Config(), name) } - mctx.AddFarVariationDependencies(append(target.Variations(), []blueprint.Variation{ - {Mutator: "version", Variation: version}, - }...), dependencyTag, name) + variations := target.Variations() + if mctx.Device() { + variations = append(variations, + blueprint.Variation{Mutator: "image", Variation: android.CoreVariation}, + blueprint.Variation{Mutator: "version", Variation: version}) + } + mctx.AddFarVariationDependencies(variations, dependencyTag, name) } } } diff --git a/cc/library_sdk_member.go b/cc/library_sdk_member.go index cff00b668..c975dd1e2 100644 --- a/cc/library_sdk_member.go +++ b/cc/library_sdk_member.go @@ -82,18 +82,19 @@ func (mt *librarySdkMemberType) AddDependencies(mctx android.BottomUpMutatorCont if version == "" { version = LatestStubsVersionFor(mctx.Config(), name) } + variations := target.Variations() + if mctx.Device() { + variations = append(variations, + blueprint.Variation{Mutator: "image", Variation: android.CoreVariation}, + blueprint.Variation{Mutator: "version", Variation: version}) + } if mt.linkTypes == nil { - mctx.AddFarVariationDependencies(append(target.Variations(), []blueprint.Variation{ - {Mutator: "image", Variation: android.CoreVariation}, - {Mutator: "version", Variation: version}, - }...), dependencyTag, name) + mctx.AddFarVariationDependencies(variations, dependencyTag, name) } else { for _, linkType := range mt.linkTypes { - mctx.AddFarVariationDependencies(append(target.Variations(), []blueprint.Variation{ - {Mutator: "image", Variation: android.CoreVariation}, - {Mutator: "link", Variation: linkType}, - {Mutator: "version", Variation: version}, - }...), dependencyTag, name) + libVariations := append(variations, + blueprint.Variation{Mutator: "link", Variation: linkType}) + mctx.AddFarVariationDependencies(libVariations, dependencyTag, name) } } } diff --git a/cc/sanitize.go b/cc/sanitize.go index 2243082ad..9bd416fb7 100644 --- a/cc/sanitize.go +++ b/cc/sanitize.go @@ -1011,10 +1011,12 @@ func sanitizerRuntimeMutator(mctx android.BottomUpMutatorContext) { // static executable gets static runtime libs depTag := libraryDependencyTag{Kind: staticLibraryDependency} - mctx.AddFarVariationDependencies(append(mctx.Target().Variations(), []blueprint.Variation{ - {Mutator: "link", Variation: "static"}, - c.ImageVariation(), - }...), depTag, deps...) + variations := append(mctx.Target().Variations(), + blueprint.Variation{Mutator: "link", Variation: "static"}) + if c.Device() { + variations = append(variations, c.ImageVariation()) + } + mctx.AddFarVariationDependencies(variations, depTag, deps...) } else if !c.static() && !c.header() { // If we're using snapshots and in vendor, redirect to snapshot whenever possible if c.VndkVersion() == mctx.DeviceConfig().VndkVersion() { @@ -1026,10 +1028,12 @@ func sanitizerRuntimeMutator(mctx android.BottomUpMutatorContext) { // dynamic executable and shared libs get shared runtime libs depTag := libraryDependencyTag{Kind: sharedLibraryDependency, Order: earlyLibraryDependency} - mctx.AddFarVariationDependencies(append(mctx.Target().Variations(), []blueprint.Variation{ - {Mutator: "link", Variation: "shared"}, - c.ImageVariation(), - }...), depTag, runtimeLibrary) + variations := append(mctx.Target().Variations(), + blueprint.Variation{Mutator: "link", Variation: "shared"}) + if c.Device() { + variations = append(variations, c.ImageVariation()) + } + mctx.AddFarVariationDependencies(variations, depTag, runtimeLibrary) } // static lib does not have dependency to the runtime library. The // dependency will be added to the executables or shared libs using