From: Bryan C. Mills <bcmills@google.com>
Date: Fri, 5 May 2023 13:29:34 +0000 (-0400)
Subject: internal/testenv: reduce init-time work for MustHaveExec
X-Git-Tag: go1.21rc1~649
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=3e35df5edbb02ecf8efd6dd6993aabd5053bfc66;p=gostls13.git

internal/testenv: reduce init-time work for MustHaveExec

In CL 486275 I added a somewhat complex init function that sets up a
callback to probe for exec support. A lot of the complexity was simply
to avoid an unnecessary call to os.Environ during init.

In CL 491660, I made the os.Environ call unconditional on all
platforms anyway in order to make HasGoBuild more robust.

Since the init-function indirection no longer serves a useful purpose,
I would like to simplify it to a package-level function, avoiding the
complexity of changing package variables at init time.

Change-Id: Ie0041d52cbde06ff14540192c8fba869a851158e
Reviewed-on: https://go-review.googlesource.com/c/go/+/492977
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
---

diff --git a/src/internal/testenv/exec.go b/src/internal/testenv/exec.go
index 481be2e649..c67ff53a72 100644
--- a/src/internal/testenv/exec.go
+++ b/src/internal/testenv/exec.go
@@ -6,6 +6,8 @@ package testenv
 
 import (
 	"context"
+	"errors"
+	"fmt"
 	"os"
 	"os/exec"
 	"runtime"
@@ -30,25 +32,24 @@ import (
 // for the resulting error.
 func MustHaveExec(t testing.TB) {
 	tryExecOnce.Do(func() {
-		tryExecOk = tryExec()
+		tryExecErr = tryExec()
 	})
-	if !tryExecOk {
-		t.Skipf("skipping test: cannot exec subprocess on %s/%s", runtime.GOOS, runtime.GOARCH)
+	if tryExecErr != nil {
+		t.Skipf("skipping test: cannot exec subprocess on %s/%s: %v", runtime.GOOS, runtime.GOARCH, tryExecErr)
 	}
 }
 
 var (
-	tryExec     = func() bool { return true }
 	tryExecOnce sync.Once
-	tryExecOk   bool
+	tryExecErr  error
 )
 
-func init() {
+func tryExec() error {
 	switch runtime.GOOS {
 	case "wasip1", "js", "ios":
 	default:
 		// Assume that exec always works on non-mobile platforms and Android.
-		return
+		return nil
 	}
 
 	// ios has an exec syscall but on real iOS devices it might return a
@@ -64,24 +65,18 @@ func init() {
 		// This isn't a standard 'go test' binary, so we don't know how to
 		// self-exec in a way that should succeed without side effects.
 		// Just forget it.
-		tryExec = func() bool { return false }
-		return
+		return errors.New("can't probe for exec support with a non-test executable")
 	}
 
 	// We know that this is a test executable. We should be able to run it with a
 	// no-op flag to check for overall exec support.
-	tryExec = func() bool {
-		exe, err := os.Executable()
-		if err != nil {
-			return false
-		}
-		cmd := exec.Command(exe, "-test.list=^$")
-		cmd.Env = origEnv
-		if err := cmd.Run(); err == nil {
-			return true
-		}
-		return false
+	exe, err := os.Executable()
+	if err != nil {
+		return fmt.Errorf("can't probe for exec support: %w", err)
 	}
+	cmd := exec.Command(exe, "-test.list=^$")
+	cmd.Env = origEnv
+	return cmd.Run()
 }
 
 var execPaths sync.Map // path -> error