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

runtime: bad GOROOT in 1.9rc1 #21313

Closed
karalabe opened this issue Aug 4, 2017 · 12 comments

Comments

Projects
None yet
9 participants
@karalabe
Copy link
Contributor

commented Aug 4, 2017

If I unpack Go 1.8.3 to a custom location, I either need to set the correct GOROOT, or it will fail to start.

$ GOROOT=/tmp/go1.8.3 /tmp/go1.8.3/bin/go version
go version go1.8.3 linux/amd64

$ /tmp/go1.8.3/bin/go version
go: cannot find GOROOT directory: /usr/local/go

Go 1.9rc1 does not have this "limitation" any more and even correctly detects its custom GOROOT:

$ /tmp/go1.9rc1/bin/go version
go version go1.9rc1 linux/amd64

$ /tmp/go1.9rc1/bin/go env
[...]
GOROOT="/tmp/go1.9rc1"
[...]

However, the runtime.GOROOT() method still defaults to the GOROOT environmental variable (with the build-path fallback if the env var cannot be found). This is a problem, because it produces runtime issues on systems where Go is not located at the correct path (such as currently Travis).

package main

import (
	"fmt"
	"runtime"
)

func main() {
	fmt.Println(runtime.GOROOT())
}
$ /tmp/go1.9rc1/bin/go run main.go 
/usr/local/go
@karalabe

This comment has been minimized.

Copy link
Contributor Author

commented Aug 4, 2017

We could argue that it works as documented, but until now this scenario was impossible because Go refused to start altogether.

@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone Aug 4, 2017

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2017

I've marked this to be fixed in 1.10. Basically I think it's going to be an unfortunate problem in 1.9.

@karalabe karalabe referenced this issue Aug 4, 2017

Closed

update gimme #8177

@fjl

This comment has been minimized.

Copy link

commented Aug 8, 2017

It would be possible to fix this by setting GOROOT in cmd/go for go run. Maybe that's simple enough for go 1.9?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2017

@fjl But that would only fix go run, so we would still be in an inconsistent state for go build or go install. I don't see enough benefit to that to change 1.9 at this point.

@crvv

This comment has been minimized.

Copy link
Contributor

commented Aug 18, 2017

Maybe only fixing go run is enough.
After go build or go install, the consistent state between runtime.GOROOT() and $GOROOT can't be guaranteed. The $GOROOT can be deleted after go build.
Besides, runtime.GOROOT() is read from /src/runtime/internal/sys/zversion.go, which is generated during toolchain bootstrapping. I guess fixing this issue for go build is not trivial.

tmthrgd added a commit to tmthrgd/go-bindata that referenced this issue Aug 25, 2017

@kevinburke

This comment has been minimized.

Copy link
Contributor

commented Aug 25, 2017

FYI - Travis CI is a build tool, and normally you'd add Go 1.9 in the test matrix as a line in the file like this:

- 1.7
- 1.8
- 1.9

The last line currently fails. Users have to (first) discover the problem - compiling packages in the standard library fails - and then Google around until they find the solution, which is to add these additional lines to the configuration:

before_install:
  - export GOROOT=$(go env GOROOT)

It's possible Travis can do some custom work on their end. But at the moment this is going to be a problem for people trying to set up CI for their Go packages.

@pierrre

This comment has been minimized.

Copy link

commented Aug 25, 2017

See travis-ci/travis-build#1148 + travis-ci/travis-ci#8177
It's fixed in recent gimme versions.

@kevinburke

This comment has been minimized.

Copy link
Contributor

commented Aug 25, 2017

Ah, you are correct, thanks!

@AlekSi

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2017

New gimme was deployed to Travis-CI.

@crawshaw

This comment has been minimized.

Copy link
Contributor

commented Sep 3, 2017

Setting the value of runtime.GOROOT() didn't occur to me when working on https://golang.org/cl/42533.

As cmd/go may be installed system-wide and the user may not have permission to modify files, we can't use a .go source file to embed its computed version of GOROOT.

How about passing the value to the linker as -X=runtime.GOROOT=/path, and having the runtime package look for the string?

I'm happy to try implementing this.

@crawshaw

This comment has been minimized.

Copy link
Contributor

commented Sep 3, 2017

In fact, if we convert runtime/internal/sys.DefaultGoroot from a const to a var, then the only other change is having cmd/go pass -X runtime/internal/sys/DefaultGoroot=/path to the linker.

@gopherbot

This comment has been minimized.

Copy link

commented Sep 3, 2017

Change https://golang.org/cl/61310 mentions this issue: cmd/go: put computed GOROOT in built binaries

fsouza added a commit to nytimes/encoding-wrapper that referenced this issue Sep 5, 2017

travis: remove GOROOT hack
It looks like gimme fixed it.

Issue being tracked on golang/go#21313.

@gopherbot gopherbot closed this in 06f4d93 Sep 9, 2017

@golang golang locked and limited conversation to collaborators Sep 9, 2018

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