]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: add detection of toolchain switch loops
authorRuss Cox <rsc@golang.org>
Fri, 26 May 2023 03:28:39 +0000 (23:28 -0400)
committerGopher Robot <gobot@golang.org>
Wed, 31 May 2023 15:20:23 +0000 (15:20 +0000)
This happens mainly during testing because the virtual
toolchain switch is not terribly robust, and if you accidentally
try to exec "1.23" instead of "go1.23" it will let you, but it
won't work right.

Of course, although we feel pretty good about the non-test
implementation, perhaps it has a toolchain switch loop lurking too,
or perhaps one will be introduced in the future.

To handle the test bug, and just in case we have a real bug later,
add detection of toolchain switch loops with clear messages.

Also fixes a bug in setting the -lang flag properly when invoking
the Go compiler: this is the first test using 'go 1.21.x' lines
during a build.

For #57001.

Change-Id: I0ece3dd718596689a23b677cf08ddf32ea97bc57
Reviewed-on: https://go-review.googlesource.com/c/go/+/498436
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/cmd/go/go_test.go
src/cmd/go/internal/gover/local.go [moved from src/cmd/go/internal/gover/latest.go with 52% similarity]
src/cmd/go/internal/toolchain/exec.go
src/cmd/go/internal/toolchain/toolchain.go
src/cmd/go/internal/work/exec.go
src/cmd/go/internal/work/gc.go
src/cmd/go/main.go
src/cmd/go/testdata/script/gotoolchain_local.txt
src/cmd/go/testdata/script/gotoolchain_loop.txt [new file with mode: 0644]
src/cmd/go/testdata/script/mod_get_exec_toolchain.txt
src/cmd/go/testdata/script/mod_toolchain.txt

