Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit bcbb1728 authored by Christopher Parsons's avatar Christopher Parsons Committed by Gerrit Code Review
Browse files

Merge "Refactor and cleanup of bazel handler"

parents b4207052 c9089dcc
Loading
Loading
Loading
Loading
+28 −74
Original line number Diff line number Diff line
@@ -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 <root>/out, instead of <root>/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 {
@@ -1109,6 +1062,7 @@ 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"}
	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
}

+85 −27
Original line number Diff line number Diff line
@@ -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
	}

	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)
	}

	actual := flags[1]
	if !strings.Contains(actual, expected) {
		t.Errorf("Expected %#v got %#v", expected, actual)
	sliceContainsSubstring := func(slice []string, substring string) bool {
		for _, s := range slice {
			if strings.Contains(s, substring) {
				return true
			}
		}
		return false
	}

	return actual
	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,
+23 −15
Original line number Diff line number Diff line
@@ -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)