From 8d82ac58b4faf7427765648ab6ecc5c76baa5283 Mon Sep 17 00:00:00 2001 From: Yu Liu Date: Tue, 17 May 2022 15:13:28 -0700 Subject: [PATCH] Support cc code coverage for mixed build Bug: 231322627 Test: Manual tests and unit tests Change-Id: I786042af0d612192c54c3572f63a86a47174a242 --- android/bazel_handler.go | 30 +++++++++++--- android/bazel_handler_test.go | 57 ++++++++++++++++++++++++-- android/util.go | 8 +++- bazel/aquery.go | 3 ++ bp2build/cc_library_conversion_test.go | 5 ++- cc/bp2build.go | 9 +++- cmd/soong_build/main.go | 2 +- 7 files changed, 101 insertions(+), 13 deletions(-) diff --git a/android/bazel_handler.go b/android/bazel_handler.go index 4d9423a3c..4dae6f6d9 100644 --- a/android/bazel_handler.go +++ b/android/bazel_handler.go @@ -111,7 +111,7 @@ type BazelContext interface { // Issues commands to Bazel to receive results for all cquery requests // queued in the BazelContext. - InvokeBazel() error + InvokeBazel(config Config) error // Returns true if bazel is enabled for the given configuration. BazelEnabled() bool @@ -191,7 +191,7 @@ func (m MockBazelContext) GetPythonBinary(label string, cfgKey configKey) (strin return result, nil } -func (m MockBazelContext) InvokeBazel() error { +func (m MockBazelContext) InvokeBazel(config Config) error { panic("unimplemented") } @@ -261,7 +261,7 @@ func (n noopBazelContext) GetPythonBinary(label string, cfgKey configKey) (strin panic("unimplemented") } -func (n noopBazelContext) InvokeBazel() error { +func (n noopBazelContext) InvokeBazel(config Config) error { panic("unimplemented") } @@ -361,6 +361,7 @@ type bazelCommand struct { type mockBazelRunner struct { bazelCommandResults map[bazelCommand]string commands []bazelCommand + extraFlags []string } func (r *mockBazelRunner) issueBazelCommand(paths *bazelPaths, @@ -368,6 +369,7 @@ func (r *mockBazelRunner) issueBazelCommand(paths *bazelPaths, command bazelCommand, extraFlags ...string) (string, string, error) { r.commands = append(r.commands, command) + r.extraFlags = append(r.extraFlags, strings.Join(extraFlags, " ")) if ret, ok := r.bazelCommandResults[command]; ok { return ret, "", nil } @@ -676,7 +678,7 @@ func (p *bazelPaths) outDir() string { // Issues commands to Bazel to receive results for all cquery requests // queued in the BazelContext. -func (context *bazelContext) InvokeBazel() error { +func (context *bazelContext) InvokeBazel(config Config) error { context.results = make(map[cqueryKey]string) var cqueryOutput string @@ -759,15 +761,31 @@ func (context *bazelContext) InvokeBazel() error { // Issue an aquery command to retrieve action information about the bazel build tree. // - // TODO(cparsons): Use --target_pattern_file to avoid command line limits. var aqueryOutput string + var coverageFlags []string + if Bool(config.productVariables.ClangCoverage) { + coverageFlags = append(coverageFlags, "--collect_code_coverage") + if len(config.productVariables.NativeCoveragePaths) > 0 || + len(config.productVariables.NativeCoverageExcludePaths) > 0 { + includePaths := JoinWithPrefixAndSeparator(config.productVariables.NativeCoveragePaths, "+", ",") + excludePaths := JoinWithPrefixAndSeparator(config.productVariables.NativeCoverageExcludePaths, "-", ",") + if len(includePaths) > 0 && len(excludePaths) > 0 { + includePaths += "," + } + coverageFlags = append(coverageFlags, fmt.Sprintf(`--instrumentation_filter=%s`, + includePaths+excludePaths)) + } + } + + extraFlags := append([]string{"--output=jsonproto"}, coverageFlags...) + aqueryOutput, _, err = context.issueBazelCommand( context.paths, bazel.AqueryBuildRootRunName, bazelCommand{"aquery", fmt.Sprintf("deps(%s)", buildrootLabel)}, // Use jsonproto instead of proto; actual proto parsing would require a dependency on Bazel's // proto sources, which would add a number of unnecessary dependencies. - "--output=jsonproto") + extraFlags...) if err != nil { return err diff --git a/android/bazel_handler_test.go b/android/bazel_handler_test.go index cfdccd7c3..dd9a7ed6d 100644 --- a/android/bazel_handler_test.go +++ b/android/bazel_handler_test.go @@ -4,11 +4,14 @@ import ( "os" "path/filepath" "reflect" + "strings" "testing" "android/soong/bazel/cquery" ) +var testConfig = TestConfig("out", nil, "", nil) + func TestRequestResultsAfterInvokeBazel(t *testing.T) { label := "//foo:bar" cfg := configKey{"arm64_armv8-a", Android} @@ -16,7 +19,7 @@ func TestRequestResultsAfterInvokeBazel(t *testing.T) { bazelCommand{command: "cquery", expression: "deps(@soong_injection//mixed_builds:buildroot, 2)"}: `//foo:bar|arm64_armv8-a|android>>out/foo/bar.txt`, }) bazelContext.QueueBazelRequest(label, cquery.GetOutputFiles, cfg) - err := bazelContext.InvokeBazel() + err := bazelContext.InvokeBazel(testConfig) if err != nil { t.Fatalf("Did not expect error invoking Bazel, but got %s", err) } @@ -30,7 +33,7 @@ func TestRequestResultsAfterInvokeBazel(t *testing.T) { func TestInvokeBazelWritesBazelFiles(t *testing.T) { bazelContext, baseDir := testBazelContext(t, map[bazelCommand]string{}) - err := bazelContext.InvokeBazel() + err := bazelContext.InvokeBazel(testConfig) if err != nil { t.Fatalf("Did not expect error invoking Bazel, but got %s", err) } @@ -86,7 +89,7 @@ func TestInvokeBazelPopulatesBuildStatements(t *testing.T) { }] }`, }) - err := bazelContext.InvokeBazel() + err := bazelContext.InvokeBazel(testConfig) if err != nil { t.Fatalf("Did not expect error invoking Bazel, but got %s", err) } @@ -97,6 +100,54 @@ func TestInvokeBazelPopulatesBuildStatements(t *testing.T) { } } +func TestCoverageFlagsAfterInvokeBazel(t *testing.T) { + testConfig.productVariables.ClangCoverage = boolPtr(true) + + testConfig.productVariables.NativeCoveragePaths = []string{"foo1", "foo2"} + testConfig.productVariables.NativeCoverageExcludePaths = []string{"bar1", "bar2"} + verifyExtraFlags(t, testConfig, `--collect_code_coverage --instrumentation_filter=+foo1,+foo2,-bar1,-bar2`) + + testConfig.productVariables.NativeCoveragePaths = []string{"foo1"} + testConfig.productVariables.NativeCoverageExcludePaths = []string{"bar1"} + verifyExtraFlags(t, testConfig, `--collect_code_coverage --instrumentation_filter=+foo1,-bar1`) + + testConfig.productVariables.NativeCoveragePaths = []string{"foo1"} + testConfig.productVariables.NativeCoverageExcludePaths = nil + verifyExtraFlags(t, testConfig, `--collect_code_coverage --instrumentation_filter=+foo1`) + + testConfig.productVariables.NativeCoveragePaths = nil + testConfig.productVariables.NativeCoverageExcludePaths = []string{"bar1"} + verifyExtraFlags(t, testConfig, `--collect_code_coverage --instrumentation_filter=-bar1`) + + testConfig.productVariables.ClangCoverage = boolPtr(false) + actual := verifyExtraFlags(t, testConfig, ``) + if strings.Contains(actual, "--collect_code_coverage") || + strings.Contains(actual, "--instrumentation_filter=") { + t.Errorf("Expected code coverage disabled, but got %#v", actual) + } +} + +func verifyExtraFlags(t *testing.T, config Config, expected string) string { + bazelContext, _ := testBazelContext(t, map[bazelCommand]string{}) + + err := bazelContext.InvokeBazel(config) + if err != nil { + t.Fatalf("Did not expect error invoking Bazel, but got %s", err) + } + + flags := bazelContext.bazelRunner.(*mockBazelRunner).extraFlags + if expected := 3; len(flags) != expected { + t.Errorf("Expected %d extra flags got %#v", expected, flags) + } + + actual := flags[1] + if !strings.Contains(actual, expected) { + t.Errorf("Expected %#v got %#v", expected, actual) + } + + return actual +} + func testBazelContext(t *testing.T, bazelCommandResults map[bazelCommand]string) (*bazelContext, string) { t.Helper() p := bazelPaths{ diff --git a/android/util.go b/android/util.go index 47c45833b..a0f716047 100644 --- a/android/util.go +++ b/android/util.go @@ -32,6 +32,12 @@ func CopyOf(s []string) []string { // JoinWithPrefix prepends the prefix to each string in the list and // returns them joined together with " " as separator. func JoinWithPrefix(strs []string, prefix string) string { + return JoinWithPrefixAndSeparator(strs, prefix, " ") +} + +// JoinWithPrefixAndSeparator prepends the prefix to each string in the list and +// returns them joined together with the given separator. +func JoinWithPrefixAndSeparator(strs []string, prefix string, sep string) string { if len(strs) == 0 { return "" } @@ -40,7 +46,7 @@ func JoinWithPrefix(strs []string, prefix string) string { buf.WriteString(prefix) buf.WriteString(strs[0]) for i := 1; i < len(strs); i++ { - buf.WriteString(" ") + buf.WriteString(sep) buf.WriteString(prefix) buf.WriteString(strs[i]) } diff --git a/bazel/aquery.go b/bazel/aquery.go index 3affcb0e3..030951eb5 100644 --- a/bazel/aquery.go +++ b/bazel/aquery.go @@ -659,6 +659,9 @@ func shouldSkipAction(a action) bool { if a.Mnemonic == "FileWrite" { return true } + if a.Mnemonic == "BaselineCoverage" { + return true + } return false } diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index a0bc9e7b4..2f912a99d 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/bp2build/cc_library_conversion_test.go @@ -959,11 +959,12 @@ func TestCcLibraryFeatures(t *testing.T) { "features": `[ "disable_pack_relocations", "-no_undefined_symbols", + "-coverage", ]`, "srcs": `["a.cpp"]`, })...) expected_targets = append(expected_targets, makeCcLibraryTargets("b", attrNameToString{ - "features": `select({ + "features": `["-coverage"] + select({ "//build/bazel/platforms/arch:x86_64": [ "disable_pack_relocations", "-no_undefined_symbols", @@ -994,6 +995,7 @@ cc_library { pack_relocations: false, allow_undefined_symbols: true, include_build_directory: false, + native_coverage: false, } cc_library { @@ -1006,6 +1008,7 @@ cc_library { }, }, include_build_directory: false, + native_coverage: false, } cc_library { diff --git a/cc/bp2build.go b/cc/bp2build.go index ba02b7ef8..cbb205603 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -55,6 +55,8 @@ type staticOrSharedAttributes struct { Enabled bazel.BoolAttribute + Native_coverage bazel.BoolAttribute + sdkAttributes } @@ -567,10 +569,15 @@ func bp2BuildParseBaseProps(ctx android.Bp2buildMutatorContext, module *Module) } } } - compilerAttrs.convertStlProps(ctx, module) (&linkerAttrs).convertStripProps(ctx, module) + if module.coverage != nil && module.coverage.Properties.Native_coverage != nil && + !Bool(module.coverage.Properties.Native_coverage) { + // Native_coverage is arch neutral + (&linkerAttrs).features.Append(bazel.MakeStringListAttribute([]string{"-coverage"})) + } + productVariableProps := android.ProductVariableProperties(ctx) (&compilerAttrs).convertProductVariables(ctx, productVariableProps) diff --git a/cmd/soong_build/main.go b/cmd/soong_build/main.go index a51db365a..80ab6d213 100644 --- a/cmd/soong_build/main.go +++ b/cmd/soong_build/main.go @@ -139,7 +139,7 @@ func runMixedModeBuild(configuration android.Config, ctx *android.Context, extra bazelHook := func() error { ctx.EventHandler.Begin("bazel") defer ctx.EventHandler.End("bazel") - return configuration.BazelContext.InvokeBazel() + return configuration.BazelContext.InvokeBazel(configuration) } ctx.SetBeforePrepareBuildActionsHook(bazelHook)