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

tools: warn users if go.goroot is configured - it mutates GOROOT env var #1501

Closed
TheJonRobinson opened this issue May 13, 2021 · 7 comments
Closed
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@TheJonRobinson
Copy link

What version of Go, VS Code & VS Code Go extension are you using?

  • Run go version to get version of Go from the VS Code integrated terminal.
    • go version go1.13.4 darwin/amd64
  • Run gopls -v version to get version of Gopls from the VS Code integrated terminal.
    • command not found: gopls
  • Run code -v or code-insiders -v to get version of VS Code or VS Code Insiders.
    • 1.56.1 e713fe9b05fc24facbec8f34fb1017133858842b x64
  • Check your installed extensions to get the version of the VS Code Go extension
    • 0.25.0
  • Run Ctrl+Shift+P (Cmd+Shift+P on Mac OS) > Go: Locate Configured Go Tools command.
Checking configured tools....
GOBIN: undefined
toolsGopath: 
gopath: undefined
GOROOT: /usr/local/go/bin/go
PATH: /Users/jonrobinson/google-cloud-sdk/bin://bin:/Users/jonrobinson/.nvm/versions/node/v12.16.1/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/go/bin

failed to run "/usr/local/go/bin/go version": Error: Command failed: /usr/local/go/bin/go version
go: cannot find GOROOT directory: /usr/local/go/bin/go
 cwd: /Volumes/Development/game-pizzarush

Share the Go related settings you have added/edited

Run Preferences: Open Settings (JSON) command to open your settings.json file.
Share all the settings with the go. or ["go"] or gopls prefixes.

    "go.formatTool": "goimports",
    "[go]": {
        "editor.insertSpaces": false,
        "editor.formatOnSave": true,
        "editor.codeActionsOnSave": {
            "source.organizeImports": true
        }
    },
    "go.useLanguageServer": true,
    "go.toolsManagement.autoUpdate": true,

Describe the bug

A clear and concise description of what the bug.

We have been happily using 0.24.2 on the extension, but after the update happened to 0.25.0, we get the following pop-up when opening the VS-Code project:

Failed to run '/usr/local/go/bin/go env. The config change may not be applied correctly.
Source: Go (Extension)

Also, in the OUTPUT window:

Failed to run '/usr/local/go/bin/go env' (cwd: /Volumes/Development/game-pizzarush): Error: Command failed: /usr/local/go/bin/go env -json GOPATH GOROOT GOPROXY GOBIN GOMODCACHE
go: cannot find GOROOT directory: /usr/local/go/bin/go

go: cannot find GOROOT directory: /usr/local/go/bin/go

After this, gopls is not present/working and the go environment seems somewhat broken.

NB If we revert the extension back to the previous one, the problems go away.

A clear and concise description of what you expected to happen.

We'd expect the behaviour to remain as before the 0.25.0 update.

Steps to reproduce the behavior:

  1. After the Go extension has updated to 0.25.0
  2. Open the workspace in VS-Code
  3. See error
@gopherbot gopherbot added this to the Untriaged milestone May 13, 2021
@hyangah
Copy link
Contributor

hyangah commented May 13, 2021

Thanks for the report @TheJonRobinson
It's probably related to the commit but I thought that should change behavior only if go.goroot setting is used. It doesn't seem like you are using it though. Is the workspace setting empty?
(Preferences: Open Workspace Settings (JSON))

@hyangah hyangah added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label May 13, 2021
@bestbeforetoday
Copy link

I also started seeing this issue with the update to v0.25.0 of the Go extension. I checked my VS Code settings and there was a go.goroot entry there. This isn't something I recall ever setting, and isn't something that was causing an issue before. Removing the go.goroot entry in my VS Code settings seems to have resolved the startup error.

@TheJonRobinson
Copy link
Author

TheJonRobinson commented May 14, 2021 via email

@hyangah
Copy link
Contributor

hyangah commented May 14, 2021

Long story - For many years go.goroot was used to set the GOROOT environment variable. Setting GOROOT environment variable is no longer recommended and in fact, probably against the best practice. Modern versions of Go find the right GOROOT by themselves when GOROOT isn't set. So, we cleaned up its behavior as part of #146 and made that not to set GOROOT environment variable any more.

Unfortunately, there are still some popular version management tools that were written long ago and somehow depends on setting GOROOT environment variable to operate correctly. And there were users who want to unset/override GOROOT environment variable set in their environment. We received multiple requests to bring back the old behavior (#776 for example). We considered completely deprecating go.goroot and recommending users to use go.toolsEnvVars when they need to modify with the go's environment variables. But there was usability & security implication. So, instead we brought back go.goroot's old behavior.

Most users shouldn't set go.goroot setting at all.

We can add extra warnings when go.goroot setting is detected.

@findleyr findleyr added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels May 14, 2021
@findleyr findleyr modified the milestones: Untriaged, Backlog May 14, 2021
@hyangah hyangah changed the title Updating to 0.25.0 from 0.24.2 causes 'Failed to run /usr/local/go/bin/go env' error tools: warn users if go.goroot is configured - it mutates GOROOT env var May 14, 2021
@hyangah hyangah added the NeedsFix The path to resolution is known, but the work has not been done. label May 14, 2021
@joelft
Copy link

joelft commented May 17, 2021

Just for the sake of thoroughness for any googlers landing here, make sure to also check WSL settings if using VS Code Remote to program in WSL.

In my case, that's where the stale go.root setting was located.

@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/320429 mentions this issue: src/goMain: warn users if go.goroot is set

@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/320549 mentions this issue: [release] src/goMain: warn users if go.goroot is set

@hyangah hyangah modified the milestones: Backlog, v0.25.1 May 21, 2021
gopherbot pushed a commit that referenced this issue May 21, 2021
Warn users if this is an obviously invalid value (binary).
Show information popup if go.goroot is configured.

And, fixes a bug in v0.25.0 - missing await when checking whether
the go.goroot value is a valid directory.

Fixes #1501

Change-Id: I3487f4b089c9ba4fe34f36e5e033ae19d63537e2
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/320429
Trust: Hyang-Ah Hana Kim <[email protected]>
Run-TryBot: Hyang-Ah Hana Kim <[email protected]>
TryBot-Result: kokoro <[email protected]>
Reviewed-by: Suzy Mueller <[email protected]>
(cherry picked from commit 9fa871b)
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/320549
@golang golang locked and limited conversation to collaborators May 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants