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

Jump to bottom

x/tools/gopls: gopls.add_dependency workspace edits not working? #44247

Closed
myitcv opened this issue Feb 13, 2021 · 7 comments
Closed

x/tools/gopls: gopls.add_dependency workspace edits not working? #44247

myitcv opened this issue Feb 13, 2021 · 7 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@myitcv
Copy link
Member

myitcv commented Feb 13, 2021

What version of Go are you using (go version)?

$ go version
go version devel +e9c9683597 Tue Feb 9 18:40:13 2021 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.1.1-0.20210204180613-842a9283d6c6
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.0.0-20210204180613-842a9283d6c6

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/myitcv/gostuff/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel +e9c9683597 Tue Feb 9 18:40:13 2021 +0000"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/gostuff/src/github.com/myitcv/govim/go.mod"
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-build474152825=/tmp/go-build -gno-record-gcc-switches"

What did you do?

This is a follow up to #44035. Thanks very much @heschik for addressing that issue in golang/tools@706a59c.

I've just tried this out, but I'm still not seeing workspace edits for go.mod. Given your change had test coverage, I suspect this is a configuration issue of some sort but I can't see what that might be.

I started with the same setup as before:

-- go.mod --
module example.com/repro

go 1.16
-- main.go --
package main

import (
	"fmt"

	_ "golang.org/x/tools/imports"
)

func main() {
	fmt.Println("hello, world!")
}

i.e. the requirement on golang.org/x/tools/imports is missing from go.{mod,sum}.

Loading main.go in Vim (using govim) gives error diagnostics as expected:

main.go|6 col 4| could not import golang.org/x/tools/imports (no required module provides package "golang.org/x/tools/imports")
main.go|6 col 4| golang.org/x/tools is not in your go.mod file

If we ask for suggested fixes at main.go:6 we get a code action response with the gopls.add_dependency command.

If we execute the gopls.add_dependency command, go.{mod,sum} change beneath the editor and gopls, i.e. we don't see any workspace edits, despite the changes in golang/tools@706a59c.

Log files:

Is there something we (govim) are not doing right here?

As a side note, I suspect the tempModfile option is no longer valid given that we have allowModfileModifications? Because I initially had tempModfile: true and this resulted in the error message:

-modfile is unsupported for this Go version

But I can raise this as a separate issue if needs be.

What did you expect to see?

gopls.add_dependency resulting in workspace edits for go.{mod,sum}.

What did you see instead?

The files changing beneath the editor, and that change getting detected by the govim file watcher

cc @stamblerre @heschik

FYI @leitzler
@myitcv myitcv added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. gopls Issues related to the Go language server, gopls. labels Feb 13, 2021
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Feb 13, 2021
@gopherbot gopherbot added this to the Unreleased milestone Feb 13, 2021
@heschi
Copy link
Contributor

heschi commented Feb 13, 2021

Sending workspace edits for a closed file causes VSCode to open it and leave the edits unsaved, which is undesirable, particularly for go.sum. To avoid that, gopls still directly edits closed files. You should see workspace edits if you open go.mod first.

@stamblerre stamblerre added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 13, 2021
@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.6.6 Feb 13, 2021
@leitzler
Copy link
Contributor

I did try out Pauls example (using the defaults; tempModFile: true / allowModfileModifications: false) according to:

  1. Open main.go
  2. Apply the suggested fix, i.e. calling execute for the provided gopls.go_get_package:
