x/tools/go/packages: should it respect GOROOT?

[hugelgupf]

What version of Go are you using (go version)?

many

Does this issue reproduce with the latest release?

yes

What did you do?

Different Go versions have slightly different behaviors in go list or go mod. That's ok.

I work on some stuff that uses x/tools/go/packages to do AST transformations on code before compiling it. x/tools/go/packages is near perfect for us, save for one issue: it does not respect GOROOT.

This often comes up for me because I use go get golang.org/dl/go1.x to test whether compilation works under different versions of Go. Those get installed as go1.13 binaries, for example, and invoking

go1.13 run transformer.go -- ...

does not have the desired effect, because for everything underneath, it still uses go from $PATH, which in my case is 1.15. See exec.Command invocation here.

I've had to replicate some of the stuff from x/tools/go/packages in a development branch in order to use $GOROOT/bin/go instead, and I'm wondering if it'd make sense for x/tools/go/packages to also use $GOROOT/bin/go rather than go from $PATH.

(Apologies for not having a specific example in mind right now -- it's been a long time since I actually worked in this code base, and I'm just now picking it up again. I may be able to give you a better example later.)

cc @heschik @stamblerre

[dmitshur]

I'm not sure if this is sufficient to address the entire issue or just a part of it, but I want to comment on a part of what you said:

This often comes up for me because I use go get golang.org/dl/go1.x to test whether compilation works under different versions of Go. Those get installed as go1.13 binaries, for example, and invoking

go1.13 run transformer.go -- ...

does not have the desired effect, because for everything underneath, it still uses go from $PATH, which in my case is 1.15.

It should have the desired effect. The golang.org/dl/go1.x commands modify PATH, adding the go1.x's GOROOT/bin to the beginning, so that exec.Command("go") does the right thing. You can test with a simple program:

package main

import ("fmt"; "log"; "os/exec")

func main() {
	out, err := exec.Command("go", "version").Output()
	if err != nil { log.Fatalln(err) }
	fmt.Printf("go version = %s", out)
	out, err = exec.Command("go", "env", "GOROOT").Output()
	if err != nil { log.Fatalln(err) }
	fmt.Printf("GOROOT of 'go' = %s", out)
}

$ go run printgover.go 
go version = go version go1.15.1 darwin/amd64
GOROOT of 'go' = /usr/local/go

$ go1.13.7 run printgover.go
go version = go version go1.13.7 darwin/amd64
GOROOT of 'go' = /Users/gopher/sdk/go1.13.7

See relevant code. Is your golang.org/dl/go1.x command up to date enough to include that?

[hugelgupf]

Ah, neat! To be fair, I haven't actually tried it without the $GOROOT/bin/go fix, which I seem to have committed more than 2 years ago. u-root/u-root@e4cad48

What you mention does solve the common go1.x case. I have at times invoked this stuff with a straight up absolute path Go -- e.g. $HOME/golang/bin/go -- when I've messed with the Go tree, but I don't have my Go development tree in my PATH. I'll admit, that's relatively rare, but I feel it should still work with these tools.

[hugelgupf]

If this use case is not very desired, I'm wondering if adding $PATH=$GOROOT/bin:$PATH to packages.Config.Env would work, so perhaps this is solvable on my end without intervention upstream. (How does Linux handle 2 PATH vars in Env? I've never tried...)

[dmitshur]

In general, I think you should definitely arrange things so that exec.LookPath("go") finds the go binary that you intend to use. If you don't do that, some code somewhere (e.g., a dependency you didn't write) that needs to use the go command may end up picking the wrong one.

I think using packages.Config.Env sounds like the right solution. I'll let others comment on whether something can/should be done in go/packages.

How does Linux handle 2 PATH vars in Env? I've never tried...

Usually duplicate environment variables are okay, the latest one wins. You can also de-duplicate them if want (e.g., see envutil.Dedup), but I wouldn't do it unless you have a concrete need.

[dmitshur]

This may be a duplicate of #26845, which was ultimately closed in favor of modifying PATH in environment so the right go binary is found.

[hugelgupf]

I'm fine with closing as a dup. I was asking about deduplication because gocommand.Invocation forces os.Environ on the user in addition to the Env configured, so I can't dedup. But our use case at least has a minimum Go version of 1.13 already, so no concrete need.

Thanks!

[heschi]

forces os.Environ

As Dmitri said, it doesn't; later variables override earlier ones. The problem is that exec.Command doesn't respect PATH when finding the command to run. To fix that we'd have to reimplement exec.LookPath.