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 · 14 comments

Comments

Projects
None yet
6 participants
@jimmyfrasche
Copy link
Member

commented Oct 5, 2018

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

This comment has been minimized.

Copy link
Member

commented Oct 5, 2018

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

This comment has been minimized.

Copy link
Member Author

commented Oct 5, 2018

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

This comment has been minimized.

Copy link
Member

commented Oct 5, 2018

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

This comment has been minimized.

Copy link
Member Author

commented Oct 5, 2018

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

This comment has been minimized.

Copy link
Member

commented Oct 5, 2018

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

This comment has been minimized.

Copy link
Contributor

commented Oct 5, 2018

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

This comment has been minimized.

Copy link
Member Author

commented Oct 5, 2018

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member Author

commented Oct 6, 2018

@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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member Author

commented Oct 8, 2018

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

@bcmills

This comment has been minimized.

Copy link
Member

commented Oct 9, 2018

See also #26845.

@alandonovan

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.