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

x/tools/gopls: deprecate the "allowModfileModifications" setting #56570

Closed
JensSkipr opened this issue Nov 4, 2022 · 7 comments
Closed

x/tools/gopls: deprecate the "allowModfileModifications" setting #56570

JensSkipr opened this issue Nov 4, 2022 · 7 comments
Assignees
Labels
gopls/metadata Issues related to metadata loading in gopls gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@JensSkipr
Copy link

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

Version Information
  • Run go version to get version of Go from the VS Code integrated terminal.
    • go version go1.18.7 linux/amd64
  • Run gopls -v version to get version of Gopls from the VS Code integrated terminal.
    • golang.org/x/tools/gopls v0.10.1
  • Run code -v or code-insiders -v to get version of VS Code or VS Code Insiders.
    • 1.73.0 (8fa188b2b301d36553cbc9ce1b0a146ccb93351f)
  • Check your installed extensions to get the version of the VS Code Go extension
    • v0.35.2
  • Run Ctrl+Shift+P (Cmd+Shift+P on Mac OS) > Go: Locate Configured Go Tools command.
      GOBIN: undefined
      toolsGopath: /home/skipr/go
      gopath: /home/skipr/go
      GOROOT: /usr/lib/golang
      PATH: /home/skipr/.local/bin:/home/skipr/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/usr/local/go/bin:/home/skipr/go/bin:/usr/local/go/bin:/home/skipr/go/bin
    
      go:    /usr/bin/go: go version go1.18.7 linux/amd64
    
      gotests:    /home/skipr/go/bin/gotests    (version: v1.6.0 built with go: go1.18.2)
      gomodifytags:    /home/skipr/go/bin/gomodifytags    (version: v1.16.0 built with go: go1.18.2)
      impl:    /home/skipr/go/bin/impl    (version: v1.1.0 built with go: go1.18.2)
      goplay:    /home/skipr/go/bin/goplay    (version: v1.0.0 built with go: go1.18.2)
      dlv:    /home/skipr/go/bin/dlv    (version: v1.8.3 built with go: go1.18.2)
      staticcheck:    /home/skipr/go/bin/staticcheck    (version: v0.3.2 built with go: go1.18.2)
      gopls:    /home/skipr/go/bin/gopls    (version: v0.10.1 built with go: go1.18.7)
    
      go env
      Workspace Folder (workspace): /home/skipr/Dev/workspace
      GO111MODULE="auto"
      GOARCH="amd64"
      GOBIN=""
      GOCACHE="/home/skipr/.cache/go-build"
      GOENV="/home/skipr/.config/go/env"
      GOEXE=""
      GOEXPERIMENT=""
      GOFLAGS=""
      GOHOSTARCH="amd64"
      GOHOSTOS="linux"
      GOINSECURE=""
      GOMODCACHE="/home/skipr/go/pkg/mod"
      GONOPROXY=""
      GONOSUMDB=""
      GOOS="linux"
      GOPATH="/home/skipr/go"
      GOPRIVATE=""
      GOPROXY="direct"
      GOROOT="/usr/lib/golang"
      GOSUMDB="off"
      GOTMPDIR=""
      GOTOOLDIR="/usr/lib/golang/pkg/tool/linux_amd64"
      GOVCS=""
      GOVERSION="go1.18.7"
      GCCGO="gccgo"
      GOAMD64="v1"
      AR="ar"
      CC="gcc"
      CXX="g++"
      CGO_ENABLED="1"
      GOMOD="/home/skipr/Dev/workspace/go.mod"
      GOWORK="/home/skipr/Dev/workspace/go.work"
      CGO_CFLAGS="-g -O2"
      CGO_CPPFLAGS=""
      CGO_CXXFLAGS="-g -O2"
      CGO_FFLAGS="-g -O2"
      CGO_LDFLAGS="-g -O2"
      PKG_CONFIG="pkg-config"
      GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3105188839=/tmp/go-build -gno-record-gcc-switches"
    

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.buildTags": "integration",
  "go.toolsGopath": "~/go",
  "go.toolsManagement.autoUpdate": true,
  "gopls": {
    "build.allowModfileModifications": true
  },
}

Describe the bug

I faced the issue in a large project, but was able to reproduce minimally following: https://go.dev/doc/tutorial/workspaces

Once you add a Golang Workspace by creating a go.work file, every go.mod file returns error: no go.mod file found in /home/skipr/Dev/workspace go list. Where /home/skipr/Dev/workspace is the root of the workspace.
image

