x/review/git-codereview: when working on the Go repo, use its bin/gofmt if present

[FiloSottile]

gofmt changed in Go 1.11, and git-codereview uses the gofmt from $PATH, so when that's the Go 1.10 version, it complains as git-codereview: gofmt needs to format these files (run 'git gofmt').

It should instead use the gofmt in GOROOT.

[gopherbot]

Change https://golang.org/cl/130695 mentions this issue: git-codereview: add -g flag to the gomft command

[josharian]

It should instead use the gofmt in GOROOT.

This is not obvious to me. Why is GOROOT better than PATH in this context?

We go to pains to tell people not to set the GOROOT env var, as it is a source of significant confusion. (This is one of @davecheney's favorites.) And tools that memorize the toolchain that they were built with are really frustrating.

It seems to me that using the gofmt in your PATH is the best available solution. Or at most fixing #27166 and then using the results of that.

[agnivade]

Seems like this needs further discussion. Re-labelled appropriately.

[mvdan]

We go to pains to tell people not to set the GOROOT env var

I presume @FiloSottile meant something like go env GOROOT or runtime.GOROOT(), both of which fall back to automatically detecting the directory if unset.

Though I generally agree with Josh. We tell people that to "select" which Go version should be used, they should modify PATH so that go executes the version they want. Why should gofmt be different?

[FiloSottile]

git-codereview implies you are submitting code to the repository you are working on. If that repository is the go tree, you should use the gofmt that come with it.

[mvdan]

Right, so this would only apply to the Go repo itself, using that clone's bin/gofmt. If special-casing that in git-codereview is fine, that seems reasonable. I think the original issue title is a bit confusing when it talks about GOROOT.

[FiloSottile]

Agreed, that was the poorly worded intention.