From f3bfd02aa9c6e3f3f9e1c65aefe0283235f2ce38 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 27 Sep 2021 15:15:06 -0700 Subject: [PATCH 1/3] Add environment variables to cc_genrule commands Pass the architecture, mulitlib type and native bridge state to each variant of a cc_genrule rule as environment variables. Bug: 200872604 Test: TestCmdPrefix Change-Id: I39c4c2d5bbd4f4cc72a4777715db1df049345b37 --- cc/genrule.go | 21 +++++++++++++- cc/genrule_test.go | 72 ++++++++++++++++++++++++++++++++++++++++++++++ genrule/genrule.go | 12 +++++++- 3 files changed, 103 insertions(+), 2 deletions(-) diff --git a/cc/genrule.go b/cc/genrule.go index 9df52280e..239064f1c 100644 --- a/cc/genrule.go +++ b/cc/genrule.go @@ -15,6 +15,8 @@ package cc import ( + "fmt" + "android/soong/android" "android/soong/genrule" "android/soong/snapshot" @@ -36,13 +38,23 @@ type GenruleExtraProperties struct { // cc_genrule is a genrule that can depend on other cc_* objects. // The cmd may be run multiple times, once for each of the different arch/etc -// variations. +// variations. The following environment variables will be set when the command +// execute: +// +// CC_ARCH the name of the architecture the command is being executed for +// +// CC_MULTILIB "lib32" if the architecture the command is being executed for is 32-bit, +// "lib64" if it is 64-bit. +// +// CC_NATIVE_BRIDGE the name of the subdirectory that native bridge libraries are stored in if +// the architecture has native bridge enabled, empty if it is disabled. func GenRuleFactory() android.Module { module := genrule.NewGenRule() extra := &GenruleExtraProperties{} module.Extra = extra module.ImageInterface = extra + module.CmdModifier = genruleCmdModifier module.AddProperties(module.Extra) android.InitAndroidArchModule(module, android.HostAndDeviceSupported, android.MultilibBoth) @@ -53,6 +65,13 @@ func GenRuleFactory() android.Module { return module } +func genruleCmdModifier(ctx android.ModuleContext, cmd string) string { + target := ctx.Target() + arch := target.Arch.ArchType + return fmt.Sprintf("CC_ARCH=%s CC_NATIVE_BRIDGE=%s CC_MULTILIB=%s && %s", + arch.Name, target.NativeBridgeRelativePath, arch.Multilib, cmd) +} + var _ android.ImageInterface = (*GenruleExtraProperties)(nil) func (g *GenruleExtraProperties) ImageMutatorBegin(ctx android.BaseModuleContext) {} diff --git a/cc/genrule_test.go b/cc/genrule_test.go index b6afb05a7..f25f70416 100644 --- a/cc/genrule_test.go +++ b/cc/genrule_test.go @@ -115,3 +115,75 @@ func TestLibraryGenruleCmd(t *testing.T) { t.Errorf(`want inputs %v, got %v`, expected, got) } } + +func TestCmdPrefix(t *testing.T) { + bp := ` + cc_genrule { + name: "gen", + cmd: "echo foo", + out: ["out"], + native_bridge_supported: true, + } + ` + + testCases := []struct { + name string + variant string + preparer android.FixturePreparer + + arch string + nativeBridge string + multilib string + }{ + { + name: "arm", + variant: "android_arm_armv7-a-neon", + arch: "arm", + multilib: "lib32", + }, + { + name: "arm64", + variant: "android_arm64_armv8-a", + arch: "arm64", + multilib: "lib64", + }, + { + name: "nativebridge", + variant: "android_native_bridge_arm_armv7-a-neon", + preparer: android.FixtureModifyConfig(func(config android.Config) { + config.Targets[android.Android] = []android.Target{ + { + Os: android.Android, + Arch: android.Arch{ArchType: android.X86, ArchVariant: "silvermont", Abi: []string{"armeabi-v7a"}}, + NativeBridge: android.NativeBridgeDisabled, + }, + { + Os: android.Android, + Arch: android.Arch{ArchType: android.Arm, ArchVariant: "armv7-a-neon", Abi: []string{"armeabi-v7a"}}, + NativeBridge: android.NativeBridgeEnabled, + NativeBridgeHostArchName: "x86", + NativeBridgeRelativePath: "arm", + }, + } + }), + arch: "arm", + multilib: "lib32", + nativeBridge: "arm", + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + result := android.GroupFixturePreparers( + PrepareForIntegrationTestWithCc, + android.OptionalFixturePreparer(tt.preparer), + ).RunTestWithBp(t, bp) + gen := result.ModuleForTests("gen", tt.variant) + sboxProto := android.RuleBuilderSboxProtoForTests(t, gen.Output("genrule.sbox.textproto")) + cmd := *sboxProto.Commands[0].Command + android.AssertStringDoesContain(t, "incorrect CC_ARCH", cmd, "CC_ARCH="+tt.arch+" ") + android.AssertStringDoesContain(t, "incorrect CC_NATIVE_BRIDGE", cmd, "CC_NATIVE_BRIDGE="+tt.nativeBridge+" ") + android.AssertStringDoesContain(t, "incorrect CC_MULTILIB", cmd, "CC_MULTILIB="+tt.multilib+" ") + }) + } +} diff --git a/genrule/genrule.go b/genrule/genrule.go index f4bde703a..96b610bac 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -156,6 +156,11 @@ type Module struct { // For other packages to make their own genrules with extra // properties Extra interface{} + + // CmdModifier can be set by wrappers around genrule to modify the command, for example to + // prefix environment variables to it. + CmdModifier func(ctx android.ModuleContext, cmd string) string + android.ImageInterface properties generatorProperties @@ -398,8 +403,13 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { var outputFiles android.WritablePaths var zipArgs strings.Builder + cmd := String(g.properties.Cmd) + if g.CmdModifier != nil { + cmd = g.CmdModifier(ctx, cmd) + } + // Generate tasks, either from genrule or gensrcs. - for _, task := range g.taskGenerator(ctx, String(g.properties.Cmd), srcFiles) { + for _, task := range g.taskGenerator(ctx, cmd, srcFiles) { if len(task.out) == 0 { ctx.ModuleErrorf("must have at least one output file") return From cfb0f5e102e2a4a946c4f6e38d429b132ed11fc5 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 24 Sep 2021 15:47:17 -0700 Subject: [PATCH 2/3] Support per-testcase directories in all test suites There are cases where two modules try to install the same test data into CTS, which results in collisions when CTS puts the data for all tests in the same directory. Add a flag that allows enabling a per-testcase directory for an individual test for all test suites. Bug: 193168159 Test: cts-tradefed run commandAndExit CtsBionicTestCases Change-Id: If034723e8fe937ca71d3e2d39b7d46702e41bc8c --- cc/androidmk.go | 2 ++ cc/test.go | 3 +++ java/androidmk.go | 13 +++++++------ java/app.go | 3 +++ java/java.go | 6 ++++++ 5 files changed, 21 insertions(+), 6 deletions(-) diff --git a/cc/androidmk.go b/cc/androidmk.go index cd52363e8..243efe60c 100644 --- a/cc/androidmk.go +++ b/cc/androidmk.go @@ -394,6 +394,8 @@ func (test *testBinary) AndroidMkEntries(ctx AndroidMkContext, entries *android. if Bool(test.Properties.Test_options.Unit_test) { entries.SetBool("LOCAL_IS_UNIT_TEST", true) } + + entries.SetBoolIfTrue("LOCAL_COMPATIBILITY_PER_TESTCASE_DIRECTORY", Bool(test.Properties.Per_testcase_directory)) }) AndroidMkWriteTestData(test.data, entries) diff --git a/cc/test.go b/cc/test.go index 047a69e3e..4328628d4 100644 --- a/cc/test.go +++ b/cc/test.go @@ -115,6 +115,9 @@ type TestBinaryProperties struct { // Add parameterized mainline modules to auto generated test config. The options will be // handled by TradeFed to download and install the specified modules on the device. Test_mainline_modules []string + + // Install the test into a folder named for the module in all test suites. + Per_testcase_directory *bool } func init() { diff --git a/java/androidmk.go b/java/androidmk.go index 1914595dc..0f1a1ec14 100644 --- a/java/androidmk.go +++ b/java/androidmk.go @@ -147,20 +147,21 @@ func (library *Library) AndroidMkEntries() []android.AndroidMkEntries { } // Called for modules that are a component of a test suite. -func testSuiteComponent(entries *android.AndroidMkEntries, test_suites []string) { +func testSuiteComponent(entries *android.AndroidMkEntries, test_suites []string, perTestcaseDirectory bool) { entries.SetString("LOCAL_MODULE_TAGS", "tests") if len(test_suites) > 0 { entries.AddCompatibilityTestSuites(test_suites...) } else { entries.AddCompatibilityTestSuites("null-suite") } + entries.SetBoolIfTrue("LOCAL_COMPATIBILITY_PER_TESTCASE_DIRECTORY", perTestcaseDirectory) } func (j *Test) AndroidMkEntries() []android.AndroidMkEntries { entriesList := j.Library.AndroidMkEntries() entries := &entriesList[0] entries.ExtraEntries = append(entries.ExtraEntries, func(ctx android.AndroidMkExtraEntriesContext, entries *android.AndroidMkEntries) { - testSuiteComponent(entries, j.testProperties.Test_suites) + testSuiteComponent(entries, j.testProperties.Test_suites, Bool(j.testProperties.Per_testcase_directory)) if j.testConfig != nil { entries.SetPath("LOCAL_FULL_TEST_CONFIG", j.testConfig) } @@ -188,7 +189,7 @@ func (j *TestHelperLibrary) AndroidMkEntries() []android.AndroidMkEntries { entriesList := j.Library.AndroidMkEntries() entries := &entriesList[0] entries.ExtraEntries = append(entries.ExtraEntries, func(ctx android.AndroidMkExtraEntriesContext, entries *android.AndroidMkEntries) { - testSuiteComponent(entries, j.testHelperLibraryProperties.Test_suites) + testSuiteComponent(entries, j.testHelperLibraryProperties.Test_suites, Bool(j.testHelperLibraryProperties.Per_testcase_directory)) }) return entriesList @@ -450,7 +451,7 @@ func (a *AndroidTest) AndroidMkEntries() []android.AndroidMkEntries { entriesList := a.AndroidApp.AndroidMkEntries() entries := &entriesList[0] entries.ExtraEntries = append(entries.ExtraEntries, func(ctx android.AndroidMkExtraEntriesContext, entries *android.AndroidMkEntries) { - testSuiteComponent(entries, a.testProperties.Test_suites) + testSuiteComponent(entries, a.testProperties.Test_suites, Bool(a.testProperties.Per_testcase_directory)) if a.testConfig != nil { entries.SetPath("LOCAL_FULL_TEST_CONFIG", a.testConfig) } @@ -466,7 +467,7 @@ func (a *AndroidTestHelperApp) AndroidMkEntries() []android.AndroidMkEntries { entriesList := a.AndroidApp.AndroidMkEntries() entries := &entriesList[0] entries.ExtraEntries = append(entries.ExtraEntries, func(ctx android.AndroidMkExtraEntriesContext, entries *android.AndroidMkEntries) { - testSuiteComponent(entries, a.appTestHelperAppProperties.Test_suites) + testSuiteComponent(entries, a.appTestHelperAppProperties.Test_suites, Bool(a.appTestHelperAppProperties.Per_testcase_directory)) // introduce a flag variable to control the generation of the .config file entries.SetString("LOCAL_DISABLE_TEST_CONFIG", "true") }) @@ -677,7 +678,7 @@ func (a *AndroidTestImport) AndroidMkEntries() []android.AndroidMkEntries { entriesList := a.AndroidAppImport.AndroidMkEntries() entries := &entriesList[0] entries.ExtraEntries = append(entries.ExtraEntries, func(ctx android.AndroidMkExtraEntriesContext, entries *android.AndroidMkEntries) { - testSuiteComponent(entries, a.testProperties.Test_suites) + testSuiteComponent(entries, a.testProperties.Test_suites, Bool(a.testProperties.Per_testcase_directory)) androidMkWriteTestData(a.data, entries) }) return entriesList diff --git a/java/app.go b/java/app.go index 2fd646322..bc0f488d0 100755 --- a/java/app.go +++ b/java/app.go @@ -1070,6 +1070,9 @@ type appTestHelperAppProperties struct { // doesn't exist next to the Android.bp, this attribute doesn't need to be set to true // explicitly. Auto_gen_config *bool + + // Install the test into a folder named for the module in all test suites. + Per_testcase_directory *bool } type AndroidTestHelperApp struct { diff --git a/java/java.go b/java/java.go index 94c12bdbb..655d51f94 100644 --- a/java/java.go +++ b/java/java.go @@ -749,6 +749,9 @@ type testProperties struct { // Names of modules containing JNI libraries that should be installed alongside the test. Jni_libs []string + + // Install the test into a folder named for the module in all test suites. + Per_testcase_directory *bool } type hostTestProperties struct { @@ -760,6 +763,9 @@ type testHelperLibraryProperties struct { // list of compatibility suites (for example "cts", "vts") that the module should be // installed into. Test_suites []string `android:"arch_variant"` + + // Install the test into a folder named for the module in all test suites. + Per_testcase_directory *bool } type prebuiltTestProperties struct { From c8caa06a36913b0633b14b9cdb46c6fc72eb6f41 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 24 Sep 2021 16:50:14 -0700 Subject: [PATCH 3/3] Add data_bins property data_bins is similar to data_libs but copies helper binaries alongside the test. Bug: 200872604 Test: atest CtsBionicTestCases Change-Id: I4f9df5f82816cfd30a0a19808fda220cf77c50a7 --- cc/cc.go | 4 ++++ cc/test.go | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/cc/cc.go b/cc/cc.go index 57e78876e..b410834be 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -95,6 +95,7 @@ type Deps struct { // Used for data dependencies adjacent to tests DataLibs []string + DataBins []string // Used by DepsMutator to pass system_shared_libs information to check_elf_file.py. SystemSharedLibs []string @@ -718,6 +719,7 @@ var ( staticVariantTag = dependencyTag{name: "static variant"} vndkExtDepTag = dependencyTag{name: "vndk extends"} dataLibDepTag = dependencyTag{name: "data lib"} + dataBinDepTag = dependencyTag{name: "data bin"} runtimeDepTag = installDependencyTag{name: "runtime lib"} testPerSrcDepTag = dependencyTag{name: "test_per_src"} stubImplDepTag = dependencyTag{name: "stub_impl"} @@ -2274,6 +2276,8 @@ func (c *Module) DepsMutator(actx android.BottomUpMutatorContext) { {Mutator: "link", Variation: "shared"}, }, dataLibDepTag, deps.DataLibs...) + actx.AddVariationDependencies(nil, dataBinDepTag, deps.DataBins...) + actx.AddVariationDependencies([]blueprint.Variation{ {Mutator: "link", Variation: "shared"}, }, runtimeDepTag, deps.RuntimeLibs...) diff --git a/cc/test.go b/cc/test.go index 4328628d4..c589165f0 100644 --- a/cc/test.go +++ b/cc/test.go @@ -80,6 +80,9 @@ type TestBinaryProperties struct { // list of shared library modules that should be installed alongside the test Data_libs []string `android:"arch_variant"` + // list of binary modules that should be installed alongside the test + Data_bins []string `android:"arch_variant"` + // list of compatibility suites (for example "cts", "vts") that the module should be // installed into. Test_suites []string `android:"arch_variant"` @@ -350,6 +353,7 @@ func (test *testBinary) linkerDeps(ctx DepsContext, deps Deps) Deps { deps = test.testDecorator.linkerDeps(ctx, deps) deps = test.binaryDecorator.linkerDeps(ctx, deps) deps.DataLibs = append(deps.DataLibs, test.Properties.Data_libs...) + deps.DataBins = append(deps.DataBins, test.Properties.Data_bins...) return deps } @@ -389,6 +393,18 @@ func (test *testBinary) install(ctx ModuleContext, file android.Path) { RelativeInstallPath: ccModule.installer.relativeInstallPath()}) } }) + ctx.VisitDirectDepsWithTag(dataBinDepTag, func(dep android.Module) { + depName := ctx.OtherModuleName(dep) + ccModule, ok := dep.(*Module) + if !ok { + ctx.ModuleErrorf("data_bin %q is not a cc module", depName) + } + if ccModule.OutputFile().Valid() { + test.data = append(test.data, + android.DataPath{SrcPath: ccModule.OutputFile().Path(), + RelativeInstallPath: ccModule.installer.relativeInstallPath()}) + } + }) var configs []tradefed.Config for _, module := range test.Properties.Test_mainline_modules {