[Trace - 23:56:03.030 PM] Sending request 'workspace/executeCommand - (5)'.
Params: {"command":"gopls.go_get_package","arguments":[{"URI":"file:///private/var/folders/j4/l2j99h6d5qd6knjlllql0bb80000gn/T/tmp.7WMpQpFw/main.go","Pkg":"
golang.org/x/tools/imports","AddRequire":true}]}
  1. Send didChangeWatchedFiles for go.mod and go.sum as the file watcher detects the updates:
[Trace - 23:56:04.156 PM] Sending notification 'workspace/didChangeWatchedFiles'.
Params: {"changes":[{"uri":"file:///private/var/folders/j4/l2j99h6d5qd6knjlllql0bb80000gn/T/tmp.7WMpQpFw/go.mod","type":2}]}


[Trace - 23:56:04.157 PM] Sending notification 'workspace/didChangeWatchedFiles'.
Params: {"changes":[{"uri":"file:///private/var/folders/j4/l2j99h6d5qd6knjlllql0bb80000gn/T/tmp.7WMpQpFw/go.sum","type":1}]}

Then I get a couple of other go.sum diagnostics back for both main.go and go.mod:

[Trace - 23:56:04.805 PM] Received notification 'textDocument/publishDiagnostics'.
Params: {"uri":"file:///private/var/folders/j4/l2j99h6d5qd6knjlllql0bb80000gn/T/tmp.7WMpQpFw/main.go","version":1,"diagnostics":[{"range":{"start":{"line":5
,"character":1},"end":{"line":5,"character":31}},"severity":1,"source":"compiler","message":"error while importing golang.org/x/tools/imports: missing go.su
m entry for module providing package golang.org/x/mod/module (imported by golang.org/x/tools/internal/imports); to add:\n\tgo get golang.org/x/tools/interna
l/imports@v0.1.0"},{"range":{"start":{"line":5,"character":1},"end":{"line":5,"character":31}},"severity":1,"source":"compiler","message":"error while impor
ting golang.org/x/tools/imports: missing go.sum entry for module providing package golang.org/x/mod/semver (imported by golang.org/x/tools/internal/gocomman
d); to add:\n\tgo get golang.org/x/tools/internal/gocommand@v0.1.0"},{"range":{"start":{"line":5,"character":1},"end":{"line":5,"character":31}},"severity":
1,"source":"compiler","message":"error while importing golang.org/x/tools/imports: missing go.sum entry for module providing package golang.org/x/sys/execab
s (imported by golang.org/x/tools/internal/gocommand); to add:\n\tgo get golang.org/x/tools/internal/gocommand@v0.1.0"}]}


[Trace - 23:56:04.805 PM] Received notification 'textDocument/publishDiagnostics'.
Params: {"uri":"file:///private/var/folders/j4/l2j99h6d5qd6knjlllql0bb80000gn/T/tmp.7WMpQpFw/go.mod","diagnostics":[{"range":{"start":{"line":4,"character":
0},"end":{"line":4,"character":33}},"severity":1,"source":"compiler","message":"error while importing golang.org/x/tools/internal/gocommand: missing go.sum
entry for module providing package golang.org/x/mod/semver (imported by golang.org/x/tools/internal/gocommand); to add:\n\tgo get golang.org/x/tools/interna
l/gocommand@v0.1.0"},{"range":{"start":{"line":4,"character":0},"end":{"line":4,"character":33}},"severity":1,"source":"compiler","message":"error while imp
orting golang.org/x/tools/internal/gocommand: missing go.sum entry for module providing package golang.org/x/mod/semver (imported by golang.org/x/tools/inte
rnal/gocommand); to add:\n\tgo get golang.org/x/tools/internal/gocommand@v0.1.0"},{"range":{"start":{"line":4,"character":0},"end":{"line":4,"character":33}
},"severity":1,"source":"compiler","message":"error while importing golang.org/x/tools/internal/gocommand: missing go.sum entry for module providing package
 golang.org/x/sys/execabs (imported by golang.org/x/tools/internal/gocommand); to add:\n\tgo get golang.org/x/tools/internal/gocommand@v0.1.0"},{"range":{"s
tart":{"line":4,"character":0},"end":{"line":4,"character":33}},"severity":1,"source":"compiler","message":"error while importing golang.org/x/tools/interna
l/gocommand: missing go.sum entry for module providing package golang.org/x/sys/execabs (imported by golang.org/x/tools/internal/gocommand); to add:\n\tgo g
et golang.org/x/tools/internal/gocommand@v0.1.0"},{"range":{"start":{"line":4,"character":0},"end":{"line":4,"character":33}},"severity":1,"source":"compile
r","message":"error while importing golang.org/x/tools/internal/imports: missing go.sum entry for module providing package golang.org/x/mod/module (imported
 by golang.org/x/tools/internal/imports); to add:\n\tgo get golang.org/x/tools/internal/imports@v0.1.0"},{"range":{"start":{"line":4,"character":0},"end":{"
line":4,"character":33}},"severity":1,"source":"compiler","message":"error while importing golang.org/x/tools/internal/imports: missing go.sum entry for mod
ule providing package golang.org/x/mod/module (imported by golang.org/x/tools/internal/imports); to add:\n\tgo get golang.org/x/tools/internal/imports@v0.1.
0"}]}

Is there something I'm missing out? (full log at https://gist.github.com/leitzler/d189c42bdb0dae8562682b47325938e2).

@leitzler
Copy link
Contributor

Just to add to the above, I expected that applying the suggested fix would have the ~same effect as running go get golang.org/x/tools v0.1.0 before I open main.go.

If I do run go get before, then I only get one diagnostic (warning) telling me that the require line should be direct (ideally gopls would fix that as well executing gopls.go_get_package).

@heschi
Copy link
Contributor

heschi commented Feb 16, 2021

@leitzler: I don't see how your problem is related to workspace edits, but it is a real (and rather embarrassing) mistake on my part, so I'll file a new bug. Thanks for the report.

@heschi heschi mentioned this issue Feb 16, 2021
Closed
@leitzler
Copy link
Contributor

Thank you and apologies for the issue hijacking. Didn't see that it was unrelated.

@stamblerre stamblerre modified the milestones: gopls/v0.6.6, gopls/v1.0.0 Feb 17, 2021
@stamblerre
Copy link
Contributor

That issue was #44307, and it is now resolved. Can we close this?

@myitcv
Copy link
Member Author

myitcv commented Feb 22, 2021

Yes, this can be closed.

@myitcv myitcv closed this as completed Feb 22, 2021
@stamblerre stamblerre removed this from the gopls/v1.0.0 milestone Feb 22, 2021
@golang golang locked and limited conversation to collaborators Feb 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@leitzler @myitcv @stamblerre @gopherbot @heschi