From 3b8531f0c0739003a208b7beb002d1058656a962 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Thu, 12 Sep 2019 15:19:57 -0700 Subject: Allow to remove env variables. Previously, we only supported setting env variables to empty, but not to remove it. This lead to the case that we never removed the CCACHE_DISABLE env variable, which kept the ccache disabled, and caused a performance regression compared to the old wrapper as the new wrapper didn't use the ccache in this case. This cl also adds tests for the real exec and run commands to prevent regressions in these cases. BUG=chromium:773875 TEST=new unit tests TEST=performance analysis for TEST=emerge-veyron_jerry --nodeps chromeos-kernel-4_19 Change-Id: I5ca88ba8d7b05c3e12e292465fcd4ff9925b0344 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/1802159 Tested-by: Tobias Bosch Reviewed-by: George Burgess --- compiler_wrapper/env.go | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) (limited to 'compiler_wrapper/env.go') diff --git a/compiler_wrapper/env.go b/compiler_wrapper/env.go index 588441dd..c8f31357 100644 --- a/compiler_wrapper/env.go +++ b/compiler_wrapper/env.go @@ -9,12 +9,11 @@ import ( "fmt" "io" "os" - "os/exec" "strings" ) type env interface { - getenv(key string) string + getenv(key string) (string, bool) environ() []string getwd() string stdin() io.Reader @@ -47,8 +46,8 @@ func newProcessEnv() (env, error) { var _ env = (*processEnv)(nil) -func (env *processEnv) getenv(key string) string { - return os.Getenv(key) +func (env *processEnv) getenv(key string) (string, bool) { + return os.LookupEnv(key) } func (env *processEnv) environ() []string { @@ -72,26 +71,11 @@ func (env *processEnv) stderr() io.Writer { } func (env *processEnv) exec(cmd *command) error { - execCmd := exec.Command(cmd.Path, cmd.Args...) - // Note: We are not using execve and pass the new environment here - // as that sometimes doesn't work well with the gentoo sandbox to - // pick update changes to SANDBOX_WRITE env variable (needed for ccache). - // Instead, we are updating our own environment and call execv. - // This update of global state is ok as we won't execute anything else - // after the exec. - for _, update := range cmd.EnvUpdates { - parts := strings.Split(update, "=") - os.Setenv(parts[0], parts[1]) - } - return libcExecv(execCmd.Path, execCmd.Args) + return libcExec(cmd) } func (env *processEnv) run(cmd *command, stdin io.Reader, stdout io.Writer, stderr io.Writer) error { - execCmd := newExecCmd(env, cmd) - execCmd.Stdin = stdin - execCmd.Stdout = stdout - execCmd.Stderr = stderr - return execCmd.Run() + return runCmd(env, cmd, stdin, stdout, stderr) } type commandRecordingEnv struct { -- cgit v1.2.3