diff --git a/android/bazel_handler.go b/android/bazel_handler.go index 9c273d9a3..85d00ccef 100644 --- a/android/bazel_handler.go +++ b/android/bazel_handler.go @@ -18,7 +18,6 @@ import ( "bytes" "fmt" "os" - "os/exec" "path" "path/filepath" "runtime" @@ -30,7 +29,6 @@ import ( "android/soong/bazel/cquery" "android/soong/shared" "android/soong/starlark_fmt" - "github.com/google/blueprint" "github.com/google/blueprint/metrics" @@ -220,8 +218,7 @@ type BazelContext interface { } type bazelRunner interface { - createBazelCommand(config Config, paths *bazelPaths, runName bazel.RunName, command bazelCommand, extraFlags ...string) *exec.Cmd - issueBazelCommand(bazelCmd *exec.Cmd, eventHandler *metrics.EventHandler) (output string, errorMessage string, error error) + issueBazelCommand(cmdRequest bazel.CmdRequest, paths *bazelPaths, eventHandler *metrics.EventHandler) (output string, errorMessage string, error error) } type bazelPaths struct { @@ -660,36 +657,6 @@ type bazelCommand struct { expression string } -type mockBazelRunner struct { - bazelCommandResults map[bazelCommand]string - // use *exec.Cmd as a key to get the bazelCommand, the map will be used in issueBazelCommand() - // Register createBazelCommand() invocations. Later, an - // issueBazelCommand() invocation can be mapped to the *exec.Cmd instance - // and then to the expected result via bazelCommandResults - tokens map[*exec.Cmd]bazelCommand - commands []bazelCommand - extraFlags []string -} - -func (r *mockBazelRunner) createBazelCommand(_ Config, _ *bazelPaths, _ bazel.RunName, - command bazelCommand, extraFlags ...string) *exec.Cmd { - r.commands = append(r.commands, command) - r.extraFlags = append(r.extraFlags, strings.Join(extraFlags, " ")) - cmd := &exec.Cmd{} - if r.tokens == nil { - r.tokens = make(map[*exec.Cmd]bazelCommand) - } - r.tokens[cmd] = command - return cmd -} - -func (r *mockBazelRunner) issueBazelCommand(bazelCmd *exec.Cmd, _ *metrics.EventHandler) (string, string, error) { - if command, ok := r.tokens[bazelCmd]; ok { - return r.bazelCommandResults[command], "", nil - } - return "", "", nil -} - type builtinBazelRunner struct { useBazelProxy bool outDir string @@ -699,17 +666,12 @@ type builtinBazelRunner struct { // Returns (stdout, stderr, error). The first and second return values are strings // containing the stdout and stderr of the run command, and an error is returned if // the invocation returned an error code. -func (r *builtinBazelRunner) issueBazelCommand(bazelCmd *exec.Cmd, eventHandler *metrics.EventHandler) (string, string, error) { +func (r *builtinBazelRunner) issueBazelCommand(cmdRequest bazel.CmdRequest, paths *bazelPaths, eventHandler *metrics.EventHandler) (string, string, error) { if r.useBazelProxy { eventHandler.Begin("client_proxy") defer eventHandler.End("client_proxy") proxyClient := bazel.NewProxyClient(r.outDir) - // Omit the arg containing the Bazel binary, as that is handled by the proxy - // server. - bazelFlags := bazelCmd.Args[1:] - // TODO(b/270989498): Refactor these functions to not take exec.Cmd, as its - // not actually executed for client proxying. - resp, err := proxyClient.IssueCommand(bazel.CmdRequest{bazelFlags, bazelCmd.Env}) + resp, err := proxyClient.IssueCommand(cmdRequest) if err != nil { return "", "", err @@ -721,26 +683,19 @@ func (r *builtinBazelRunner) issueBazelCommand(bazelCmd *exec.Cmd, eventHandler } else { eventHandler.Begin("bazel command") defer eventHandler.End("bazel command") - stderr := &bytes.Buffer{} - bazelCmd.Stderr = stderr - if output, err := bazelCmd.Output(); err != nil { - return "", string(stderr.Bytes()), - fmt.Errorf("bazel command failed: %s\n---command---\n%s\n---env---\n%s\n---stderr---\n%s---", - err, bazelCmd, strings.Join(bazelCmd.Env, "\n"), stderr) - } else { - return string(output), string(stderr.Bytes()), nil - } + + stdout, stderr, err := bazel.ExecBazel(paths.bazelPath, absolutePath(paths.syntheticWorkspaceDir()), cmdRequest) + return string(stdout), string(stderr), err } } -func (r *builtinBazelRunner) createBazelCommand(config Config, paths *bazelPaths, runName bazel.RunName, command bazelCommand, - extraFlags ...string) *exec.Cmd { +func (context *mixedBuildBazelContext) createBazelCommand(config Config, runName bazel.RunName, command bazelCommand, + extraFlags ...string) bazel.CmdRequest { cmdFlags := []string{ - "--output_base=" + absolutePath(paths.outputBase), + "--output_base=" + absolutePath(context.paths.outputBase), command.command, command.expression, - // TODO(asmundak): is it needed in every build? - "--profile=" + shared.BazelMetricsFilename(paths, runName), + "--profile=" + shared.BazelMetricsFilename(context.paths, runName), // We don't need to set --host_platforms because it's set in bazelrc files // that the bazel shell script wrapper passes @@ -755,15 +710,13 @@ func (r *builtinBazelRunner) createBazelCommand(config Config, paths *bazelPaths } cmdFlags = append(cmdFlags, extraFlags...) - bazelCmd := exec.Command(paths.bazelPath, cmdFlags...) - bazelCmd.Dir = absolutePath(paths.syntheticWorkspaceDir()) extraEnv := []string{ - "HOME=" + paths.homeDir, + "HOME=" + context.paths.homeDir, pwdPrefix(), - "BUILD_DIR=" + absolutePath(paths.soongOutDir), + "BUILD_DIR=" + absolutePath(context.paths.soongOutDir), // Make OUT_DIR absolute here so build/bazel/bin/bazel uses the correct // OUT_DIR at /out, instead of /out/soong/workspace/out. - "OUT_DIR=" + absolutePath(paths.outDir()), + "OUT_DIR=" + absolutePath(context.paths.outDir()), // Disables local host detection of gcc; toolchain information is defined // explicitly in BUILD files. "BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1", @@ -775,15 +728,15 @@ func (r *builtinBazelRunner) createBazelCommand(config Config, paths *bazelPaths } extraEnv = append(extraEnv, fmt.Sprintf("%s=%s", envvar, val)) } - bazelCmd.Env = append(os.Environ(), extraEnv...) + envVars := append(os.Environ(), extraEnv...) - return bazelCmd + return bazel.CmdRequest{cmdFlags, envVars} } -func printableCqueryCommand(bazelCmd *exec.Cmd) string { - outputString := strings.Join(bazelCmd.Env, " ") + " \"" + strings.Join(bazelCmd.Args, "\" \"") + "\"" +func (context *mixedBuildBazelContext) printableCqueryCommand(bazelCmd bazel.CmdRequest) string { + args := append([]string{context.paths.bazelPath}, bazelCmd.Argv...) + outputString := strings.Join(bazelCmd.Env, " ") + " \"" + strings.Join(args, "\" \"") + "\"" return outputString - } func (context *mixedBuildBazelContext) mainBzlFileContents() []byte { @@ -1106,9 +1059,10 @@ func (p *bazelPaths) outDir() string { const buildrootLabel = "@soong_injection//mixed_builds:buildroot" var ( - cqueryCmd = bazelCommand{"cquery", fmt.Sprintf("deps(%s, 2)", buildrootLabel)} - aqueryCmd = bazelCommand{"aquery", fmt.Sprintf("deps(%s)", buildrootLabel)} - buildCmd = bazelCommand{"build", "@soong_injection//mixed_builds:phonyroot"} + cqueryCmd = bazelCommand{"cquery", fmt.Sprintf("deps(%s, 2)", buildrootLabel)} + aqueryCmd = bazelCommand{"aquery", fmt.Sprintf("deps(%s)", buildrootLabel)} + buildCmd = bazelCommand{"build", "@soong_injection//mixed_builds:phonyroot"} + allBazelCommands = []bazelCommand{aqueryCmd, cqueryCmd, buildCmd} ) // Issues commands to Bazel to receive results for all cquery requests @@ -1170,12 +1124,12 @@ func (context *mixedBuildBazelContext) runCquery(config Config, ctx invokeBazelC extraFlags = append(extraFlags, "--collect_code_coverage") } - cqueryCommandWithFlag := context.createBazelCommand(config, context.paths, bazel.CqueryBuildRootRunName, cqueryCmd, extraFlags...) - cqueryOutput, cqueryErrorMessage, cqueryErr := context.issueBazelCommand(cqueryCommandWithFlag, eventHandler) + cqueryCmdRequest := context.createBazelCommand(config, bazel.CqueryBuildRootRunName, cqueryCmd, extraFlags...) + cqueryOutput, cqueryErrorMessage, cqueryErr := context.issueBazelCommand(cqueryCmdRequest, context.paths, eventHandler) if cqueryErr != nil { return cqueryErr } - cqueryCommandPrint := fmt.Sprintf("cquery command line:\n %s \n\n\n", printableCqueryCommand(cqueryCommandWithFlag)) + cqueryCommandPrint := fmt.Sprintf("cquery command line:\n %s \n\n\n", context.printableCqueryCommand(cqueryCmdRequest)) if err := os.WriteFile(filepath.Join(soongInjectionPath, "cquery.out"), []byte(cqueryCommandPrint+cqueryOutput), 0666); err != nil { return err } @@ -1233,8 +1187,8 @@ func (context *mixedBuildBazelContext) runAquery(config Config, ctx invokeBazelC extraFlags = append(extraFlags, "--instrumentation_filter="+strings.Join(paths, ",")) } } - aqueryOutput, _, err := context.issueBazelCommand(context.createBazelCommand(config, context.paths, bazel.AqueryBuildRootRunName, aqueryCmd, - extraFlags...), eventHandler) + aqueryOutput, _, err := context.issueBazelCommand(context.createBazelCommand(config, bazel.AqueryBuildRootRunName, aqueryCmd, + extraFlags...), context.paths, eventHandler) if err != nil { return err } @@ -1249,7 +1203,7 @@ func (context *mixedBuildBazelContext) generateBazelSymlinks(config Config, ctx // Issue a build command of the phony root to generate symlink forests for dependencies of the // Bazel build. This is necessary because aquery invocations do not generate this symlink forest, // but some of symlinks may be required to resolve source dependencies of the build. - _, _, err := context.issueBazelCommand(context.createBazelCommand(config, context.paths, bazel.BazelBuildPhonyRootRunName, buildCmd), eventHandler) + _, _, err := context.issueBazelCommand(context.createBazelCommand(config, bazel.BazelBuildPhonyRootRunName, buildCmd), context.paths, eventHandler) return err } diff --git a/android/bazel_handler_test.go b/android/bazel_handler_test.go index c67d7fb66..b17b7652b 100644 --- a/android/bazel_handler_test.go +++ b/android/bazel_handler_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "android/soong/bazel" "android/soong/bazel/cquery" analysis_v2_proto "prebuilts/bazel/common/proto/analysis_v2" @@ -19,6 +20,34 @@ var testConfig = TestConfig("out", nil, "", nil) type testInvokeBazelContext struct{} +type mockBazelRunner struct { + testHelper *testing.T + // Stores mock behavior. If an issueBazelCommand request is made for command + // k, and {k:v} is present in this map, then the mock will return v. + bazelCommandResults map[bazelCommand]string + // Requests actually made of the mockBazelRunner with issueBazelCommand, + // keyed by the command they represent. + bazelCommandRequests map[bazelCommand]bazel.CmdRequest +} + +func (r *mockBazelRunner) bazelCommandForRequest(cmdRequest bazel.CmdRequest) bazelCommand { + for _, arg := range cmdRequest.Argv { + for _, cmdType := range allBazelCommands { + if arg == cmdType.command { + return cmdType + } + } + } + r.testHelper.Fatalf("Unrecognized bazel request: %s", cmdRequest) + return cqueryCmd +} + +func (r *mockBazelRunner) issueBazelCommand(cmdRequest bazel.CmdRequest, paths *bazelPaths, eventHandler *metrics.EventHandler) (string, string, error) { + command := r.bazelCommandForRequest(cmdRequest) + r.bazelCommandRequests[command] = cmdRequest + return r.bazelCommandResults[command], "", nil +} + func (t *testInvokeBazelContext) GetEventHandler() *metrics.EventHandler { return &metrics.EventHandler{} } @@ -36,9 +65,7 @@ func TestRequestResultsAfterInvokeBazel(t *testing.T) { `@//foo:foo|arm64_armv8-a|android|within_apex|29>>out/foo/foo.txt`, `@//foo:bar|arm64_armv8-a|android>>out/foo/bar.txt`, } - bazelContext, _ := testBazelContext(t, map[bazelCommand]string{ - bazelCommand{command: "cquery", expression: "deps(@soong_injection//mixed_builds:buildroot, 2)"}: strings.Join(cmd_results, "\n"), - }) + bazelContext, _ := testBazelContext(t, map[bazelCommand]string{cqueryCmd: strings.Join(cmd_results, "\n")}) bazelContext.QueueBazelRequest(label_foo, cquery.GetOutputFiles, cfg_foo) bazelContext.QueueBazelRequest(label_bar, cquery.GetOutputFiles, cfg_bar) @@ -139,8 +166,7 @@ func TestInvokeBazelPopulatesBuildStatements(t *testing.T) { if err != nil { t.Error(err) } - bazelContext, _ := testBazelContext(t, map[bazelCommand]string{ - bazelCommand{command: "aquery", expression: "deps(@soong_injection//mixed_builds:buildroot)"}: string(data)}) + bazelContext, _ := testBazelContext(t, map[bazelCommand]string{aqueryCmd: string(data)}) err = bazelContext.InvokeBazel(testConfig, &testInvokeBazelContext{}) if err != nil { @@ -166,30 +192,26 @@ func TestCoverageFlagsAfterInvokeBazel(t *testing.T) { testConfig.productVariables.NativeCoveragePaths = []string{"foo1", "foo2"} testConfig.productVariables.NativeCoverageExcludePaths = []string{"bar1", "bar2"} - verifyExtraFlags(t, testConfig, `--collect_code_coverage --instrumentation_filter=+foo1,+foo2,-bar1,-bar2`) + verifyAqueryContainsFlags(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`) + verifyAqueryContainsFlags(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`) + verifyAqueryContainsFlags(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`) + verifyAqueryContainsFlags(t, testConfig, "--collect_code_coverage", "--instrumentation_filter=-bar1") testConfig.productVariables.NativeCoveragePaths = []string{"*"} testConfig.productVariables.NativeCoverageExcludePaths = nil - verifyExtraFlags(t, testConfig, `--collect_code_coverage --instrumentation_filter=+.*`) + verifyAqueryContainsFlags(t, testConfig, "--collect_code_coverage", "--instrumentation_filter=+.*") 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) - } + verifyAqueryDoesNotContainSubstrings(t, testConfig, "collect_code_coverage", "instrumentation_filter") } func TestBazelRequestsSorted(t *testing.T) { @@ -268,7 +290,8 @@ func TestIsModuleNameAllowed(t *testing.T) { } } -func verifyExtraFlags(t *testing.T, config Config, expected string) string { +func verifyAqueryContainsFlags(t *testing.T, config Config, expected ...string) { + t.Helper() bazelContext, _ := testBazelContext(t, map[bazelCommand]string{}) err := bazelContext.InvokeBazel(config, &testInvokeBazelContext{}) @@ -276,17 +299,49 @@ func verifyExtraFlags(t *testing.T, config Config, expected string) string { 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) + sliceContains := func(slice []string, x string) bool { + for _, s := range slice { + if s == x { + return true + } + } + return false } - actual := flags[1] - if !strings.Contains(actual, expected) { - t.Errorf("Expected %#v got %#v", expected, actual) + aqueryArgv := bazelContext.bazelRunner.(*mockBazelRunner).bazelCommandRequests[aqueryCmd].Argv + + for _, expectedFlag := range expected { + if !sliceContains(aqueryArgv, expectedFlag) { + t.Errorf("aquery does not contain expected flag %#v. Argv was: %#v", expectedFlag, aqueryArgv) + } + } +} + +func verifyAqueryDoesNotContainSubstrings(t *testing.T, config Config, substrings ...string) { + t.Helper() + bazelContext, _ := testBazelContext(t, map[bazelCommand]string{}) + + err := bazelContext.InvokeBazel(config, &testInvokeBazelContext{}) + if err != nil { + t.Fatalf("Did not expect error invoking Bazel, but got %s", err) } - return actual + sliceContainsSubstring := func(slice []string, substring string) bool { + for _, s := range slice { + if strings.Contains(s, substring) { + return true + } + } + return false + } + + aqueryArgv := bazelContext.bazelRunner.(*mockBazelRunner).bazelCommandRequests[aqueryCmd].Argv + + for _, substring := range substrings { + if sliceContainsSubstring(aqueryArgv, substring) { + t.Errorf("aquery contains unexpected substring %#v. Argv was: %#v", substring, aqueryArgv) + } + } } func testBazelContext(t *testing.T, bazelCommandResults map[bazelCommand]string) (*mixedBuildBazelContext, string) { @@ -296,11 +351,14 @@ func testBazelContext(t *testing.T, bazelCommandResults map[bazelCommand]string) outputBase: "outputbase", workspaceDir: "workspace_dir", } - aqueryCommand := bazelCommand{command: "aquery", expression: "deps(@soong_injection//mixed_builds:buildroot)"} - if _, exists := bazelCommandResults[aqueryCommand]; !exists { - bazelCommandResults[aqueryCommand] = "" + if _, exists := bazelCommandResults[aqueryCmd]; !exists { + bazelCommandResults[aqueryCmd] = "" + } + runner := &mockBazelRunner{ + testHelper: t, + bazelCommandResults: bazelCommandResults, + bazelCommandRequests: map[bazelCommand]bazel.CmdRequest{}, } - runner := &mockBazelRunner{bazelCommandResults: bazelCommandResults} return &mixedBuildBazelContext{ bazelRunner: runner, paths: &p, diff --git a/bazel/bazel_proxy.go b/bazel/bazel_proxy.go index d7f5e6464..2940b9995 100644 --- a/bazel/bazel_proxy.go +++ b/bazel/bazel_proxy.go @@ -128,6 +128,24 @@ func NewProxyServer(logger ServerLogger, outDir string, workspaceDir string) *Pr } } +func ExecBazel(bazelPath string, workspaceDir string, request CmdRequest) (stdout []byte, stderr []byte, cmdErr error) { + bazelCmd := exec.Command(bazelPath, request.Argv...) + bazelCmd.Dir = workspaceDir + bazelCmd.Env = request.Env + + stderrBuffer := &bytes.Buffer{} + bazelCmd.Stderr = stderrBuffer + + if output, err := bazelCmd.Output(); err != nil { + cmdErr = fmt.Errorf("bazel command failed: %s\n---command---\n%s\n---env---\n%s\n---stderr---\n%s---", + err, bazelCmd, strings.Join(bazelCmd.Env, "\n"), stderrBuffer) + } else { + stdout = output + } + stderr = stderrBuffer.Bytes() + return +} + func (b *ProxyServer) handleRequest(conn net.Conn) error { defer conn.Close() @@ -137,23 +155,13 @@ func (b *ProxyServer) handleRequest(conn net.Conn) error { return fmt.Errorf("Error decoding request: %s", err) } - bazelCmd := exec.Command("./build/bazel/bin/bazel", req.Argv...) - bazelCmd.Dir = b.workspaceDir - bazelCmd.Env = req.Env - - stderr := &bytes.Buffer{} - bazelCmd.Stderr = stderr - var stdout string - var bazelErrString string - - if output, err := bazelCmd.Output(); err != nil { - bazelErrString = fmt.Sprintf("bazel command failed: %s\n---command---\n%s\n---env---\n%s\n---stderr---\n%s---", - err, bazelCmd, strings.Join(bazelCmd.Env, "\n"), stderr) - } else { - stdout = string(output) + stdout, stderr, cmdErr := ExecBazel("./build/bazel/bin/bazel", b.workspaceDir, req) + errorString := "" + if cmdErr != nil { + errorString = cmdErr.Error() } - resp := CmdResponse{stdout, string(stderr.Bytes()), bazelErrString} + resp := CmdResponse{string(stdout), string(stderr), errorString} enc := gob.NewEncoder(conn) if err := enc.Encode(&resp); err != nil { return fmt.Errorf("Error encoding response: %s", err)