cmd/go: "go install" of std/cmd in a different GOROOT silently does the wrong thing

[mvdan]

$ pwd
/home/mvdan/tip/src/cmd/gofmt
$ go version
go version devel go1.17-8e91458b19 Fri Apr 30 20:00:36 2021 +0000 linux/amd64
$ go install
$ which gofmt
/home/mvdan/tip/bin/gofmt
$ /usr/bin/go version
go version go1.16.3 linux/amd64
$ /usr/bin/go install
$ go version $(which gofmt)
/home/mvdan/tip/bin/gofmt: devel go1.17-8e91458b19 Fri Apr 30 20:00:36 2021 +0000

Note that Go 1.16.3 happily does nothing when it's told to go install cmd/gofmt from tip, even though that should most definitely error. From the last command, you can see that the gofmt I'm using has not changed. The gofmt from /usr/bin can't have been changed either, as I'm not root.

I think I get why this is happening. /usr/bin/go install turns into /usr/bin/go install cmd/gofmt, so that ends up installing 1.16.3's cmd/gofmt, which is already up to date - hence nothing to do.

But that feels wrong. If I'm installing the package at the current directory, and this is a package under a different GOROOT than the toolchain's, that should simply fail very early on.

This caused me confusion on https://go-review.googlesource.com/c/go/+/284139. After a rebase on a cmd/gofmt change, I ran go install and it finished incredibly quickly, even though I knew I had likely made a mistake. Plus, I would expect the first build to take about a second, as I'd need a new binary for sure. It was five minutes later that I realised I had forgot to re-run make.bash earlier in the day, so my go binary was falling back to the system's 1.16.3, rather than tip.

cc @bcmills @matloob

[mvdan]

Also, there isn't a gofmt in my GOBIN either.

[mvdan]

I've also seen a very similar pattern with new contributors to Go, where they would do the equivalent of:

$ go1.16 version
go version go1.16.3 linux/amd64
$ [clone and build Go tip]
$ cd tip/src
$ go1.16 test ./encoding/json

That should fail with a very clear message, like "you're using the Go toolchain at /usr/lib/go with the GOROOT at /home/user/tip".

[cherrymui]

cc @bcmills @jayconrod @matloob

[cherrymui]

I had forgot to re-run make.bash earlier in the day, so my go binary was falling back to the system's 1.16.3, rather than tip.

I'm confused by this. Why re-running make.bash does anything with what "go" binary it is on your PATH?

[mvdan]

I'm confused by this. Why re-running make.bash does anything with what "go" binary it is on your PATH?

PATH=$HOME/.bin:$HOME/tip/bin:$HOME/go/bin:$PATH

That is, go from tip takes precedence over the system/s in /usr/bin, if it exists.

That's not the issue at hand, though. The problem is that using the go executable from GOROOT "foo" in GOROOT "bar" silently does weird things.

[cherrymui]

You delete the go binary in GOROOT/bin/go before rerunning make.bash? (I agree this is a valid thing to do. I just never done it, so the "go" binary still there, although may be stale.)

[mvdan]

Yes. I essentially do a full git clean whenever I pull the latest master, and shortly after I do a make.bash. Except the time above, when I forgot the extra step :)

[bcmills]

I put the current funky semantics in place for #30756 — @josharian explicitly wanted to be able to run commands while in a different GOROOT, and that was what I could come up with that seemed to fit his existing usage without being too semantically odd (see #30756 (comment)).

That said, this behavior affects a very small number of developers, most of whom are Go contributors, so if we think something else would be clearer (and not break anybody's workflow too badly), I'm happy to adjust it.

[bcmills]

Ah, but the funky semantics only really work for module std. For module cmd there are no path-prefix shenanigans, so the import path really does collide with the cmd/gofmt in the real GOROOT/src/cmd/gofmt.

I think that makes this particular example a duplicate of #35270?

[mvdan]

I don't think this is the same as #30756. Running Go from GOROOT_A in GOROOT_B, I think it's fine for go build bytes to refer to GOROOT_A/src/bytes. But cd GOROOT_B/src/bytes; go build . should error, as I really am referring to the package in the current directory, not the global "bytes".

Similarly, in my example above I was doing a go install, AKA go install ., not a go install cmd/gofmt.

I think that makes this particular example a duplicate of #35270?

I don't mind if this issue is closed as a duplicate, as long as we agree on the issue and that it needs a fix :) I couldn't find a previous issue about it.

[bcmills]

But cd GOROOT_B/src/bytes; go build . should error, as I really am referring to the package in the current directory, not the global "bytes".

Based on the semantics in #30756, cd GOROOT_B/src/bytes; go build . should succeed, and should build the package in the current directory as the package with path std/bytes (not bytes), which doesn't collide with the standard library because std/bytes is not normally a valid package path.

Similarly, in my example above I was doing a go install, AKA go install ., not a go install cmd/gofmt.

Ah, but cmd is different because it does have a path prefix, and that path prefix does collide with the standard library. cmd/gofmt really does collide.

[bcmills]

(To be clear, I would also be ok with changing the semantics to diverge from #30756, if we can come to a consensus about what the actual behavior ought to be.)

[mvdan]

I honestly don't mind what the final semantics are, as long as running go install of relative paths inside GOROOT_B doesn't silently use packages from GOROOT_A. The source of my confusion was not seeing a new binary or an error when running go install in those scenarios.

I'd slightly lean towards making both std and cmd cases error, because I struggle to imagine why building a std/bytes package would ever be a useful thing. It's also a common thing amongst Go newcomers that they build Go from master but try to go test one of its std packages with a different go command version. It can be an easy thing to miss, given that $PATH can be confusing to inexperienced people. It would be very useful if that scenario simply errored with a useful message right away.