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

proposal: cmd/go: add GOCMD environment variable #28043

Closed
jimmyfrasche opened this issue Oct 5, 2018 · 20 comments
Closed

proposal: cmd/go: add GOCMD environment variable #28043

jimmyfrasche opened this issue Oct 5, 2018 · 20 comments

Comments

@jimmyfrasche
Copy link
Member

Tools for interacting with go code need to exec the go command. Sometimes there are multiple versions of the go command available, rarely themselves named "go": go, vgo, go1.1xbetax, go1.1x, etc.

Tools that exec "go" cannot use these without $PATH manipulation and shell scripts named "go" that forward to the desired go command. Possible, but awkward.

This should be the convention:

goBin := os.Getenv("GOCMD")
if goBin == "" {
  goBin = "go"
}
// exec goBin

If this environment variable were not known to the go command itself, this would be required: GOCMD=go1.12beta1 go1.12beta1 generate But the go command can always set GOCMD to itself.

By showing up in $GOCMD env and $GOCMD help environment, the convention is official, encouraging all tools that need to exec go to follow the convention.

If it is official, go/packages can use it making the majority of tooling just work (once ported over to using go/packages, at least) since it greatly reduces the need to manually exec the go command.

Previous discussion in #26845

cc: @rsc @bcmills @alandonovan @ianthehat @marwan-at-work

@jimmyfrasche jimmyfrasche added this to the Proposal milestone Oct 5, 2018
@alexbrainman
Copy link
Member

Please, not another $GO... environment variable for everyone to remember. How many users are out there that have more than one go.exe command in their PATH? Why should we support them?

Alex

@jimmyfrasche
Copy link
Member Author

It's true that it's one more thing for most people to skip over in the docs.

The people with multiple go commands are largely the ones testing/developing future versions of Go, running CI/CD against multiple versions of Go, or writing the tools that everyone uses.

Moreover, the go commands that are not named go largely come from the Go project itself to try to make it simpler to help test changes. It would be nice to also have a simple way to access tools while doing that, which in turn makes it more likely for a regression in a tool to be found

@alexbrainman
Copy link
Member

The people with multiple go commands are largely the ones testing/developing future versions of Go, running CI/CD against multiple versions of Go, or writing the tools that everyone uses.

That is me - I develop / debug Go. I do not need new GOCMD environment variable. I am doing just fine as is.

Having new GOCMD environment variable is not free. Someone would havve to document it, and keep documentation updated. Someone would have to educate users about GOCMD variable. Someone would have to debug problems related to GOCMD environment variable. And so on ...

Alex

@jimmyfrasche
Copy link
Member Author

Everything you say is true. But I still think this is worth it.

This solves a lot of issues transparently and when it doesn't it makes them much more easily solvable than they are currently. And I think these issues are going to become more common as go/packages becomes more common as more tools are going to be execing go in the background (though they should have been doing so anyway).

Even if that's not a problem for you, a frequent contributor for a long time, or for me, an occasional contributor for a much shorter time, it's going to be a problem for the user who decided to go the extra mile and try a beta for the first time and can't figure out why stringer is saying that there's no such type when they ran go1.XXbetaY generate because it's type checking the file off the wrong copy of the stdlib.

There's also a cost to losing eyes and to having to explain the current workarounds.

@alexbrainman
Copy link
Member

This solves a lot of issues

I really don't understand the problem you are trying to solve. Perhaps that is why I do not like your proposal. But you do not need to convince me, maybe others think differently.

Alex

@randall77
Copy link
Contributor

Note that internally we have a package called internal/testenv which does this sort of thing for internal tests. We use it when we need to recursively invoke go with modified command-line options (like adding -gcflags=-S to print assembly output). It basically looks for a go binary at runtime.GOROOT()/bin/go. @jimmyfrasche Would that work for your use case? If not, why not?

@jimmyfrasche
Copy link
Member Author

Tooling needs to exec the go command, to get information from go env, go list, etc.

go/packages, which is planned to be the de facto way to write tools, execs the go command on one's behalf to get the packages to load from go list, for example. Even with that, other tools may need to exec go to get information that go/packages doesn't provide.

