Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extended forwards compatibility for Go #57001

Closed
rsc opened this issue Nov 30, 2022 · 208 comments
Closed

extended forwards compatibility for Go #57001

rsc opened this issue Nov 30, 2022 · 208 comments

Comments

@rsc
Copy link
Contributor

rsc commented Nov 30, 2022

Many people believe the go line in the go.mod file specifies which Go toolchain to use. This proposal would correct this widely held misunderstanding by making it reality. At the same time, the proposal would improve forward compatibility by making sure that old Go toolchains never try to build newer Go programs.

Define the “work module” as the one containing the directory where the go command is run. We sometimes call this the “main module”, but I am using “work module” in this document for clarity.

Updating the go line in the go.mod of the work module, or the go.work file in the current workspace, would change the minimum Go toolchain used to run go commands. A new toolchain line would provide finer-grained control over Go toolchain selection.

An environment variable GOTOOLCHAIN would control this new behavior. The default, GOTOOLCHAIN=auto, would use the information in go.mod. Setting GOTOOLCHAIN to something else would override the go.mod. GOTOOLCHAIN=local would force use of the locally installed toolchain, and other values would choose specific releases. For example, to test the package in the current directory with Go 1.17.2:

GOTOOLCHAIN=go1.17.2 go test

As part of this change, older Go toolchains would refuse to try to build newer Go code. The new system would arrange that normally this would not come up - the new toolchain would be used automatically. But if forced, such as when using GOTOOLCHAIN=local, the older Go toolchain would no longer assume it can build newer Go code. This in turn would make it safe to revisit and fix for loop scoping (discussion #56010).

See my talk on this topic at GopherCon for more background.

See the design document for details.

@gopherbot gopherbot added this to the Proposal milestone Nov 30, 2022
@rsc
Copy link
Contributor Author

rsc commented Nov 30, 2022

Updated link to design doc: https://go.dev/design/57001-gotoolchain.

@ianlancetaylor
Copy link
Contributor

I just want to note that the potential for confusion here is high. go build and go install should report whenever they invoke a different toolchain.

@rsc
Copy link
Contributor Author

rsc commented Nov 30, 2022

I'm not sure about that. What I hope will be a common mode of usage is that you install some update-aware Go toolchain on your machine and then from that point on just edit go or toolchain lines in your go.mod in various projects and never explicitly update the locally installed Go toolchain ever again. The Go toolchain becomes a detail managed inside go.mod just like all the other inputs to your build. In that mode of usage, this reporting would print on literally every go command that gets run, which is too noisy.

@dominikh
Copy link
Member

I think the combination of auto-upgrading and not allowing old toolchains to build new code will change the Go ecosystem to either no longer support multiple versions of Go, or to delay experimenting with new language features.

Where previously a Go module could target Go 1.21 but make use of 1.22 features in optional, build-tagged files, it can no longer do so. To be allowed to use Go 1.22 language features, the go.mod has to specify go 1.22, but this triggers auto-upgrading / build failures on Go 1.21. The module now only supports Go 1.22.

Auto-uprading makes this seem fine at first: anyone who needs Go 1.22 will get it for free, automatically. However, I don't think auto-upgrading can be assumed to be pervasive. For one, most people who hack on Go at least occasionally will be using GOTOOLCHAIN=local, pointed at a git checkout. Furthermore, I predict that some Linux distributions will patch their Go packages to disable the automatic upgrading, as it side-steps their packages and some are allergic to that. This'll lead to users who are left behind even more than they're now due to slow-moving distributions.

@zikaeroh
Copy link
Contributor

I personally have a few reservations about this proposal.

Firstly, I don't feel that the Go version in go.mod should be the "minimum" supported Go version for a module. The way I have understood it has been that it's a "feature unlocker". See https://twitter.com/_myitcv/status/1391819772992659461, https://twitter.com/marcosnils/status/1391819263325974530, #46201

It's very possible to enable a newer version of Go in go.mod to use things that are only available in that version, but gate those uses behind build tags, falling back to something else for older versions of Go. For example, //go:embed can be placed behind build tags such that it won't be used if the version of Go in use can't support it. The same applies for new exported stdlib types/functions. If go.mod enforced a minimum, that code couldn't be written (or at least be annoying to test).

This pattern seems common enough; x/tools and x/tools/gopls both set go 1.18, but in actuality test and support versions back to Go 1.16. It seems like this setup wouldn't be very tenable with this proposal implemented, as Go would automatically ignore that and download something else. Sure, maybe they could use GOTOOLCHAIN to work around that, but I think that'd be a major shift in how people generally get Go in CI. (It's also awkward to be able to go forward by changing Go on $PATH, but not backwards.)

Secondly, the automatic download/execution of binary releases of Go seems really surprising. I feel like it's going to be very awkward for Linux distributions to lose control of the toolchain in use without environment variables (especially if they patch Go). I do wonder how many distros might patch Go entirely to force GOTOOLCHAIN=local as the default. I believe there are also examples of corporate forks of Go (I've seen Microsoft's mentioned before), and those would also likely patch away this behavior becuase it'd be a bad thing for those to start bypassing the expected toolchain, especially without any sort of warning message that it's happening.

There are also systems where the binaries downloaded from golang.org won't be functional, e.g. Nix/NixOS (where the system layout is entirely different and outside binaries require a lot of work to use), musl-based distros like alpine (musl is not a first class libc for Go, as far as I know), and so on. Those distributors may also have to disable this download functionality to prevent breakage (be it to make things work consistently in the first place, or to just stop users from reporting the failures as distro bugs).

This part of the proposal is being compared to module dependency management itself. I strongly feel that modules work really, really well in comparison to other languages' package management scenarios (like node_modules, virtualenv, etc) in that it all happens automatically for you. But, I think the way this proposal does this for Go versions itself is going to be too surprising and likely to break.

(I mentioned some of this in #55092 (comment), but my thread seems to have been missed as all of the threads around it were replied to.)

@rsc
Copy link
Contributor Author

rsc commented Nov 30, 2022

I spoke to @ianlancetaylor about his concerns. For most commands, you can always run go version to see what version you are going to get in the next command. I think that will be enough for those commands (Ian does not completely agree but is willing to wait and see). The one exception is go install path@version.

For go install path@version, I think we probably have to do the module fetch and then potentially re-exec the go command based on the go line. I would be inclined to say we don't respect any toolchain line, just as we don't respect any replace lines. That is, today go install path@version is shorthand for:

mkdir /tmp/empty
cd /tmp/empty
go mod init empty
go get path@version
go install path

I think it should continue to be shorthand for that, which would mean ignoring the toolchain line in path/go.mod.

In the case where go install path@version needs to fetch and run a newer Go toolchain, I think it would be appropriate to print a message:

$ go install path@version
go: building path@version using go1.25.1
$ 

@rsc
Copy link
Contributor Author

rsc commented Nov 30, 2022

@dominikh

I think the combination of auto-upgrading and not allowing old toolchains to build new code will change the Go ecosystem to either no longer support multiple versions of Go, or to delay experimenting with new language features.

Where previously a Go module could target Go 1.21 but make use of 1.22 features in optional, build-tagged files, it can no longer do so. To be allowed to use Go 1.22 language features, the go.mod has to specify go 1.22, but this triggers auto-upgrading / build failures on Go 1.21. The module now only supports Go 1.22.

This is true. Older versions of the module will still support Go 1.21, of course. It's just the latest version of the module that only supports Go 1.22. I don't think it's a sure thing that this is a problem. The same happens today for dependencies, of course, and it seems to be fine.

I agree that making it easier to update to a new Go toolchain may well result in people updating more quickly.

Auto-upgrading makes this seem fine at first: anyone who needs Go 1.22 will get it for free, automatically. However, I don't think auto-upgrading can be assumed to be pervasive. For one, most people who hack on Go at least occasionally will be using GOTOOLCHAIN=local, pointed at a git checkout. Furthermore, I predict that some Linux distributions will patch their Go packages to disable the automatic upgrading, as it side-steps their packages and some are allergic to that. This'll lead to users who are left behind even more than they're now due to slow-moving distributions.

People who hack on Go will be using GOTOOLCHAIN=local (that will be the default in their builds), but they will also presumably be hacking on the latest Go version, which will not be too old for any code.

I agree that some Linux distributions are likely to patch Go to default to GOTOOLCHAIN=local. That seems fine too, as long as users can still go env -w GOTOOLCHAIN=auto.

@rsc
Copy link
Contributor Author

rsc commented Nov 30, 2022

@zikaeroh, Alpine should not have problems running standard Go distributions starting in Go 1.20. I am not sure about NixOS. The only thing it should need is a libc.so.6 for the dynamic linker to resolve. Or maybe we should build the distribution cmd/go with -tags netgo and then it wouldn't even need that. I wonder what that would break...

@zikaeroh
Copy link
Contributor

My (rudimentary) understanding is that libc.6.so is not located in the "typical" location on NixOS, so would not be found by a normal Go binary. It'd be at a long path like /nix/store/ikl21vjfq900ccbqg1xasp83kadw6q8y-glibc-2.32-46/lib/libc.so.6 as each package deterministically uses specific other packages. So, a flavor of the Go package that uses the precompiled binary releases of Go would use patchelf to fix this. Downloaded versions would not have that patching applied, and therefore would not work.

See also: https://nixos.wiki/wiki/Packaging/Binaries

@rsc
Copy link
Contributor Author

rsc commented Nov 30, 2022

@zikaeroh

This pattern seems common enough; x/tools and x/tools/gopls both set go 1.18, but in actuality test and support versions back to Go 1.16. It seems like this setup wouldn't be very tenable with this proposal implemented, as Go would automatically ignore that and download something else. Sure, maybe they could use GOTOOLCHAIN to work around that, but I think that'd be a major shift in how people generally get Go in CI. (It's also awkward to be able to go forward by changing Go on $PATH, but not backwards.)

x/tools/gopls only tests that far back because they want to build with what's on people's machines. If what's on people's machines knew how to fetch a newer toolchain then we'd have stopped needing to support older versions long ago.

I think that one reason people are slow to update to new Go versions because it is too difficult. What's difficult is managing Go installations. This proposal removes that difficulty, which in turn should make it easier for people to keep up with newer versions.

@rsc
Copy link
Contributor Author

rsc commented Nov 30, 2022

Regarding Linux distributions setting GOTOOLCHAIN=local by default (which I think would be fine), I'm curious whether they apply similar rules to rustup or nvm. Does anyone know?

@zikaeroh
Copy link
Contributor

zikaeroh commented Nov 30, 2022

I think that one reason people are slow to update to new Go versions because it is too difficult. What's difficult is managing Go installations. This proposal removes that difficulty, which in turn should make it easier for people to keep up with newer versions.

I guess I'm confused; my impression was that the hardest problem in upgrading was not obtaining a new version of Go, but making sure that the Go code works for that new version of Go, which is the backwards compatibility proposal (not this one).

I would be very interested to know what proportion of Go users obtain Go via a package manager (versus golang.org). It seems to me like most package managers are going to set GOTOOLCHAIN=local (as noted by @dominikh and me above). But it also seems to me like most Linux users are getting Go via their distro's package manager and macOS users via brew (and I personally use scoop/chocolately/winget on Windows).

If all of those users are going to end up getting local as their default, is anyone going to use auto?

I'm curious whether they apply similar rules to rustup or nvm. Does anyone know?

On Arch, you can either install the rust package or rustup; both "provide" rust, but the actual packages are usually built in a clean chroot and automatically get the former, the latest rust compiler package.

Arch doesn't package nvm or any other node version manager; without the AUR, only the latest node/npm/yarn are provided. But, I think it's the case that many users may end up installing nvm (or its many alternatives), in which case the version in use is basically whatever anyone wants. The closest example I can think is projects which check in .node_version or use volta to pin a particular version for their project, but I've more often seen this for pinning a local dev setup and then more versions are checked in CI anyway.

@ianlancetaylor
Copy link
Contributor

We should describe what happens if we drop an existing port (per https://go.dev/wiki/PortingPolicy). Presumably the go command 1.N will see "go 1.N+1", try to download the binaries for 1.N+1, fail, and then give an error and stop the build.

For cases like NixOS I think we would have to expect users on that system to set GOTOOLCHAIN=local. And that in turn suggests that perhaps there should be a way to build a toolchain such that GOTOOLCHAIN=local is the default if not overridden by the environment variable.

@rsc
Copy link
Contributor Author

rsc commented Nov 30, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor Author

rsc commented Nov 30, 2022

I filed #57007 for building cmd/go without libc.so.6, which I think would make NixOS happy.

I agree that it should be possible to build a toolchain with GOTOOLCHAIN=local as the default, but I don't think most package managers should do this. For example I don't think it makes sense for user-installed package managers like Chocolatey or Homebrew to do this at all. They are not as pedantic about "we are the only way to install software on your machine!" as the operating system-installed package managers are, and they should not be going out of their way to break what will end up being a core feature of the Go experience.

I also think a fair number of Go developers still use the installers we provide, and those of course will have the GOTOOLCHAIN=auto default.

@rsc
Copy link
Contributor Author

rsc commented Nov 30, 2022

We should describe what happens if we drop an existing port (per https://go.dev/wiki/PortingPolicy). Presumably the go command 1.N will see "go 1.N+1", try to download the binaries for 1.N+1, fail, and then give an error and stop the build.

Yes, exactly. And we can give a good error.

@rsc
Copy link
Contributor Author

rsc commented Nov 30, 2022

I guess I'm confused; my impression was that the hardest problem in upgrading was not obtaining a new version of Go, but making sure that the Go code works for that new version of Go, which is the backwards compatibility proposal (not this one).

Go's backward compatibility is already very good. To the extent that it needs work, the backwards compatibility proposal will make it even better. That will leave actually getting the upgraded Go toolchain as the hardest problem. My experience maintaining other machines where I build Go programs but don't do Go development has been that I don't upgrade often at all because it is annoying to go download and unpack the right tar files. If all it took was editing a go.mod, I would do that far more often.

Another point that I forgot to make in the doc is that setting the Go toolchain in go.mod means that all developers working on a project will agree on the toolchain version, without other special arrangements. This was of course one of the big improvements of modules and moving out of GOPATH, for dependency versions. The same property can be provided for the Go toolchain version. This would also mean that if you are moving back and forth between two projects that have chosen different Go toolchain versions as their standard toolchain, you get the right one for that project automatically by virtue of just being in that project. You don't have to change your PATH each time you switch, or maintain a global symlink in $HOME/bin, or remember to type 'go1.18 build' in one place and 'go1.19 build' in the other, or any other kludge. It just does the right thing.

@rittneje
Copy link

rittneje commented Dec 1, 2022

In a CI/CD environment, I don't think this feature would be useful. I would expect that job to be configured to use the correct Go version in the first place. Downloading a newer toolchain might not work anyway due to firewall restrictions. And as others have mentioned, using an older compiler with GOTOOLCHAIN=local should not fail, as it may mean the module only uses newer language features conditionally and is verifying compatibility. In addition, this could easily lead to a situation where a developer who attempts to test with an older go version on their local machine but forgets to set GOTOOLCHAIN=local (or doesn't know about it) will get different results than the build server.

On another note, while the go directive in go.mod controls language features like octal literals and generics, today it has no bearing on the standard library implementation. My expectation is that if I run go build with compiler version 1.X, then it will use standard library version 1.X. But with this change, that will not be the case if I use an older compiler, unless I set GOTOOLCHAIN=local.

Another point that I forgot to make in the doc is that setting the Go toolchain in go.mod means that all developers working on a project will agree on the toolchain version, without other special arrangements.

We build our own version of the toolchain and distribute it to our developers, rather than using what is on golang.org. So this would not address that problem for us, especially because it sounds like such a toolchain will default to GOTOOLCHAIN=local anyway. What would be more useful for us in this regard is a way to do an exact string match on the compiler's go version and fail if it is wrong. But this is independent of the go directive. (For example, we are currently using 1.18, but our go.mod file says 1.17 to prevent developers from using generics.) I see this is touched on with the new toolchain directive but it's not clear it would buy much in our particular use case.

Another use case is if I am writing a library and want to easily test it with multiple go versions on my local machine. It would be convenient if I could just do something like go test -go=1.17 ./... and have it test with the latest 1.17 release (regardless of what my "real" go version is). But the key is I'd want to specify on the command line, not in go.mod.

@zikaeroh
Copy link
Contributor

zikaeroh commented Dec 1, 2022

Another point that I forgot to make in the doc is that setting the Go toolchain in go.mod means that all developers working on a project will agree on the toolchain version, without other special arrangements.

I find this to conflict with the idea from the original proposal that it's a minimum version; if my project says "go 1.13" because that's the minimum, I very likely do not want to use that version of Go for development. A newer toolchain will be faster and behave better when called by tooling like gopls or other analyzers (regardless of the version of Go that gopls is compiled with). Or, I will definitely want to publish binaries using the absolute latest version of Go possible. For example, esbuild says "go 1.13", but the actual binaries published to npm are from Go 1.19. If 1.13 this were to be the expected development version, that wouldn't be very optimal.

@mateusz834
Copy link
Member

mateusz834 commented Dec 1, 2022

An environment variable GOTOOLCHAIN would control this new behavior. The default, GOTOOLCHAIN=auto, would use the information in go.mod. Setting GOTOOLCHAIN to something else would override the go.mod. GOTOOLCHAIN=local would force use of the locally installed toolchain, and other values would choose specific releases. For example, to test the package in the current directory with Go 1.17.2:

GOTOOLCHAIN=go1.17.2 go test

To be honest I am fine with: go install github.org/dl/go1.17.2@latest.

@rittneje
Copy link

rittneje commented Dec 1, 2022

If you have a module that says go 1.12 [...] Cloud Native Buildpacks will always build your code with Go 1.12, even if much newer releases of Go exist. [...] The GitHub Action setup-go [...] has the same problems that Cloud Native Buildpacks do.

I feel like this proposal also doesn't really address this problem. To me, it sounds like both of these features are fundamentally mis-designed. If I am publishing a library on GitHub, then I should be specifying a range of minor versions (e.g, 1.17+), and then it tests with the latest patch release of each of them as part of my merge gate. Since the go.mod file cannot be relied upon to denote the lower or the upper bound, it really has no bearing on this, except possibly as the default lower bound. Separately, if I am publishing an actual binary as a release artifact, then I should be specifying the Go version (for build reproducibility), but the go.mod should not be considered at all.

One final feature of treating the go version this way is that it would provide a way to fix for loop scoping, as discussed in discussion #56010. If we make that change, older Go toolchains must not assume that they can compile newer Go code successfully just because there are no compiler errors.

Existing versions of the Go compiler already do not fail just because the go directive is newer.

@ianlancetaylor
Copy link
Contributor

if my project says "go 1.13" because that's the minimum, I very likely do not want to use that version of Go for development. A newer toolchain will be faster and behave better when called by tooling like gopls or other analyzers (regardless of the version of Go that gopls is compiled with).

I don't think this is a concern here. This proposal does not say that newer Go versions should download an older Go toolchain. It says that older Go versions should download a newer Go toolchain. When a newer Go version sees an older "go" line in go.mod, it will emulate the language features of the older Go language (that is already true today).

@zikaeroh
Copy link
Contributor

zikaeroh commented Dec 1, 2022

don't think this is a concern here. This proposal does not say that newer Go versions should download an older Go toolchain.

I agree; that was my original interpretation of the proposal. It just seemed like the followups implied otherwise. (That is how pinning tends to work in other languages like node.)

@gopherbot
Copy link

Change https://go.dev/cl/450916 mentions this issue: cmd/go: draft of forward compatibility work

@beoran
Copy link

beoran commented Dec 3, 2022

As others here have explained, this proposal would cause a lot of problems for CI/CD, for package management, for conditionally using newer features in older codebases with conditional compilation, ...

The version of go in go.mod is now a minimum, which is fine. Rather than a magic environment variable, I would add a new command, go upgrade, that can upgrade the go compiler if installed locally from the official packages, but gives a helpful message if one should upgrade using the package manager in stead.

@zikaeroh
Copy link
Contributor

zikaeroh commented Dec 5, 2022

x/tools/gopls only tests that far back because they want to build with what's on people's machines. If what's on people's machines knew how to fetch a newer toolchain then we'd have stopped needing to support older versions long ago.

FWIW I only mentioned x/tools as it's an example of one the Go team maintains that was at the top of my mind. I don't find this particular point to be convincing as while this may be true for gopls specifically as it's an executable tool that users need to run somehow, most other examples are libraries (which are of course going to be built with "what's on people's machines"). If everyone is bumping the go directive in go.mod to gain access to new features, they still may be supporting older versions of Go (especially if they are following Go's own supported version policy).

This also is not exclusive to language features; I recall a series of CLs updating the x repos for lazy loading (#36460), for example CL 316111, which was only possible by bumping the version directive. All of the CLs in that series said:

Note that this does not prevent users with earlier go versions from successfully building packages from this module.

Which to me very much supports the idea that go directive is not a minimum version at all, but a feature unlocker.

(I apparently forgot to actually submit this reply days ago, oops.)

@mark-pictor-csec
Copy link

mark-pictor-csec commented Dec 5, 2022

This seems like a bad idea to me.

Other text file formats include a line identifying the tool version that created the file, and do not imply that older versions cannot process the file properly. So the current behavior is not without precedent. (Examples of these other formats aren't springing to mind, but it wasn't a novel concept when I first saw it in go.mod).

This change assumes that upgrading the toolchain will always be beneficial and regressions will never happen. That stance doesn't seem much different to me than that of a certain OS, where users can be forced to reboot for an update - with no possibility to defer or skip the update. With a language rather than an OS, this behavior may be less infuriating and less disruptive for most users, but I think it's still unacceptable.

If a change is to be made, I would instead deprecate the go keyword and replace it with two, such as recommended_go and minimum_go. The former would have the same behavior as the go keyword currently has, while the latter would cause the toolchain to exit with error. Neither of these keywords would cause silent upgrades, and the names should be less likely to mislead.

If I understand correctly, the minimum_go keyword's behavior should allow for the for loop scoping fix mentioned in the description, as it'd put a lower bound on the toolchain version in use.

@rsc
Copy link
Contributor Author

rsc commented Dec 6, 2022

@mark-pictor-csec I think you just described the actual proposal.

minimum_go is spelled 'go'. It sets the minimum Go toolchain version that must be used to compile the module, whether used directly (via git clone + cd into it + run go commands) or indirectly (as a module dependency).

recommended_go is spelled 'toolchain'. It only applies when code is used directly by cd'ing into the module. It is ignored when the module is used indirectly as a required dependency.

It is not true that the proposal "assumes that upgrading the toolchain will always be beneficial and regressions will never happen." In fact I have gone out of my way in the past to explain why that's false. See for example https://research.swtch.com/vgo-mvs. Instead, the proposal, like Go modules as a whole, puts the user in control of which toolchain they use. There are no gremlins going around updating the Go toolchain behind your back. You only get a new go toolchain when you explicitly make a change to your go.mod file to indicate that. This is the same behavior as other modules: you keep getting the one listed in go.mod, even if there are newer ones, until you explicitly upgrade.

@gopherbot
Copy link

Backport issue(s) opened: #61197 (for 1.19), #61198 (for 1.20).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@gopherbot
Copy link

Change https://go.dev/cl/518415 mentions this issue: cmd/go: do not index std as a module in modcache

gopherbot pushed a commit that referenced this issue Aug 14, 2023
We do not index std as a whole module ever.

When working in the main Go repo, files in package change often,
so we don't want to pay the cost of reindexing all of std when what
we really need is just to reindex strings. Per-package indexing
works better for that case.

When using a released Go toolchain, we don't have to worry about
the whole module changing, but if we switch to whole-module indexing
at that point, we have the potential for bugs that only happen in
released toolchains. Probably not worth the risk.

For similar reasons, we don't index the current work module as
a whole module (individual packages are changing), so we use the heuristic
that we only do whole-module indexing in the module cache.

The new toolchain modules live in the module cache, though, and
our heuristic was causing whole-module indexing for them.
As predicted, enabling whole-module indexing for std when it's
completely untested does in fact lead to bugs (a very minor one).

This CL turns off whole-module indexing for std even when it is
in the module cache, to bring toolchain module behavior back in
line with the other ways to run toolchains.

Updates #57001.
For #61873.

Change-Id: I5012dc713f566846eb4b2848facc7f75bc956eb9
Reviewed-on: https://go-review.googlesource.com/c/go/+/504119
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
(cherry picked from commit a7b1793)
Reviewed-on: https://go-review.googlesource.com/c/go/+/518415
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/519216 mentions this issue: _content/doc: update go.dev/doc/toolchain's backport description

gopherbot pushed a commit to golang/website that referenced this issue Aug 15, 2023
Match the behavior as implemented in CL 518675 and CL 518815,
and ready to be released in the next set of minor releases.

For golang/go#57001.

Change-Id: I4404530994e636173f636aefb83bd64e208eba3c
Reviewed-on: https://go-review.googlesource.com/c/website/+/519216
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
@gaby
Copy link

gaby commented Aug 31, 2023

I don't understand why the default is "auto", all our CI jobs are failing now because they require running go mod tidy before starting anything.

The default should be local.

The ci jobs for running govulncheck are also failing, since go forces you to use the latest go to be able to effectively run govulncheck.

@dmitshur
Copy link
Contributor

@gaby Thanks for reporting, but this is a closed issue and it's not appropriate to investigate here. Both go mod tidy and govulncheck should be working successfully even with the default auto. Given you're seeing different behavior, please open a separate new issue for each problem, and fill in the template with information, as that will help be able to investigate and reproduce it. Thanks.

@gopherbot
Copy link

Change https://go.dev/cl/546635 mentions this issue: cmd/go/internal/toolchain: make a best effort to parse 'go run' and 'go install' flags

gopherbot pushed a commit that referenced this issue Dec 14, 2023
…go install' flags

When the argument to 'go install' or 'go run' looks like a versioned
package, we make a best effort to switch to a toolchain compatible
with the module containing that package, by fetching its go.mod file
and checking the go version it specifies.

At this point in the code, we have not yet parsed the arguments given
on the command line: instead, we just make a best effort to find one
we can use to select a toolchain version. Since that toolchain may be
newer, the command to install it may also include flags that are only
supported by that Go version — and we don't want to fail due to an
error that would be resolved by switching to a more appropriate
toolchain.

So at this point in the code we can't parse the flags in a way that
will surface errors, but we want to make a best effort to parse the
ones that we know about. It turns out that “parse the flags we know
about” is already a familiar problem: that's also what we do in
'go test', so we can reuse the cmdflag library from that to do the
best-effort pass of parsing.

If it turns out that we don't need to switch toolchains after all,
cmd/go's main function will parse the flags again, and will report any
errors at that point.

This fixes a regression, introduced in CL 497879, which caused
'go install -modcacherw pkg@version' to unset the write bit for
directories created while selecting the toolchain to use.

Fixes #64282.
Updates #57001.

Change-Id: Icc409c57858aa15c7d58a97a61964b4bc2560547
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest,gotip-windows-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/546635
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/550055 mentions this issue: [release-branch.go1.21] cmd/go/internal/toolchain: make a best effort to parse 'go run' and 'go install' flags

@gopherbot
Copy link

Change https://go.dev/cl/550237 mentions this issue: cmd/go/internal/toolchain: simplify flag parsing and stop at the first non-flag

@gopherbot
Copy link

Change https://go.dev/cl/551215 mentions this issue: cmd/go/internal/toolchain: revert "make a best effort to parse 'go run' and 'go install' flags"

gopherbot pushed a commit that referenced this issue Dec 19, 2023
…n' and 'go install' flags"

This caused other problems, and for the purposes of the Go 1.22
release, we can just roll back to the Go 1.21 behavior and then
decide in January what the correct path forward is.

Revert of CL 546635.

Unfixes #64282.
Fixes #64738.
For #57001.

This reverts commit e44b8b1.

Change-Id: I78753c76dcd0bc6dbc90caa17f73248c42e5f64a
Reviewed-on: https://go-review.googlesource.com/c/go/+/551215
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Than McIntosh <thanm@google.com>
ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
…go install' flags

When the argument to 'go install' or 'go run' looks like a versioned
package, we make a best effort to switch to a toolchain compatible
with the module containing that package, by fetching its go.mod file
and checking the go version it specifies.

At this point in the code, we have not yet parsed the arguments given
on the command line: instead, we just make a best effort to find one
we can use to select a toolchain version. Since that toolchain may be
newer, the command to install it may also include flags that are only
supported by that Go version — and we don't want to fail due to an
error that would be resolved by switching to a more appropriate
toolchain.

So at this point in the code we can't parse the flags in a way that
will surface errors, but we want to make a best effort to parse the
ones that we know about. It turns out that “parse the flags we know
about” is already a familiar problem: that's also what we do in
'go test', so we can reuse the cmdflag library from that to do the
best-effort pass of parsing.

If it turns out that we don't need to switch toolchains after all,
cmd/go's main function will parse the flags again, and will report any
errors at that point.

This fixes a regression, introduced in CL 497879, which caused
'go install -modcacherw pkg@version' to unset the write bit for
directories created while selecting the toolchain to use.

Fixes golang#64282.
Updates golang#57001.

Change-Id: Icc409c57858aa15c7d58a97a61964b4bc2560547
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest,gotip-windows-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/546635
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
…n' and 'go install' flags"

This caused other problems, and for the purposes of the Go 1.22
release, we can just roll back to the Go 1.21 behavior and then
decide in January what the correct path forward is.

Revert of CL 546635.

Unfixes golang#64282.
Fixes golang#64738.
For golang#57001.

This reverts commit e44b8b1.

Change-Id: I78753c76dcd0bc6dbc90caa17f73248c42e5f64a
Reviewed-on: https://go-review.googlesource.com/c/go/+/551215
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Than McIntosh <thanm@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Accepted
Development

No branches or pull requests