From 774d5b366ce43ed7e304ea3917e353112df9daf7 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 26 Apr 2024 15:58:35 -0400 Subject: make.bash: avoid ksh bug in nogoenv ksh handles make.bash surprisingly well and is a smaller supply chain attack surface, so it's reasonable to want to use "ksh make.bash" to build Go. The only place where ksh and bash disagree in running make.bash is an arguable bug in ksh that X=Y foo accidentally changes the real value of X following that command when foo is a shell function. (It correctly preserves the original value of X when foo is a command being invoked.) More specifically, GOROOT=$GOROOT_BOOTSTRAP nogoenv foo incorrectly changes $GOROOT in the rest of the script. CL 580020 suggested using a subshell, but subshells historically have lost "set -e", so we'd have to use (...) || exit 1. Instead of that, this CL refactors nogoenv into bootstrapenv, putting it in charge of changing $GOROOT the same way it changes all the other environment variables. This CL also updates make.rc for parallelism. It does not bother updating make.bat: that part is already a bit different, and attempting to change it is all risk, no reward. Change-Id: I5923a6fb5016a3862363363859365d1cd4f61a1d Reviewed-on: https://go-review.googlesource.com/c/go/+/582076 LUCI-TryBot-Result: Go LUCI Reviewed-by: Ian Lance Taylor Reviewed-by: Eric Grosse --- src/make.bash | 11 ++++++----- src/make.rc | 11 ++++++----- 2 files changed, 12 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/src/make.bash b/src/make.bash index 933573dd9d..5b49fcccf7 100755 --- a/src/make.bash +++ b/src/make.bash @@ -159,14 +159,15 @@ if [[ -z "$GOROOT_BOOTSTRAP" ]]; then fi export GOROOT_BOOTSTRAP -nogoenv() { - GO111MODULE=off GOENV=off GOOS= GOARCH= GOEXPERIMENT= GOFLAGS= "$@" +bootstrapenv() { + GOROOT="$GOROOT_BOOTSTRAP" GO111MODULE=off GOENV=off GOOS= GOARCH= GOEXPERIMENT= GOFLAGS= "$@" } export GOROOT="$(cd .. && pwd)" IFS=$'\n'; for go_exe in $(type -ap go); do if [[ ! -x "$GOROOT_BOOTSTRAP/bin/go" ]]; then - goroot=$(GOROOT= nogoenv "$go_exe" env GOROOT) + GOROOT_BOOTSTRAP="" + goroot=$(bootstrapenv "$go_exe" env GOROOT) if [[ "$goroot" != "$GOROOT" ]]; then if [[ "$goroot_bootstrap_set" == "true" ]]; then printf 'WARNING: %s does not exist, found %s from env\n' "$GOROOT_BOOTSTRAP/bin/go" "$go_exe" >&2 @@ -184,7 +185,7 @@ fi # Get the exact bootstrap toolchain version to help with debugging. # We clear GOOS and GOARCH to avoid an ominous but harmless warning if # the bootstrap doesn't support them. -GOROOT_BOOTSTRAP_VERSION=$(nogoenv "$GOROOT_BOOTSTRAP/bin/go" version | sed 's/go version //') +GOROOT_BOOTSTRAP_VERSION=$(bootstrapenv "$GOROOT_BOOTSTRAP/bin/go" version | sed 's/go version //') echo "Building Go cmd/dist using $GOROOT_BOOTSTRAP. ($GOROOT_BOOTSTRAP_VERSION)" if $verbose; then echo cmd/dist @@ -195,7 +196,7 @@ if [[ "$GOROOT_BOOTSTRAP" == "$GOROOT" ]]; then exit 1 fi rm -f cmd/dist/dist -GOROOT="$GOROOT_BOOTSTRAP" nogoenv "$GOROOT_BOOTSTRAP/bin/go" build -o cmd/dist/dist ./cmd/dist +bootstrapenv "$GOROOT_BOOTSTRAP/bin/go" build -o cmd/dist/dist ./cmd/dist # -e doesn't propagate out of eval, so check success by hand. eval $(./cmd/dist/dist env -p || echo FAIL=true) diff --git a/src/make.rc b/src/make.rc index 607e9360dc..27456f759d 100755 --- a/src/make.rc +++ b/src/make.rc @@ -44,8 +44,8 @@ if(~ $1 -v) { shift } -fn nogoenv { - GO111MODULE=off GOENV=off GOOS=() GOARCH=() GOEXPERIMENT=() GOFLAGS=() $* +fn bootstrapenv { + GOROOT=$GOROOT_BOOTSTRAP GO111MODULE=off GOENV=off GOOS=() GOARCH=() GOEXPERIMENT=() GOFLAGS=() $* } bootgo = 1.20.6 @@ -60,8 +60,9 @@ if(! ~ $#GOROOT_BOOTSTRAP 1){ } for(p in $path){ if(! test -x $GOROOT_BOOTSTRAP/bin/go){ + GOROOT_BOOTSTRAP = () if(go_exe = `{path=$p whatis go}){ - goroot = `{GOROOT=() nogoenv $go_exe env GOROOT} + goroot = `{bootstrapenv $go_exe env GOROOT} if(! ~ $goroot $GOROOT){ if(~ $goroot_bootstrap_set 'true'){ echo 'WARNING: '$GOROOT_BOOTSTRAP'/bin/go does not exist, found '$go_exe' from env' >[1=2] @@ -86,11 +87,11 @@ if(~ $GOROOT_BOOTSTRAP $GOROOT){ # Get the exact bootstrap toolchain version to help with debugging. # We clear GOOS and GOARCH to avoid an ominous but harmless warning if # the bootstrap doesn't support them. -GOROOT_BOOTSTRAP_VERSION=`{nogoenv $GOROOT_BOOTSTRAP/bin/go version | sed 's/go version //'} +GOROOT_BOOTSTRAP_VERSION=`{bootstrapenv $GOROOT_BOOTSTRAP/bin/go version | sed 's/go version //'} echo 'Building Go cmd/dist using '$GOROOT_BOOTSTRAP'. ('$"GOROOT_BOOTSTRAP_VERSION')' if(~ $#vflag 1) echo cmd/dist -GOROOT=$GOROOT_BOOTSTRAP nogoenv $GOROOT_BOOTSTRAP/bin/go build -o cmd/dist/dist ./cmd/dist +bootstrapenv $GOROOT_BOOTSTRAP/bin/go build -o cmd/dist/dist ./cmd/dist eval `{./cmd/dist/dist env -9} if(~ $#vflag 1) -- cgit v1.2.3-54-g00ecf