If they always invoke the literal string "go", and you're working in a project using go1.XX, it's going to invoke the wrong go command. That may or may not work. When it works, it does so by coincidence. If you want to use a tool that execs go when working with a non-standard go command you need to write a script named go that calls the appropriate go command and put that in your path ahead of the standard go command. Doable but not simple. If you don't do that, you lose access to a lot of tools.

Working on Go itself is simpler because you can just change your path. The go command at tip is still named go.

The tests for the go command can make an assumption that their runtime.GOROOT is correct. In general the only way for an external tool to reliably find the go root is to exec go env GOROOT so there's a bootstrapping issue.

When I was trying out modules with vgo, I lost a lot of tools that were unaware of module support. Some of those would have worked just fine if they'd known to exec vgo list -json instead of go list -json, however.

The same problem comes up with the beta releases and the go1.XX releases. There are a lot of go commands not named "go".

@mvdan
Copy link
Member

mvdan commented Oct 6, 2018

It seems to me like $PATH should be enough to make this work, even for the more complex cases. Why are you finding that awkward?

That is, both of those should be equal:

GOCMD=/some/bin/go go build ...
PATH=/some/bin:$PATH go build ...

@jimmyfrasche
Copy link
Member Author

@mvdan $PATH is fine, if a bit awkward, when the alternate go command is named go.

It's more involved when it's not named go. If you grab a beta the binary is already going to be in the path but it's going to be named something like go1.XXbetaY.

To use that with tools that exec "go", you need to write a script named "go" that runs that particular go command and add that to your path. If you have multiple alternate go commands you need to do this for each. It looks more like

PATH=/some-dir/alternate-go-commands/use-go1.12beta1:$PATH some-tool-that-execs-go-list

where use-go1.12beta1 has a single file named go containing

#!/bin/sh
go1.12beta1 $*

That's a lot of bookkeeping for what's supposed to be the "easily try things out" version.

You don't have to do that to use the go1.12beta1 command itself, but you do need to do it if you want to use any tool that execs the go command.

Say you're writing a small program to try out a new function in the next version of the stdlib. You want to run you favorite linter. It involves type checking. It's going to fail because it execs go list -json not go1.12beta1 list -json. It typechecks against the 1.11 stdlib which doesn't have that new function yet.

That linter could support the $GOCMD env var without being in the go command itself.

But the same problem comes up if you want to use generate and the tool that does the generation execs go. You have to stutter and do

GOCMD=go1.12beta1 go1.12beta1 generate

but go1.12beta1 could have transparently set $GOCMD here.

The major benefit of having it be an official environment variable is standardization. That avoids having to sometimes set $GOCMD, sometimes $GOTOOL, sometimes $ALTGO, sometimes needing to create a script and mess with path, etc. It also lets tool authors know that something is needed here. Probably most importantly, it means that go/packages can use it. If that becomes the standard for writing tools, then most tools would support this use case without having to do anything.

@bcmills
Copy link
Contributor

bcmills commented Oct 8, 2018

@jimmyfrasche, it seems like we could address your go generate use-case by having the golang.org/x/build/version wrapper scripts add GOROOT/bin to the beginning of the PATH before invoking the named subcommand. Edit by @dmitshur: This has been done via issue #28103 and CL 143545.

For other cases, go env GOROOT should make editing PATH yourself pretty straightforward. Consider something like:

PATH=$(go1.12beta1 env GOROOT)/bin:$PATH some-tool-that-execs-go-list

@bcmills
Copy link
Contributor

bcmills commented Oct 8, 2018

The major benefit of having it be an official environment variable is standardization.

Yes: that's arguably why PATH is the best approach. It is already the standard today. 🙂

@jimmyfrasche
Copy link
Member Author

@bcmills That still wouldn't work with something like vgo, but that would go pretty far.

@bcmills
Copy link
Contributor

bcmills commented Oct 9, 2018

See also #26845.

@alandonovan
Copy link
Contributor

I agree with Bryan that there are ways to address this without new mechanisms; you could even run vgo if you put a link to it named go in a new directory on your PATH. We're nearly rid of the need for both GOROOT and GOPATH in the environment; let's not go backwards.

@hajimehoshi
Copy link
Member

If a library depends on go/packages, which calls go list internally, I'd need to switch to use go instead of vgo or go1.13rc1.

I was wondering if I'd need to care a library and all its dependencies can call go command. It doesn't sound good if I need to care implementation details. Or, should I always use go command in any cases?

