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.

Sign up for GitHub

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

Jump to bottom

runtime: bad GOROOT in 1.9rc1 #21313

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

runtime: bad GOROOT in 1.9rc1 #21313

karalabe opened this issue Aug 4, 2017 · 12 comments
Labels
FrozenDueToAge release-blocker
Milestone
Go1.10

Comments

@karalabe
Copy link
Contributor

karalabe commented Aug 4, 2017 •
edited

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
Copy link
Contributor Author

karalabe 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
Copy link
Contributor

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 mentioned this issue Aug 4, 2017
Closed
@fjl
Copy link

fjl 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
Copy link
Contributor

@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.

@ALTree ALTree mentioned this issue Aug 11, 2017
Closed
@crvv
Copy link
Contributor

crvv 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.

This was referenced Aug 25, 2017
Merged
Closed
tmthrgd added a commit to tmthrgd/go-bindata that referenced this issue Aug 25, 2017
@tmthrgd
Travis CI workaround for go1.9 GOROOT bug (see golang/go#21313)
f75b10b
@kevinburke
Copy link
Contributor

kevinburke commented Aug 25, 2017 •
edited

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
Copy link

pierrre commented Aug 25, 2017 •
edited

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

@kevinburke
Copy link
Contributor

Ah, you are correct, thanks!

@AlekSi
Copy link
Contributor

AlekSi commented Aug 30, 2017

New gimme was deployed to Travis-CI.

@crawshaw
Copy link
Member

crawshaw commented Sep 3, 2017 •
edited

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
Copy link
Member

crawshaw 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
Copy link

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

fsouza pushed a commit to nytimes/encoding-wrapper that referenced this issue Sep 5, 2017
travis: remove GOROOT hack
36fb9db 
It looks like gimme fixed it.

Issue being tracked on golang/go#21313.
@rsc rsc mentioned this issue Oct 5, 2017
Closed
@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.
Assignees
No one assigned
Labels
FrozenDueToAge release-blocker
Projects
None yet
Milestone
Go1.10
Development

No branches or pull requests
9 participants
@fjl @AlekSi @karalabe @pierrre @crawshaw @kevinburke @ianlancetaylor @crvv @gopherbot