Steps to reproduce the behavior:

  1. Start with a blank directory
  2. Create a folder hello
  3. Start a Go module using go mod init example.com/hello (run inside hello directory)
  4. Create hello/main.go containing
package main

func main() {}
  1. Start a Go workspace using go work init ./hello (run inside root/workspace directory)
  2. Open hello/go.mod

Adding a go.mod to the root doesn't solve the issue. Nor adding . to the go.work. When doing the latter, you even get the same error in the root go.mod:
image
image

Screenshots or recordings

See above

@findleyr
Copy link
Contributor

findleyr commented Nov 4, 2022

Thanks, I was able to reproduce, but only using "build.allowModfileModifications": true

Disabling that setting should fix your problem, but this is a bug and will be prioritized for the next gopls patch release.

@findleyr findleyr changed the title gopls: no go.mod file found in workspace root x/tools/gopls: no go.mod file found in workspace root with "allowModfileModifications" setting Nov 4, 2022
@findleyr findleyr transferred this issue from golang/vscode-go Nov 4, 2022
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Nov 4, 2022
@gopherbot gopherbot added this to the Unreleased milestone Nov 4, 2022
@findleyr findleyr modified the milestones: Unreleased, gopls/v0.10.1 Nov 4, 2022
@JensSkipr
Copy link
Author

Thanks for the fast response! I tried to remove the option as well, but just realize I only removed it from my workspace config in VS Code, not from my user config. So, it was still active from gopls' point-of-view.

@findleyr
Copy link
Contributor

findleyr commented Nov 4, 2022

It was easy to find the bug thanks to your detailed report!

Unfortunately, this is a case where the 'allowModfileModifications' flag has deep implications into the behavior of gopls, and therefore should really be tested against the entire gopls test suite (but isn't).

AFAIK very few people use this setting, and I'd love to deprecate it in favor of go.mod code-lens. Can you share why you have it enabled?

@JensSkipr
Copy link
Author

JensSkipr commented Nov 4, 2022

I did some digging and I believe we enabled it when we did bump Go to 1.16. Without this flag, we got following error in VS Code:

Error loading workspace: err: exit status 1: stderr: go: updates to go.sum needed, disabled by -mod=readonly : packages.Load error

Based on your comment, our in-house workflow might not be following best practices. If that's the case, fine for me to remove the flag in our config and move to best practices instead.

I assume above issue could be resolved by doing a go mod tidy.

@findleyr findleyr modified the milestones: gopls/v0.10.1, gopls/v0.10.2 Nov 4, 2022
@findleyr
Copy link
Contributor

findleyr commented Nov 6, 2022

I assume above issue could be resolved by doing a go mod tidy.

Thanks for explaining. Yes, that indicates that your workspace is not tidy.

JensSkipr added a commit to JensSkipr/LinuxSetup that referenced this issue Nov 7, 2022
Having the go.mod to auto-update is not a best practice and can lead to weird bugs. See golang/go#56570 for more info.
@findleyr findleyr modified the milestones: gopls/v0.11.0, gopls/v0.12.0 Dec 8, 2022
@findleyr findleyr modified the milestones: gopls/v0.12.0, gopls/v0.13.0 Mar 27, 2023
@adonovan adonovan added the gopls/metadata Issues related to metadata loading in gopls label Aug 31, 2023
@findleyr findleyr modified the milestones: gopls/v0.14.0, gopls/v0.15.0 Oct 9, 2023
@findleyr
Copy link
Contributor

findleyr commented Feb 4, 2024

I think the action to take here is to deprecate the 'allowModfileModifications' setting.

Generally speaking, this setting means "whatever the go command does to your go.mod file is OK". That's not a coherent mode of operation for gopls to support, which is why it has no test coverage. Additionally, the documentation says "this option will eventually be removed".

We should surface the deprecation notice in gopls@v0.15.0, and remove in gopls@v0.16.0.

@findleyr findleyr changed the title x/tools/gopls: no go.mod file found in workspace root with "allowModfileModifications" setting x/tools/gopls: deprecate the "allowModfileModifications" setting Feb 4, 2024
@findleyr findleyr self-assigned this Feb 4, 2024
@gopherbot
Copy link

Change https://go.dev/cl/561975 mentions this issue: gopls/internal/settings: deprecate "allowModfileModifications"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls/metadata Issues related to metadata loading in gopls gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants