cmd/go: for unset GOROOT, try to derive from os.Executable() before runtime.GOROOT() #18678

minux · 2017-01-17T01:30:55Z

Proposal:

For Go 1.9, I'd like to have cmd/go use the new os.Executable APi to find GOROOT automatically. And then we can eliminate all mentions of GOROOT in user facing documents (in fact, we should actively discourage setting GOROOT in user facing documents.)

In fact, I'd like to go so far as to display a warning if the $GOROOT setting contradicts with auto-detected one.

The proposed GOROOT logic is (error handling omitted):

// if the embedded goroot is wrong, then:
gorootAuto = filepath.Dir(filepath.Dir(filepath.EvalSymlinks(os.Executable())))
// issue a warning if gorootAuto is correct (having matching VERSION) and the VERSION file
// is not os.SameFile with $GOROOT/VERSION.
// use $GOROOT if set, otherwise gorootAuto. [*]

Potential problems:

hardlinking bin/go to another location.
moving bin/go to another location.

In both cases, the active fallback step [*] will preserve the current behavior.

distribution changes that don't follow the official GOROOT layout.

In this case, the distribution must already carry patches on the go command, so they will have to adapt the patch if this proposal is accepted.

The text was updated successfully, but these errors were encountered:

davecheney · 2017-01-17T01:33:20Z

I'd sell my firstborn male heir to get rid of GOROOT once and for all.

rsc · 2017-01-23T20:24:11Z

I don't object to this in theory, but I don't understand the exact proposal.

// if the embedded goroot is wrong, then:

Please define "wrong".

rsc · 2017-01-23T20:58:13Z

@davecheney Do you still have to use GOROOT regularly? I don't see why.

davecheney · 2017-01-23T21:08:47Z

I don't, but an incorrectly set GOROOT is by far the most common cause of issue reports that I see on the gobridge forum and the mailing list. Perhaps others can offer supporting evidence from other forums. Five years of telling, begging, and pleading people not to set GOROOT (which they then forget they have done) has proved ineffective. I think the solution is to nullify GOROOT.

…

On Tue, 24 Jan 2017, 07:58 Russ Cox ***@***.***> wrote: @davecheney <https://github.com/davecheney> Do you still have to use GOROOT regularly? I don't see why. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#18678 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAcAyyAkMP_JPVeOMVGkdvKEuKS818Zks5rVRRngaJpZM4LlIpq> .

minux · 2017-01-23T21:43:06Z

Wrong is defined as $GOROOT/VERSION doesn't exist. Perhaps we can additionally require that $GOROOT/VERSION matches runtime.Version(), but I'm not very sure.

rsc · 2017-01-23T22:05:20Z

My $GOROOT/VERSION never exists, since I'm always building from a git checkout. Maybe this should be split into two things:

Make empty $GOROOT fall back to os.Executable before the baked-in runtime.defaultGoroot.
Make go command fail loudly if runtime.Version != $(cat GOROOT/VERSION).

I've wanted each of those separately, and I don't think they necessarily need to be mixed. It sounds like you are proposing:

Instead of making go command fail loudly, override $GOROOT silently.

I think that might not be necessary. If we have 1, then all the people who never set $GOROOT are taken care of, and if we have 2, then that avoids silent misbehavior. Trying to go further in 3 and correct silent misbehavior into silent maybe-correct-but-maybe-still-wrong behavior is where things get tricky.

ianlancetaylor · 2017-01-23T22:15:55Z

I'm concerned that your number 1 (Make empty $GOROOT fall back to os.Executable before the baked-in runtime.defaultGoroot) will break packages that install the go tool in a system-wide bin directory, and expect the baked-in defaultGoroot to let it know where to find the packages.

davecheney · 2017-01-24T00:11:09Z

Make go command fail loudly if runtime.Version != $(cat GOROOT/VERSION).

I tried to implement this in gb about a year ago because gb, but sadly operating system distros ruined the party because they strip files they think aren't important.

constabulary/gb#325 :(

rsc · 2017-01-24T00:17:10Z

Re 1, there is a test in the go command already for identifying a $GOROOT. Something like src/cmd/go/doc.go existing or some other similar file. We could use os.Executable first, but if that doesn't produce a valid GOROOT, then fall back to defaultGoroot.

Re 2, reading VERSION is probably not good enough anyway, but the go command could invoke go tool compile version before any compilation steps.

rsc · 2017-01-30T21:29:51Z

It sounds like we should probably do 1 (use os.Executable for default GOROOT instead of defaultGoroot).

For 2, we'd need to make sure this does not make a noticeable change to compile times. Running

time $(go tool -n compile) -V

in a loop I get times of about 0.050 seconds.

Maybe this issue should be restricted to 1 (what to do when GOROOT is unset).

davecheney · 2017-01-31T11:21:32Z

For 2, could cmd/go provide the value to cmd/compile to avoid the cost of the lookup?

…

On Tue, 31 Jan 2017, 08:29 Russ Cox ***@***.***> wrote: It sounds like we should probably do 1 (use os.Executable for default GOROOT instead of defaultGoroot). For 2, we'd need to make sure this does not make a noticeable change to compile times. Running time $(go tool -n compile) -V in a loop I get times of about 0.050 seconds. Maybe this issue should be restricted to 1 (what to do when GOROOT is unset). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#18678 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAcA2Q_HTWm4wo7BUsC9s3OazIE2k7Cks5rXlZRgaJpZM4LlIpq> .

rsc · 2017-02-13T20:44:55Z

@davecheney Yes, I think so. Filed #19064 for that.

This issue is now down to just changing the default for an unset GOROOT. I've heard no objections to trying os.Executable before falling back to defaultGoroot.

davecheney · 2017-02-13T20:48:47Z

Yes, I think so. Filed #19064 for that.

Thanks @rsc

gopherbot · 2017-05-03T19:02:01Z

CL https://golang.org/cl/42533 mentions this issue.

egonelbre · 2017-05-12T09:19:18Z

I have test failure after CL 42533:

--- FAIL: TestExecutableGOROOT (0.27s)
        go_test.go:3986: C:\Users\Egon\AppData\Local\Temp\gotest815240476\newgoroot\bin\go.exe env GOROOT = "c:\\Go.tip", want "C:\\Go.tip"
FAIL
FAIL    cmd/go  134.329s

Windows paths are still case-insensitive (at least usually).

alexbrainman · 2017-05-12T09:57:19Z

--- FAIL: TestExecutableGOROOT

Issue #20336 ?

Alex

bradfitz · 2017-06-08T22:18:17Z

This was a dup of #17833. Closing that one too.

minux added the Proposal label Jan 17, 2017

minux added this to the Proposal milestone Jan 17, 2017

minux mentioned this issue Jan 17, 2017

README.md: outdated install instructions, GOROOT setting is too prominent #18675

Closed

rsc changed the title ~~Proposal: cmd/go: use os.Executable to find GOROOT if the embedded one is incorrect~~ proposal: cmd/go: use os.Executable to find GOROOT if the embedded one is incorrect Jan 23, 2017

rsc mentioned this issue Feb 13, 2017

cmd/go: pass expected version to compiler #19064

Closed

rsc changed the title ~~proposal: cmd/go: use os.Executable to find GOROOT if the embedded one is incorrect~~ cmd/go: for unset GOROOT, try to derive from os.Executable() before runtime.GOROOT() Feb 13, 2017

rsc added Proposal-Accepted and removed Proposal labels Feb 13, 2017

rsc modified the milestones: Go1.9, Proposal Feb 13, 2017

crawshaw mentioned this issue Feb 22, 2017

cmd/go: use os.Executable and drop GOROOT instructions #19232

Closed

gopherbot closed this as completed in b8c7fdd May 4, 2017

bradfitz mentioned this issue Jun 8, 2017

cmd/go: use os.Executable to find GOROOT #17833

Closed

markus-oberhumer mentioned this issue Jul 13, 2017

cmd/go: still need to set GOROOT with go1.9beta2 #20997

Closed

rhcarvalho mentioned this issue Apr 23, 2018

doc: setting GOROOT is not necessary when installing to custom location #25002

Closed

golang locked and limited conversation to collaborators Jun 8, 2018

gopherbot added the FrozenDueToAge label Jun 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/go: for unset GOROOT, try to derive from os.Executable() before runtime.GOROOT() #18678

cmd/go: for unset GOROOT, try to derive from os.Executable() before runtime.GOROOT() #18678

minux commented Jan 17, 2017

davecheney commented Jan 17, 2017

rsc commented Jan 23, 2017

rsc commented Jan 23, 2017

davecheney commented Jan 23, 2017 via email

minux commented Jan 23, 2017 via email

rsc commented Jan 23, 2017

ianlancetaylor commented Jan 23, 2017

davecheney commented Jan 24, 2017

rsc commented Jan 24, 2017

rsc commented Jan 30, 2017

davecheney commented Jan 31, 2017 via email

rsc commented Feb 13, 2017

davecheney commented Feb 13, 2017

gopherbot commented May 3, 2017

egonelbre commented May 12, 2017

alexbrainman commented May 12, 2017

bradfitz commented Jun 8, 2017

cmd/go: for unset GOROOT, try to derive from os.Executable() before runtime.GOROOT() #18678

cmd/go: for unset GOROOT, try to derive from os.Executable() before runtime.GOROOT() #18678

Comments

minux commented Jan 17, 2017

davecheney commented Jan 17, 2017

rsc commented Jan 23, 2017

rsc commented Jan 23, 2017

davecheney commented Jan 23, 2017 via email

minux commented Jan 23, 2017 via email

rsc commented Jan 23, 2017

ianlancetaylor commented Jan 23, 2017

davecheney commented Jan 24, 2017

rsc commented Jan 24, 2017

rsc commented Jan 30, 2017

davecheney commented Jan 31, 2017 via email

rsc commented Feb 13, 2017

davecheney commented Feb 13, 2017

gopherbot commented May 3, 2017

egonelbre commented May 12, 2017

alexbrainman commented May 12, 2017

bradfitz commented Jun 8, 2017