diff --git a/android/bazel_handler.go b/android/bazel_handler.go index e7ff08f72..e7b84e304 100644 --- a/android/bazel_handler.go +++ b/android/bazel_handler.go @@ -607,7 +607,7 @@ func NewBazelContext(c *config) (BazelContext, error) { dclaEnabledModules := map[string]bool{} addToStringSet(dclaEnabledModules, dclaMixedBuildsEnabledList) return &mixedBuildBazelContext{ - bazelRunner: &builtinBazelRunner{}, + bazelRunner: &builtinBazelRunner{c.UseBazelProxy, absolutePath(c.outDir)}, paths: &paths, modulesDefaultToBazel: c.BuildMode == BazelDevMode, bazelEnabledModules: enabledModules, @@ -684,23 +684,46 @@ func (r *mockBazelRunner) issueBazelCommand(bazelCmd *exec.Cmd, _ *metrics.Event return "", "", nil } -type builtinBazelRunner struct{} +type builtinBazelRunner struct { + useBazelProxy bool + outDir string +} // Issues the given bazel command with given build label and additional flags. // 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) { - 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) + 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}) + + if err != nil { + return "", "", err + } + if len(resp.ErrorString) > 0 { + return "", "", fmt.Errorf(resp.ErrorString) + } + return resp.Stdout, resp.Stderr, nil } else { - return string(output), string(stderr.Bytes()), nil + 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 + } } } diff --git a/android/config.go b/android/config.go index c0f84c8aa..1f90ad7bd 100644 --- a/android/config.go +++ b/android/config.go @@ -87,6 +87,8 @@ type CmdArgs struct { BazelModeDev bool BazelModeStaging bool BazelForceEnabledModules string + + UseBazelProxy bool } // Build modes that soong_build can run as. @@ -251,6 +253,10 @@ type config struct { // specified modules. They are passed via the command-line flag // "--bazel-force-enabled-modules" bazelForceEnabledModules map[string]struct{} + + // If true, for any requests to Bazel, communicate with a Bazel proxy using + // unix sockets, instead of spawning Bazel as a subprocess. + UseBazelProxy bool } type deviceConfig struct { @@ -442,6 +448,8 @@ func NewConfig(cmdArgs CmdArgs, availableEnv map[string]string) (Config, error) mixedBuildDisabledModules: make(map[string]struct{}), mixedBuildEnabledModules: make(map[string]struct{}), bazelForceEnabledModules: make(map[string]struct{}), + + UseBazelProxy: cmdArgs.UseBazelProxy, } config.deviceConfig = &deviceConfig{ diff --git a/bazel/Android.bp b/bazel/Android.bp index d11c78b04..4709f5cd3 100644 --- a/bazel/Android.bp +++ b/bazel/Android.bp @@ -7,6 +7,7 @@ bootstrap_go_package { pkgPath: "android/soong/bazel", srcs: [ "aquery.go", + "bazel_proxy.go", "configurability.go", "constants.go", "properties.go", diff --git a/bazel/bazel_proxy.go b/bazel/bazel_proxy.go new file mode 100644 index 000000000..d7f5e6464 --- /dev/null +++ b/bazel/bazel_proxy.go @@ -0,0 +1,219 @@ +// Copyright 2023 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package bazel + +import ( + "bytes" + "encoding/gob" + "fmt" + "net" + os_lib "os" + "os/exec" + "path/filepath" + "strings" + "time" +) + +// Logs fatal events of ProxyServer. +type ServerLogger interface { + Fatal(v ...interface{}) + Fatalf(format string, v ...interface{}) +} + +// CmdRequest is a request to the Bazel Proxy server. +type CmdRequest struct { + // Args to the Bazel command. + Argv []string + // Environment variables to pass to the Bazel invocation. Strings should be of + // the form "KEY=VALUE". + Env []string +} + +// CmdResponse is a response from the Bazel Proxy server. +type CmdResponse struct { + Stdout string + Stderr string + ErrorString string +} + +// ProxyClient is a client which can issue Bazel commands to the Bazel +// proxy server. Requests are issued (and responses received) via a unix socket. +// See ProxyServer for more details. +type ProxyClient struct { + outDir string +} + +// ProxyServer is a server which runs as a background goroutine. Each +// request to the server describes a Bazel command which the server should run. +// The server then issues the Bazel command, and returns a response describing +// the stdout/stderr of the command. +// Client-server communication is done via a unix socket under the output +// directory. +// The server is intended to circumvent sandboxing for subprocesses of the +// build. The build orchestrator (soong_ui) can launch a server to exist outside +// of sandboxing, and sandboxed processes (such as soong_build) can issue +// bazel commands through this socket tunnel. This allows a sandboxed process +// to issue bazel requests to a bazel that resides outside of sandbox. This +// is particularly useful to maintain a persistent Bazel server which lives +// past the duration of a single build. +// The ProxyServer will only live as long as soong_ui does; the +// underlying Bazel server will live past the duration of the build. +type ProxyServer struct { + logger ServerLogger + outDir string + workspaceDir string + // The server goroutine will listen on this channel and stop handling requests + // once it is written to. + done chan struct{} +} + +// NewProxyClient is a constructor for a ProxyClient. +func NewProxyClient(outDir string) *ProxyClient { + return &ProxyClient{ + outDir: outDir, + } +} + +func unixSocketPath(outDir string) string { + return filepath.Join(outDir, "bazelsocket.sock") +} + +// IssueCommand issues a request to the Bazel Proxy Server to issue a Bazel +// request. Returns a response describing the output from the Bazel process +// (if the Bazel process had an error, then the response will include an error). +// Returns an error if there was an issue with the connection to the Bazel Proxy +// server. +func (b *ProxyClient) IssueCommand(req CmdRequest) (CmdResponse, error) { + var resp CmdResponse + var err error + // Check for connections every 1 second. This is chosen to be a relatively + // short timeout, because the proxy server should accept requests quite + // quickly. + d := net.Dialer{Timeout: 1 * time.Second} + var conn net.Conn + conn, err = d.Dial("unix", unixSocketPath(b.outDir)) + if err != nil { + return resp, err + } + defer conn.Close() + + enc := gob.NewEncoder(conn) + if err = enc.Encode(req); err != nil { + return resp, err + } + dec := gob.NewDecoder(conn) + err = dec.Decode(&resp) + return resp, err +} + +// NewProxyServer is a constructor for a ProxyServer. +func NewProxyServer(logger ServerLogger, outDir string, workspaceDir string) *ProxyServer { + return &ProxyServer{ + logger: logger, + outDir: outDir, + workspaceDir: workspaceDir, + done: make(chan struct{}), + } +} + +func (b *ProxyServer) handleRequest(conn net.Conn) error { + defer conn.Close() + + dec := gob.NewDecoder(conn) + var req CmdRequest + if err := dec.Decode(&req); err != nil { + 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) + } + + resp := CmdResponse{stdout, string(stderr.Bytes()), bazelErrString} + enc := gob.NewEncoder(conn) + if err := enc.Encode(&resp); err != nil { + return fmt.Errorf("Error encoding response: %s", err) + } + return nil +} + +func (b *ProxyServer) listenUntilClosed(listener net.Listener) error { + for { + // Check for connections every 1 second. This is a blocking operation, so + // if the server is closed, the goroutine will not fully close until this + // deadline is reached. Thus, this deadline is short (but not too short + // so that the routine churns). + listener.(*net.UnixListener).SetDeadline(time.Now().Add(time.Second)) + conn, err := listener.Accept() + + select { + case <-b.done: + return nil + default: + } + + if err != nil { + if opErr, ok := err.(*net.OpError); ok && opErr.Timeout() { + // Timeout is normal and expected while waiting for client to establish + // a connection. + continue + } else { + b.logger.Fatalf("Listener error: %s", err) + } + } + + err = b.handleRequest(conn) + if err != nil { + b.logger.Fatal(err) + } + } +} + +// Start initializes the server unix socket and (in a separate goroutine) +// handles requests on the socket until the server is closed. Returns an error +// if a failure occurs during initialization. Will log any post-initialization +// errors to the server's logger. +func (b *ProxyServer) Start() error { + unixSocketAddr := unixSocketPath(b.outDir) + if err := os_lib.RemoveAll(unixSocketAddr); err != nil { + return fmt.Errorf("couldn't remove socket '%s': %s", unixSocketAddr, err) + } + listener, err := net.Listen("unix", unixSocketAddr) + + if err != nil { + return fmt.Errorf("error listening on socket '%s': %s", unixSocketAddr, err) + } + + go b.listenUntilClosed(listener) + return nil +} + +// Close shuts down the server. This will stop the server from listening for +// additional requests. +func (b *ProxyServer) Close() { + b.done <- struct{}{} +} diff --git a/cmd/soong_build/main.go b/cmd/soong_build/main.go index 5f27fa783..5c187f6cf 100644 --- a/cmd/soong_build/main.go +++ b/cmd/soong_build/main.go @@ -81,6 +81,7 @@ func init() { flag.BoolVar(&cmdlineArgs.BazelMode, "bazel-mode", false, "use bazel for analysis of certain modules") flag.BoolVar(&cmdlineArgs.BazelModeStaging, "bazel-mode-staging", false, "use bazel for analysis of certain near-ready modules") flag.BoolVar(&cmdlineArgs.BazelModeDev, "bazel-mode-dev", false, "use bazel for analysis of a large number of modules (less stable)") + flag.BoolVar(&cmdlineArgs.UseBazelProxy, "use-bazel-proxy", false, "communicate with bazel using unix socket proxy instead of spawning subprocesses") // Flags that probably shouldn't be flags of soong_build, but we haven't found // the time to remove them yet diff --git a/tests/persistent_bazel_test.sh b/tests/persistent_bazel_test.sh new file mode 100755 index 000000000..4e2982a39 --- /dev/null +++ b/tests/persistent_bazel_test.sh @@ -0,0 +1,83 @@ +#!/bin/bash -eu + +# Copyright (C) 2023 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -o pipefail + +source "$(dirname "$0")/lib.sh" + +# This test verifies that adding USE_PERSISTENT_BAZEL creates a Bazel process +# that outlasts the build process. +# This test should only be run in sandboxed environments (because this test +# verifies a Bazel process using global process list, and may spawn lingering +# Bazel processes). +function test_persistent_bazel { + setup + + # Ensure no existing Bazel process. + if [[ -e out/bazel/output/server/server.pid.txt ]]; then + kill $(cat out/bazel/output/server/server.pid.txt) 2>/dev/null || true + if kill -0 $(cat out/bazel/output/server/server.pid.txt) 2>/dev/null ; then + fail "Error killing pre-setup bazel" + fi + fi + + USE_PERSISTENT_BAZEL=1 run_soong nothing + + if ! kill -0 $(cat out/bazel/output/server/server.pid.txt) 2>/dev/null ; then + fail "Persistent bazel process expected, but not found after first build" + fi + BAZEL_PID=$(cat out/bazel/output/server/server.pid.txt) + + USE_PERSISTENT_BAZEL=1 run_soong nothing + + if ! kill -0 $BAZEL_PID 2>/dev/null ; then + fail "Bazel pid $BAZEL_PID was killed after second build" + fi + + kill $BAZEL_PID 2>/dev/null + if ! kill -0 $BAZEL_PID 2>/dev/null ; then + fail "Error killing bazel on shutdown" + fi +} + +# Verifies that USE_PERSISTENT_BAZEL mode operates as expected in the event +# that there are Bazel failures. +function test_bazel_failure { + setup + + # Ensure no existing Bazel process. + if [[ -e out/bazel/output/server/server.pid.txt ]]; then + kill $(cat out/bazel/output/server/server.pid.txt) 2>/dev/null || true + if kill -0 $(cat out/bazel/output/server/server.pid.txt) 2>/dev/null ; then + fail "Error killing pre-setup bazel" + fi + fi + + # Introduce a syntax error in a BUILD file which is used in every build + # (Note this is a BUILD file which is copied as part of test setup, so this + # has no effect on sources outside of this test. + rm -rf build/bazel/rules + + USE_PERSISTENT_BAZEL=1 run_soong nothing 1>out/failurelog.txt 2>&1 && fail "Expected build failure" || true + + if ! grep -sq "'build/bazel/rules' is not a package" out/failurelog.txt ; then + fail "Expected error to contain 'build/bazel/rules' is not a package, instead got:\n$(cat out/failurelog.txt)" + fi + + kill $(cat out/bazel/output/server/server.pid.txt) 2>/dev/null || true +} + +scan_and_run_tests diff --git a/tests/run_integration_tests.sh b/tests/run_integration_tests.sh index eb76a461f..676df23a2 100755 --- a/tests/run_integration_tests.sh +++ b/tests/run_integration_tests.sh @@ -7,6 +7,7 @@ TOP="$(readlink -f "$(dirname "$0")"/../../..)" "$TOP/build/soong/tests/bootstrap_test.sh" "$TOP/build/soong/tests/mixed_mode_test.sh" "$TOP/build/soong/tests/bp2build_bazel_test.sh" +"$TOP/build/soong/tests/persistent_bazel_test.sh" "$TOP/build/soong/tests/soong_test.sh" "$TOP/build/bazel/ci/rbc_regression_test.sh" aosp_arm64-userdebug @@ -17,4 +18,4 @@ TOP="$(readlink -f "$(dirname "$0")"/../../..)" "$TOP/build/soong/tests/apex_cc_module_arch_variant_tests.sh" "$TOP/build/soong/tests/apex_cc_module_arch_variant_tests.sh" "aosp_arm" "armv7-a" -"$TOP/build/soong/tests/apex_cc_module_arch_variant_tests.sh" "aosp_cf_arm64_phone" "armv8-a" "cortex-a53" \ No newline at end of file +"$TOP/build/soong/tests/apex_cc_module_arch_variant_tests.sh" "aosp_cf_arm64_phone" "armv8-a" "cortex-a53" diff --git a/ui/build/config.go b/ui/build/config.go index 73e2c45e1..b5ee44064 100644 --- a/ui/build/config.go +++ b/ui/build/config.go @@ -1568,6 +1568,10 @@ func (c *configImpl) IsBazelMixedBuildForceDisabled() bool { return c.Environment().IsEnvTrue("BUILD_BROKEN_DISABLE_BAZEL") } +func (c *configImpl) IsPersistentBazelEnabled() bool { + return c.Environment().IsEnvTrue("USE_PERSISTENT_BAZEL") +} + func (c *configImpl) BazelModulesForceEnabledByFlag() string { return c.bazelForceEnabledModules } diff --git a/ui/build/soong.go b/ui/build/soong.go index e6543ec17..a5a3263bb 100644 --- a/ui/build/soong.go +++ b/ui/build/soong.go @@ -21,6 +21,7 @@ import ( "strconv" "strings" + "android/soong/bazel" "android/soong/ui/metrics" "android/soong/ui/status" @@ -268,6 +269,9 @@ func bootstrapBlueprint(ctx Context, config Config) { if config.bazelStagingMode { mainSoongBuildExtraArgs = append(mainSoongBuildExtraArgs, "--bazel-mode-staging") } + if config.IsPersistentBazelEnabled() { + mainSoongBuildExtraArgs = append(mainSoongBuildExtraArgs, "--use-bazel-proxy") + } if len(config.bazelForceEnabledModules) > 0 { mainSoongBuildExtraArgs = append(mainSoongBuildExtraArgs, "--bazel-force-enabled-modules="+config.bazelForceEnabledModules) } @@ -497,6 +501,12 @@ func runSoong(ctx Context, config Config) { ctx.BeginTrace(metrics.RunSoong, name) defer ctx.EndTrace() + if config.IsPersistentBazelEnabled() { + bazelProxy := bazel.NewProxyServer(ctx.Logger, config.OutDir(), filepath.Join(config.SoongOutDir(), "workspace")) + bazelProxy.Start() + defer bazelProxy.Close() + } + fifo := filepath.Join(config.OutDir(), ".ninja_fifo") nr := status.NewNinjaReader(ctx, ctx.Status.StartTool(), fifo) defer nr.Close()