index e50144f7f09627fb608c887d2517d7df60f70725..54249f6f7a47f5c878764ece3a34deb402f33081 100644 (file)
@@ -36,6 +36,7 @@ import (
        "cmd/go/internal/gover"
        "cmd/go/internal/robustio"
        "cmd/go/internal/search"
+       "cmd/go/internal/toolchain"
        "cmd/go/internal/vcs"
        "cmd/go/internal/vcweb/vcstest"
        "cmd/go/internal/web"
@@ -108,10 +109,8 @@ func TestMain(m *testing.M) {
        // We set CMDGO_TEST_RUN_MAIN via os.Setenv and testScript.setup.
        if os.Getenv("CMDGO_TEST_RUN_MAIN") != "" {
                cfg.SetGOROOT(cfg.GOROOT, true)
-
-               if v := os.Getenv("TESTGO_VERSION"); v != "" {
-                       gover.TestVersion = v
-               }
+               gover.TestVersion = os.Getenv("TESTGO_VERSION")
+               toolchain.TestVersionSwitch = os.Getenv("TESTGO_VERSION_SWITCH")
                if v := os.Getenv("TESTGO_TOOLCHAIN_VERSION"); v != "" {
                        work.ToolchainVersion = v
                }
similarity index 52%
rename from src/cmd/go/internal/gover/latest.go
rename to src/cmd/go/internal/gover/local.go
index 16357b8d3031099ebe75c455fd5280a6e18aa60a..8183a5c3d47497d4f70f4af5b4a8dc2e8d2b0da1 100644 (file)
@@ -17,14 +17,26 @@ var TestVersion string
 
 // Local returns the local Go version, the one implemented by this go command.
 func Local() string {
-       v := runtime.Version()
+       v, _ := local()
+       return v
+}
+
+// LocalToolchain returns the local toolchain name, the one implemented by this go command.
+func LocalToolchain() string {
+       _, t := local()
+       return t
+}
+
+func local() (goVers, toolVers string) {
+       toolVers = runtime.Version()
        if TestVersion != "" {
-               v = TestVersion
+               toolVers = TestVersion
        }
-       if v := FromToolchain(v); v != "" {
-               return v
+       goVers = FromToolchain(toolVers)
+       if goVers == "" {
+               // Development branch. Use "Dev" version with just 1.N, no rc1 or .0 suffix.
+               goVers = "1." + strconv.Itoa(goversion.Version)
+               toolVers = "go" + goVers
        }
-
-       // Development branch. Use "Dev" version with just 1.N, no rc1 or .0 suffix.
-       return "1." + strconv.Itoa(goversion.Version)
+       return goVers, toolVers
 }
index 4e6a13e35fa263d654fb6116c67d41f77f06e415..820fe93e87c377e770b7fe1573de50a74f12dc3d 100644 (file)
@@ -20,7 +20,7 @@ import (
 // The GOROOT directory is empty if we are invoking a command named
 // gotoolchain found in $PATH.
 func execGoToolchain(gotoolchain, dir, exe string) {
-       os.Setenv(gotoolchainSwitchEnv, "1")
+       os.Setenv(targetEnv, gotoolchain)
        if dir == "" {
                os.Unsetenv("GOROOT")
        } else {
index 3a8d348abb588f7c10a71a2a7408ec6ca8f999d2..ab03fbe4ff1ae859a62391055c735d83d8ba4221 100644 (file)
@@ -16,6 +16,7 @@ import (
        "path/filepath"
        "runtime"
        "sort"
+       "strconv"
        "strings"
 
        "cmd/go/internal/base"
@@ -40,16 +41,46 @@ const (
        gotoolchainModule  = "golang.org/toolchain"
        gotoolchainVersion = "v0.0.1"
 
-       // gotoolchainSwitchEnv is a special environment variable
-       // set to 1 during the toolchain switch by the parent process
-       // and cleared in the child process. When set, that indicates
-       // to the child not to do its own toolchain switch logic,
-       // to avoid an infinite recursion if for some reason a toolchain
-       // did not believe it could handle its own version and then
-       // reinvoked itself.
-       gotoolchainSwitchEnv = "GOTOOLCHAIN_INTERNAL_SWITCH"
+       // targetEnv is a special environment variable set to the expected
+       // toolchain version during the toolchain switch by the parent
+       // process and cleared in the child process. When set, that indicates
+       // to the child to confirm that it provides the expected toolchain version.
+       targetEnv = "GOTOOLCHAIN_INTERNAL_SWITCH_VERSION"
+
+       // countEnv is a special environment variable
+       // that is incremented during each toolchain switch, to detect loops.
+       // It is cleared before invoking programs in 'go run', 'go test', 'go generate', and 'go tool'
+       // by invoking them in an environment filtered with FilterEnv,
+       // so user programs should not see this in their environment.
+       countEnv = "GOTOOLCHAIN_INTERNAL_SWITCH_COUNT"
+
+       // maxSwitch is the maximum toolchain switching depth.
+       // Most uses should never see more than three.
+       // (Perhaps one for the initial GOTOOLCHAIN dispatch,
+       // a second for go get doing an upgrade, and a third if
+       // for some reason the chosen upgrade version is too small
+       // by a little.)
+       // When the count reaches maxSwitch - 10, we start logging
+       // the switched versions for debugging before crashing with
+       // a fatal error upon reaching maxSwitch.
+       // That should be enough to see the repetition.
+       maxSwitch = 100
 )
 
+// FilterEnv returns a copy of env with internal GOTOOLCHAIN environment
+// variables filtered out.
+func FilterEnv(env []string) []string {
+       // Note: Don't need to filter out targetEnv because Switch does that.
+       var out []string
+       for _, e := range env {
+               if strings.HasPrefix(e, countEnv+"=") {
+                       continue
+               }
+               out = append(out, e)
+       }
+       return out
+}
+
 // Switch invokes a different Go toolchain if directed by
 // the GOTOOLCHAIN environment variable or the user's configuration
 // or go.mod file.
@@ -58,10 +89,6 @@ func Switch() {
        log.SetPrefix("go: ")
        defer log.SetPrefix("")
 
-       sw := os.Getenv(gotoolchainSwitchEnv)
-       os.Unsetenv(gotoolchainSwitchEnv)
-       // The sw == "1" check is delayed until later so that we still fill in gover.Startup for use in errors.
-
        if !modload.WillBeEnabled() {
                return
        }
@@ -78,20 +105,19 @@ func Switch() {
                return
        }
 
-       var minToolchain, minVers string
-       if x, y, ok := strings.Cut(gotoolchain, "+"); ok { // go1.2.3+auto
-               orig := gotoolchain
-               minToolchain, gotoolchain = x, y
-               minVers = gover.FromToolchain(minToolchain)
-               if minVers == "" {
-                       base.Fatalf("invalid GOTOOLCHAIN %q: invalid minimum toolchain %q", orig, minToolchain)
+       minToolchain := gover.LocalToolchain()
+       minVers := gover.Local()
+       if min, mode, ok := strings.Cut(gotoolchain, "+"); ok { // go1.2.3+auto
+               v := gover.FromToolchain(min)
+               if v == "" {
+                       base.Fatalf("invalid GOTOOLCHAIN %q: invalid minimum toolchain %q", gotoolchain, min)
                }
-               if gotoolchain != "auto" && gotoolchain != "path" {
-                       base.Fatalf("invalid GOTOOLCHAIN %q: only version suffixes are +auto and +path", orig)
+               minToolchain = min
+               minVers = v
+               if mode != "auto" && mode != "path" {
+                       base.Fatalf("invalid GOTOOLCHAIN %q: only version suffixes are +auto and +path", gotoolchain)
                }
-       } else {
-               minVers = gover.Local()
-               minToolchain = "go" + minVers
+               gotoolchain = mode
        }
 
        if gotoolchain == "auto" || gotoolchain == "path" {
@@ -153,7 +179,34 @@ func Switch() {
                }
        }
 
-       if sw == "1" || gotoolchain == "local" || gotoolchain == "go"+gover.Local() {
+       // If we are invoked as a target toolchain, confirm that
+       // we provide the expected version and then run.
+       // This check is delayed until after the handling of auto and path
+       // so that we have initialized gover.Startup for use in error messages.
+       if target := os.Getenv(targetEnv); target != "" && TestVersionSwitch != "loop" {
+               if gover.LocalToolchain() != target {
+                       base.Fatalf("toolchain %v invoked to provide %v", gover.LocalToolchain(), target)
+               }
+               os.Unsetenv(targetEnv)
+
+               // Note: It is tempting to check that if gotoolchain != "local"
+               // then target == gotoolchain here, as a sanity check that
+               // the child has made the same version determination as the parent.
+               // This turns out not always to be the case. Specifically, if we are
+               // running Go 1.21 with GOTOOLCHAIN=go1.22+auto, which invokes
+               // Go 1.22, then 'go get go@1.23.0' or 'go get needs_go_1_23'
+               // will invoke Go 1.23, but as the Go 1.23 child the reason for that
+               // will not be apparent here: it will look like we should be using Go 1.22.
+               // We rely on the targetEnv being set to know not to downgrade.
+               // A longer term problem with the sanity check is that the exact details
+               // may change over time: there may be other reasons that a future Go
+               // version might invoke an older one, and the older one won't know why.
+               // Best to just accept that we were invoked to provide a specific toolchain
+               // (which we just checked) and leave it at that.
+               return
+       }
+
+       if gotoolchain == "local" || gotoolchain == gover.LocalToolchain() {
                // Let the current binary handle the command.
                return
        }
@@ -287,6 +340,14 @@ func HasPath() bool {
        return env == "path" || strings.HasSuffix(env, "+path")
 }
 
+// TestVersionSwitch is set in the test go binary to the value in $TESTGO_VERSION_SWITCH.
+// Valid settings are:
+//
+//     "switch" - simulate version switches by reinvoking the test go binary with a different TESTGO_VERSION.
+//     "mismatch" - like "switch" but forget to set TESTGO_VERSION, so it looks like we invoked a mismatched toolchain
+//     "loop" - like "switch" but
+var TestVersionSwitch string
+
 // SwitchTo invokes the specified Go toolchain or else prints an error and exits the process.
 // If $GOTOOLCHAIN is set to path or min+path, SwitchTo only considers the PATH
 // as a source of Go toolchains. Otherwise SwitchTo tries the PATH but then downloads
@@ -294,16 +355,32 @@ func HasPath() bool {
 func SwitchTo(gotoolchain string) {
        log.SetPrefix("go: ")
 
+       count, _ := strconv.Atoi(os.Getenv(countEnv))
+       if count >= maxSwitch-10 {
+               fmt.Fprintf(os.Stderr, "go: switching from go%v to %v [depth %d]\n", gover.Local(), gotoolchain, count)
+       }
+       if count >= maxSwitch {
+               base.Fatalf("too many toolchain switches")
+       }
+       os.Setenv(countEnv, fmt.Sprint(count+1))
+
        env := cfg.Getenv("GOTOOLCHAIN")
        pathOnly := env == "path" || strings.HasSuffix(env, "+path")
 
        // For testing, if TESTGO_VERSION is already in use
        // (only happens in the cmd/go test binary)
-       // and TESTGO_VERSION_SWITCH=1 is set,
+       // and TESTGO_VERSION_SWITCH=switch is set,
        // "switch" toolchains by changing TESTGO_VERSION
        // and reinvoking the current binary.
-       if gover.TestVersion != "" && os.Getenv("TESTGO_VERSION_SWITCH") == "1" {
+       // The special cases =loop and =mismatch skip the
+       // setting of TESTGO_VERSION so that it looks like we
+       // accidentally invoked the wrong toolchain,
+       // to test detection of that failure mode.
+       switch TestVersionSwitch {
+       case "switch":
                os.Setenv("TESTGO_VERSION", gotoolchain)
+               fallthrough
+       case "loop", "mismatch":
                exe, err := os.Executable()
                if err != nil {
                        base.Fatalf("%v", err)
@@ -422,7 +499,7 @@ func modGoToolchain() (file, goVers, toolchain string) {
 
 // goInstallVersion looks at the command line to see if it is go install m@v or go run m@v.
 // If so, it returns the m@v and the go version from that module's go.mod.
-func goInstallVersion() (m module.Version, goVers string, ok bool) {
+func goInstallVersion() (m module.Version, goVers string, found bool) {
        // Note: We assume there are no flags between 'go' and 'install' or 'run'.
        // During testing there are some debugging flags that are accepted
        // in that position, but in production go binaries there are not.
@@ -503,10 +580,15 @@ func goInstallVersion() (m module.Version, goVers string, ok bool) {
        }
        noneSelected := func(path string) (version string) { return "none" }
        _, err := modload.QueryPackages(ctx, m.Path, m.Version, noneSelected, allowed)
-       tooNew, ok := err.(*gover.TooNewError)
-       if !ok {
-               return module.Version{}, "", false
+       if tooNew, ok := err.(*gover.TooNewError); ok {
+               m.Path, m.Version, _ = strings.Cut(tooNew.What, "@")
+               return m, tooNew.GoVersion, true
        }
-       m.Path, m.Version, _ = strings.Cut(tooNew.What, "@")
-       return m, tooNew.GoVersion, true
+
+       // QueryPackages succeeded, or it failed for a reason other than
+       // this Go toolchain being too old for the modules encountered.
+       // Either way, we identified the m@v on the command line,
+       // so return found == true so the caller does not fall back to
+       // consulting go.mod.
+       return m, "", true
 }
index 3303b7c21158988bb2981733e9ad09b83f25a51f..998d0007d012e4cc8d7ec96814f0a3517ac294ae 100644 (file)
@@ -35,6 +35,7 @@ import (
        "cmd/go/internal/cache"
        "cmd/go/internal/cfg"
        "cmd/go/internal/fsys"
+       "cmd/go/internal/gover"
        "cmd/go/internal/load"
        "cmd/go/internal/modload"
        "cmd/go/internal/slices"
@@ -431,17 +432,7 @@ func allowedVersion(v string) bool {
        if v == "" {
                return true
        }
-       // Special case "1.0" means "go1", which is OK.
-       if v == "1.0" {
-               return true
-       }
-       // Otherwise look through release tags of form "go1.23" for one that matches.
-       for _, tag := range cfg.BuildContext.ReleaseTags {
-               if strings.HasPrefix(tag, "go") && tag[2:] == v {
-                       return true
-               }
-       }
-       return false
+       return gover.Compare(gover.Local(), v) >= 0
 }
 
 const (
index 480338bfb247678239f35bb8b49500b497ddd7b0..6043ad5353e404ccf19e51ed40011dea1207d333 100644 (file)
@@ -19,6 +19,7 @@ import (
        "cmd/go/internal/base"
        "cmd/go/internal/cfg"
        "cmd/go/internal/fsys"
+       "cmd/go/internal/gover"
        "cmd/go/internal/load"
        "cmd/go/internal/str"
        "cmd/internal/objabi"
@@ -99,7 +100,7 @@ func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg
                        v = "1.16"
                }
                if allowedVersion(v) {
-                       defaultGcFlags = append(defaultGcFlags, "-lang=go"+v)
+                       defaultGcFlags = append(defaultGcFlags, "-lang=go"+gover.Lang(v))
                }
        }
        if p.Standard {
index c4a75f87e3663ef45c39923483c62be683a4d8b2..03ac15a37d9335840d88070b9f8f346f918b1915 100644 (file)
@@ -209,7 +209,7 @@ func invoke(cmd *base.Command, args []string) {
        // the same default computation of these as we do,
        // but in practice there might be skew
        // This makes sure we all agree.
-       cfg.OrigEnv = os.Environ()
+       cfg.OrigEnv = toolchain.FilterEnv(os.Environ())
        cfg.CmdEnv = envcmd.MkEnv()
        for _, env := range cfg.CmdEnv {
                if os.Getenv(env.Name) != env.Value {
index a7e2b36cc226af0f4e4fab6793d4bd9fdb16c554..18b4faabde7e49d239a5f1c2a384456d148adce9 100644 (file)
@@ -3,7 +3,7 @@
 # See gotoolchain_net.txt and gotoolchain_path.txt for tests of network and PATH toolchains.
 
 env TESTGO_VERSION=go1.500
-env TESTGO_VERSION_SWITCH=1
+env TESTGO_VERSION_SWITCH=switch
 
 # Default setting should be auto
 env GOTOOLCHAIN=
diff --git a/src/cmd/go/testdata/script/gotoolchain_loop.txt b/src/cmd/go/testdata/script/gotoolchain_loop.txt
new file mode 100644 (file)
index 0000000..a803d2e
--- /dev/null
@@ -0,0 +1,65 @@
+env GOTOOLCHAIN=auto
+env TESTGO_VERSION=go1.21.1
+
+# Basic switch should work.
+env TESTGO_VERSION_SWITCH=switch
+go version
+stdout go1.21.99
+
+# Toolchain target mismatch should be detected.
+env TESTGO_VERSION_SWITCH=mismatch
+! go version
+stderr '^go: toolchain go1.21.1 invoked to provide go1.21.99$'
+
+# Toolchain loop should be detected.
+env TESTGO_VERSION_SWITCH=loop
+! go version
+stderr -count=10 '^go: switching from go1.21.1 to go1.21.99 \[depth 9[0-9]\]$'
+stderr -count=1 '^go: switching from go1.21.1 to go1.21.99 \[depth 100\]$'
+stderr '^go: too many toolchain switches$'
+
+[short] skip
+
+# Internal env vars should not leak to go test or go run.
+env TESTGO_VERSION_SWITCH=switch
+go version
+stdout go1.21.99
+go test
+stdout clean
+go run .
+stdout clean
+
+-- go.mod --
+module m
+go 1.21.99
+
+-- m_test.go --
+package main
+
+import "testing"
+
+func TestEnv(t *testing.T) {
+       // the check is in func init in m.go
+}
+
+-- m.go --
+package main
+
+import "os"
+
+func init() {
+       envs := []string{
+               "GOTOOLCHAIN_INTERNAL_SWITCH_COUNT",
+               "GOTOOLCHAIN_INTERNAL_SWITCH_VERSION",
+       }
+       for _, e := range envs {
+               if v := os.Getenv(e); v != "" {
+                       panic("$"+e+"="+v)
+               }
+       }
+       os.Stdout.WriteString("clean\n")
+}
+
+func main() {
+}
+
index a9aa27ec0ac6b1671dea20ca5fb64dac795ca181..ac8e2cc698401d9ae44723acc367792f93cf9810 100644 (file)
@@ -1,5 +1,5 @@
 env TESTGO_VERSION=go1.21
-env TESTGO_VERSION_SWITCH=1
+env TESTGO_VERSION_SWITCH=switch
 
 # GOTOOLCHAIN=auto should run the newer toolchain
 env GOTOOLCHAIN=auto
index d0f8b913e7d2a9cdde8f2ebdaaf74b90521da70e..f92d9822324efead8ec8c68910b70a4403e497c1 100644 (file)
@@ -1,5 +1,5 @@
 env TESTGO_VERSION=go1.100
-env TESTGO_VERSION_SWITCH=1
+env TESTGO_VERSION_SWITCH=switch
 
 go get toolchain@go1.22.1
 stderr '^go: added toolchain go1.22.1$'