@dmitshur
Copy link
Contributor

dmitshur commented Aug 24, 2019

@hajimehoshi When using a wrapper command around go that has a different name, it's that wrapper command's responsibility to update PATH variable before invoking the real go command such that programs find the correct version of the go binary. All of the golang.org/dl/... wrapper commands do this.

For example:

$ cat main.go
package main

import "fmt"
import "os/exec"

func main() {
	path, err := exec.LookPath("go")
	if err != nil {
		panic(err)
	}
	fmt.Println(path)

	out, err := exec.Command("go", "version").CombinedOutput()
	if err != nil {
		panic(err)
	}
	fmt.Print(string(out))
}

$ go run main.go
/usr/local/go/bin/go
go version go1.12.9 darwin/amd64

$ go1.13rc1 run main.go
/Users/dmitri/sdk/go1.13rc1/bin/go
go version go1.13rc1 darwin/amd64

That means when using something like go1.13rc1, you don't need to care about implementation details of libraries or dependencies. They will find the appropriate version of the "go" binary.

@hajimehoshi
Copy link
Member

Hi,

OK, I understood there is a rule that 1) the binary name must be go and 2) the binary must be in $PATH, and vgo and go1.13rc1 should have a hack to obey this rule. Thank you!

Then I have another question: can we get a warning or an error when we violate this rule? For example, I did investigation with my own-built go like ~/go-code/bin/go run foo.go and foo.go used go/packages. It took a little time that go/packages invoked a go in $PATH instead of my own go. Aborting might be hard due to backward compatibility, but I'd want a way to know whether we obey the rule or not.

@dmitshur
Copy link
Contributor

dmitshur commented Aug 24, 2019

Then I have another question: can we get a warning or an error when we violate this rule?

I don't think it's possible to implement such a warning or error without false positives, at least not in the general case.

Imagine if instead of ~/go-code/bin/go run foo.go you did ~/go-code/bin/go build foo.go, then you copied the resulting foo binary to another computer. The foo binary will use whatever "go" binary it can find on the new computer, which may not be the same version as your local ~/go-code/bin/go binary.

As long as go/packages relies on finding a "go" binary on your computer, you need to be aware of that when you're doing local development involving go/packages and custom builds of go.

The only remaining idea I have is to consider modifying go run behavior to automatically modify PATH so that the same "go" binary is found. But that feels magical and can cause very surprising and unexpected behavior, so I think it's better not to do that.

@hajimehoshi
Copy link
Member

I understood. Thank you for explaining!

@gopherbot
Copy link

Change https://golang.org/cl/214898 mentions this issue: cmd: replace goBin() with a plain string "go"

gopherbot pushed a commit to golang/mobile that referenced this issue Jan 15, 2020
This was introduced at https://go-review.googlesource.com/c/mobile/+/191518
originally, but this change was against the decision at
golang/go#26845.

"go" always works even when Go command that name is not "go", like
"go1.14beta1" is used. See also the discussion at golang/go#28043

Change-Id: Ifebe969edaeda0373b2840d25a4f4030509176fa
Reviewed-on: https://go-review.googlesource.com/c/mobile/+/214898
Run-TryBot: Hajime Hoshi <hajimehoshi@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
hajimehoshi added a commit to hajimehoshi/wasmserve that referenced this issue Feb 9, 2020
See also golang/go#26845.

"go" always works even when Go command that name is not "go", like
"go1.14beta1" is used. See also the discussion at golang/go#28043.
andydotxyz pushed a commit to fyne-io/fyne that referenced this issue Mar 5, 2020
This was introduced at https://go-review.googlesource.com/c/mobile/+/191518
originally, but this change was against the decision at
golang/go#26845.

"go" always works even when Go command that name is not "go", like
"go1.14beta1" is used. See also the discussion at golang/go#28043

Cherry picked from github.com/golang/mobile
andydotxyz pushed a commit to fyne-io/fyne that referenced this issue Apr 5, 2020
This was introduced at https://go-review.googlesource.com/c/mobile/+/191518
originally, but this change was against the decision at
golang/go#26845.

"go" always works even when Go command that name is not "go", like
"go1.14beta1" is used. See also the discussion at golang/go#28043

Cherry picked from github.com/golang/mobile
@golang golang locked and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants