From f89cd0949c573b86a12eeaf0a799f5faf8e1040a Mon Sep 17 00:00:00 2001 From: Roland Levillain Date: Mon, 29 Jul 2019 16:22:59 +0100 Subject: [PATCH] Handle `test_per_src` modules as indirect dependencies in APEXes. In `apex.apexBundle.GenerateAndroidBuildActions`, we used to pass the "all tests" ("") module as `module` for all `apexFile` objects created from a test module using `test_per_src: true`. An immediate issue of this situation was that the "" module is hidden from Make, which made all the generated `apexFile` objects hidden from Make too. This would break the construction of flattened APEXes, as they rely on Make logic to install their files. Instead of collecting `test_per_src` test variations' output files in `cc.Module.GenerateAndroidBuildActions` and using them in `apex.apexBundle.GenerateAndroidBuildActions` as part of handling the "" variation as a direct dependency of an `apexBundle`, process them as indirect dependencies (and do nothing for the "" variation direct dependency). Adjust the indirect dependency logic in `apex.apexBundle.GenerateAndroidBuildActions` to allow not only shared/runtime native libraries as indirect dependencies of an `apexBundle`, but also `test_per_src` tests. Test: m (`apex/apex_test.go` amended) Bug: 129534335 Change-Id: I845e0f0dd3a98d61d0b7118c5eaf61f3e5335724 --- apex/apex.go | 97 +++++++++++++++++++++++++---------------------- apex/apex_test.go | 18 ++++++++- cc/cc.go | 67 ++++++++++++++++---------------- 3 files changed, 104 insertions(+), 78 deletions(-) diff --git a/apex/apex.go b/apex/apex.go index 0466ccbe4..34e09c09e 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -694,12 +694,6 @@ func getCopyManifestForExecutable(cc *cc.Module) (fileToCopy android.Path, dirIn return } -func getCopyManifestForTestPerSrcExecutables(cc *cc.Module) (filesToCopy []android.Path, dirInApex string) { - dirInApex = filepath.Join("bin", cc.RelativeInstallPath()) - filesToCopy = cc.TestPerSrcOutputFiles() - return -} - func getCopyManifestForPyBinary(py *python.Module) (fileToCopy android.Path, dirInApex string) { dirInApex = "bin" fileToCopy = py.HostToolPath().Path() @@ -780,10 +774,10 @@ func (a *apexBundle) GenerateAndroidBuildActions(ctx android.ModuleContext) { }) ctx.WalkDepsBlueprint(func(child, parent blueprint.Module) bool { + depTag := ctx.OtherModuleDependencyTag(child) + depName := ctx.OtherModuleName(child) if _, ok := parent.(*apexBundle); ok { // direct dependencies - depTag := ctx.OtherModuleDependencyTag(child) - depName := ctx.OtherModuleName(child) switch depTag { case sharedLibTag: if cc, ok := child.(*cc.Module); ok { @@ -834,22 +828,19 @@ func (a *apexBundle) GenerateAndroidBuildActions(ctx android.ModuleContext) { ctx.PropertyErrorf("prebuilts", "%q is not a prebuilt_etc module", depName) } case testTag: - if cc, ok := child.(*cc.Module); ok { - if cc.TestPerSrcOutputFiles() != nil { - // Multiple-output test module (using `test_per_src`). - filesToCopy, dirInApex := getCopyManifestForTestPerSrcExecutables(cc) - for _, fileToCopy := range filesToCopy { - // Handle modules created as `test_per_src` variations of a single test module: - // replace the name of the original test module (`depName`, shared by all - // `test_per_src` variants of that module) with the name of the generated test - // binary. - moduleName := filepath.Base(fileToCopy.String()) - filesInfo = append(filesInfo, apexFile{fileToCopy, moduleName, dirInApex, nativeTest, cc, nil}) - } + if ccTest, ok := child.(*cc.Module); ok { + if ccTest.IsTestPerSrcAllTestsVariation() { + // Multiple-output test module (where `test_per_src: true`). + // + // `ccTest` is the "" ("all tests") variation of a `test_per_src` module. + // We do not add this variation to `filesInfo`, as it has no output; + // however, we do add the other variations of this module as indirect + // dependencies (see below). + return true } else { - // Single-output test module (not using `test_per_src`). - fileToCopy, dirInApex := getCopyManifestForExecutable(cc) - filesInfo = append(filesInfo, apexFile{fileToCopy, depName, dirInApex, nativeTest, cc, nil}) + // Single-output test module (where `test_per_src: false`). + fileToCopy, dirInApex := getCopyManifestForExecutable(ccTest) + filesInfo = append(filesInfo, apexFile{fileToCopy, depName, dirInApex, nativeTest, ccTest, nil}) } return true } else { @@ -875,30 +866,46 @@ func (a *apexBundle) GenerateAndroidBuildActions(ctx android.ModuleContext) { } else { // indirect dependencies if am, ok := child.(android.ApexModule); ok && am.CanHaveApexVariants() && am.IsInstallableToApex() { - if cc, ok := child.(*cc.Module); ok { - if android.InList(cc.Name(), providedNativeSharedLibs) { - // If we're using a shared library which is provided from other APEX, - // don't include it in this APEX - return false - } - if !a.Host() && (cc.IsStubs() || cc.HasStubsVariants()) { - // If the dependency is a stubs lib, don't include it in this APEX, - // but make sure that the lib is installed on the device. - // In case no APEX is having the lib, the lib is installed to the system - // partition. - // - // Always include if we are a host-apex however since those won't have any - // system libraries. - if !android.DirectlyInAnyApex(ctx, cc.Name()) && !android.InList(cc.Name(), a.externalDeps) { - a.externalDeps = append(a.externalDeps, cc.Name()) + // We cannot use a switch statement on `depTag` here as the checked + // tags used below are private (e.g. `cc.sharedDepTag`). + if cc.IsSharedDepTag(depTag) || cc.IsRuntimeDepTag(depTag) { + if cc, ok := child.(*cc.Module); ok { + if android.InList(cc.Name(), providedNativeSharedLibs) { + // If we're using a shared library which is provided from other APEX, + // don't include it in this APEX + return false } - // Don't track further - return false + if !a.Host() && (cc.IsStubs() || cc.HasStubsVariants()) { + // If the dependency is a stubs lib, don't include it in this APEX, + // but make sure that the lib is installed on the device. + // In case no APEX is having the lib, the lib is installed to the system + // partition. + // + // Always include if we are a host-apex however since those won't have any + // system libraries. + if !android.DirectlyInAnyApex(ctx, cc.Name()) && !android.InList(cc.Name(), a.externalDeps) { + a.externalDeps = append(a.externalDeps, cc.Name()) + } + // Don't track further + return false + } + fileToCopy, dirInApex := getCopyManifestForNativeLibrary(cc, handleSpecialLibs) + filesInfo = append(filesInfo, apexFile{fileToCopy, depName, dirInApex, nativeSharedLib, cc, nil}) + return true } - depName := ctx.OtherModuleName(child) - fileToCopy, dirInApex := getCopyManifestForNativeLibrary(cc, handleSpecialLibs) - filesInfo = append(filesInfo, apexFile{fileToCopy, depName, dirInApex, nativeSharedLib, cc, nil}) - return true + } else if cc.IsTestPerSrcDepTag(depTag) { + if cc, ok := child.(*cc.Module); ok { + fileToCopy, dirInApex := getCopyManifestForExecutable(cc) + // Handle modules created as `test_per_src` variations of a single test module: + // use the name of the generated test binary (`fileToCopy`) instead of the name + // of the original test module (`depName`, shared by all `test_per_src` + // variations of that module). + moduleName := filepath.Base(fileToCopy.String()) + filesInfo = append(filesInfo, apexFile{fileToCopy, moduleName, dirInApex, nativeTest, cc, nil}) + return true + } + } else { + ctx.ModuleErrorf("unexpected tag %q for indirect dependency %q", depTag, depName) } } } diff --git a/apex/apex_test.go b/apex/apex_test.go index e44163e97..cecdaaf39 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -1393,7 +1393,7 @@ func TestPrebuiltOverrides(t *testing.T) { } func TestApexWithTests(t *testing.T) { - ctx, _ := testApex(t, ` + ctx, config := testApex(t, ` apex_test { name: "myapex", key: "myapex.key", @@ -1445,6 +1445,22 @@ func TestApexWithTests(t *testing.T) { ensureContains(t, copyCmds, "image.apex/bin/test/mytest1") ensureContains(t, copyCmds, "image.apex/bin/test/mytest2") ensureContains(t, copyCmds, "image.apex/bin/test/mytest3") + + // Ensure the module is correctly translated. + apexBundle := ctx.ModuleForTests("myapex", "android_common_myapex").Module().(*apexBundle) + data := android.AndroidMkDataForTest(t, config, "", apexBundle) + name := apexBundle.BaseModuleName() + prefix := "TARGET_" + var builder strings.Builder + data.Custom(&builder, name, prefix, "", data) + androidMk := builder.String() + ensureContains(t, androidMk, "LOCAL_MODULE := myapex.mytest\n") + ensureContains(t, androidMk, "LOCAL_MODULE := myapex.mytest1\n") + ensureContains(t, androidMk, "LOCAL_MODULE := myapex.mytest2\n") + ensureContains(t, androidMk, "LOCAL_MODULE := myapex.mytest3\n") + ensureContains(t, androidMk, "LOCAL_MODULE := myapex.apex_manifest.json\n") + ensureContains(t, androidMk, "LOCAL_MODULE := myapex.apex_pubkey\n") + ensureContains(t, androidMk, "LOCAL_MODULE := myapex\n") } func TestApexUsesOtherApex(t *testing.T) { diff --git a/cc/cc.go b/cc/cc.go index 2cee80717..e31eb2d46 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -340,6 +340,7 @@ type dependencyTag struct { blueprint.BaseDependencyTag name string library bool + shared bool reexportFlags bool @@ -347,10 +348,10 @@ type dependencyTag struct { } var ( - sharedDepTag = dependencyTag{name: "shared", library: true} - sharedExportDepTag = dependencyTag{name: "shared", library: true, reexportFlags: true} - earlySharedDepTag = dependencyTag{name: "early_shared", library: true} - lateSharedDepTag = dependencyTag{name: "late shared", library: true} + sharedDepTag = dependencyTag{name: "shared", library: true, shared: true} + sharedExportDepTag = dependencyTag{name: "shared", library: true, shared: true, reexportFlags: true} + earlySharedDepTag = dependencyTag{name: "early_shared", library: true, shared: true} + lateSharedDepTag = dependencyTag{name: "late shared", library: true, shared: true} staticDepTag = dependencyTag{name: "static", library: true} staticExportDepTag = dependencyTag{name: "static", library: true, reexportFlags: true} lateStaticDepTag = dependencyTag{name: "late static", library: true} @@ -375,6 +376,21 @@ var ( testPerSrcDepTag = dependencyTag{name: "test_per_src"} ) +func IsSharedDepTag(depTag blueprint.DependencyTag) bool { + ccDepTag, ok := depTag.(dependencyTag) + return ok && ccDepTag.shared +} + +func IsRuntimeDepTag(depTag blueprint.DependencyTag) bool { + ccDepTag, ok := depTag.(dependencyTag) + return ok && ccDepTag == runtimeDepTag +} + +func IsTestPerSrcDepTag(depTag blueprint.DependencyTag) bool { + ccDepTag, ok := depTag.(dependencyTag) + return ok && ccDepTag == testPerSrcDepTag +} + // Module contains the properties and members used by all C/C++ module types, and implements // the blueprint.Module interface. It delegates to compiler, linker, and installer interfaces // to construct the output file. Behavior can be customized with a Customizer interface @@ -408,9 +424,6 @@ type Module struct { outputFile android.OptionalPath - // Test output files, in the case of a test module using `test_per_src`. - testPerSrcOutputFiles []android.Path - cachedToolchain config.Toolchain subAndroidMkOnce map[subAndroidMkProvider]bool @@ -433,10 +446,6 @@ func (c *Module) OutputFile() android.OptionalPath { return c.outputFile } -func (c *Module) TestPerSrcOutputFiles() []android.Path { - return c.testPerSrcOutputFiles -} - func (c *Module) UnstrippedOutputFile() android.Path { if c.linker != nil { return c.linker.unstrippedOutputFilePath() @@ -950,28 +959,20 @@ func orderStaticModuleDeps(module *Module, staticDeps []*Module, sharedDeps []*M return results } +func (c *Module) IsTestPerSrcAllTestsVariation() bool { + test, ok := c.linker.(testPerSrc) + return ok && test.isAllTestsVariation() +} + func (c *Module) GenerateAndroidBuildActions(actx android.ModuleContext) { // Handle the case of a test module split by `test_per_src` mutator. - if test, ok := c.linker.(testPerSrc); ok { - // The `test_per_src` mutator adds an extra variant named "", depending on all the - // other `test_per_src` variants of the test module. Collect the output files of - // these dependencies and record them in the `testPerSrcOutputFiles` for later use - // (see e.g. `apexBundle.GenerateAndroidBuildActions`). - if test.isAllTestsVariation() { - var testPerSrcOutputFiles []android.Path - for _, dep := range actx.GetDirectDepsWithTag(testPerSrcDepTag) { - if ccDep, ok := dep.(*Module); ok { - depOutputFile := ccDep.OutputFile().Path() - testPerSrcOutputFiles = - append(testPerSrcOutputFiles, depOutputFile) - } - } - c.testPerSrcOutputFiles = testPerSrcOutputFiles - // Set outputFile to an empty path, as this module does not produce an - // output file per se. - c.outputFile = android.OptionalPath{} - return - } + // + // The `test_per_src` mutator adds an extra variation named "", depending on all the other + // `test_per_src` variations of the test module. Set `outputFile` to an empty path for this + // module and return early, as this module does not produce an output file per se. + if c.IsTestPerSrcAllTestsVariation() { + c.outputFile = android.OptionalPath{} + return } c.makeLinkType = c.getMakeLinkType(actx) @@ -2050,12 +2051,14 @@ func (c *Module) getMakeLinkType(actx android.ModuleContext) string { } // Overrides ApexModule.IsInstallabeToApex() -// Only shared libraries are installable to APEX. +// Only shared/runtime libraries and "test_per_src" tests are installable to APEX. func (c *Module) IsInstallableToApex() bool { if shared, ok := c.linker.(interface { shared() bool }); ok { return shared.shared() + } else if _, ok := c.linker.(testPerSrc); ok { + return true } return false }