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: inconsistent vendoring issue breaks gopls #39100

Open
evanmoses-okta opened this issue May 15, 2020 · 11 comments
Open

x/tools/gopls: inconsistent vendoring issue breaks gopls #39100

evanmoses-okta opened this issue May 15, 2020 · 11 comments
Assignees
Labels
Milestone

Comments

@evanmoses-okta
Copy link

@evanmoses-okta evanmoses-okta commented May 15, 2020

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

1.14.1

Does this issue reproduce with the latest release?

yes

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

GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/evanmoses/Library/Caches/go-build"
GOENV="/Users/evanmoses/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/evanmoses/dev/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.14.1/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.14.1/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/evanmoses/dev/go/src/github.com/ScaleFT/device-tools/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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/_m/8vc05c994b35bl9vgctvkykc0000gn/T/go-build196076989=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Using emacs with LSP-mode and gopls, I got this error after pulling my repo:

LSP :: err: exit status 1: stderr: go: inconsistent vendoring in /Users/evanmoses/dev/go/src/github.com/ScaleFT/device-tools:
	<redacted>: is marked as explicit in vendor/modules.txt, but not explicitly required in go.mod

run 'go mod vendor' to sync, or use -mod=mod or -mod=readonly to ignore the vendor directory

and all LSP-related functionality broke. Runing go mod vendor added the // indirect annotation to the <redacted> module in go.mod, but did not affect the error.

What did you expect to see?

No error, possibly a warning, and LSP/gopls would continue to work. While there was indeed inconsistent vendoring that was resolved by running go mod tidy and then go mod vendor, it seems like that shouldn't break gopls and it could continue to provide tooling for my editor. The change in the repo that caused the error was the removal of an explicit import of the dependency, which was still included implicitly via another import.

This is the same issue as #34657

What did you see instead?

An error that broke all tooling in my editor.

I propose two independent fixes:

  1. the error text should include a reference to running go mod tidy as well as go mod vendor to assist users with cleaning up their dependencies
  2. gopls should report a warning and continue providing lsp services rather than failing with a hard error.
@gopherbot
Copy link

@gopherbot gopherbot commented May 15, 2020

Thank you for filing a gopls issue! Please take a look at the Troubleshooting guide, and make sure that you have provided all of the relevant information here.

@gopherbot gopherbot added the gopls label May 15, 2020
@stamblerre stamblerre changed the title Inconsistent vendoring issue breaks gopls, could be a warning instead x/tools/gopls: inconsistent vendoring issue breaks gopls May 15, 2020
@gopherbot gopherbot added the Tools label May 15, 2020
@gopherbot gopherbot added this to the Unreleased milestone May 15, 2020
@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.5.0 May 15, 2020
@heschik
Copy link
Contributor

@heschik heschik commented May 15, 2020

This error message comes straight from the go command, and I don't think go mod tidy is always the right solution. But Bryan and Jay should speak to that.

As for continuing, unfortunately gopls needs information from the go command to provide editor services and in this situation it simply can't get it. Turning off vendor mode might fix the problem, but I don't think gopls should be doing that of its own volition. It might result in downloading stuff, or it might make things worse.

@bcmills @jayconrod

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented May 18, 2020

Indeed, go mod tidy can delete requirements, and we shouldn't necessarily recommend running it.

Just to confirm, go mod vendor alone did not resolve the error, but go mod tidy followed by go mod vendor did? What changes did go mod tidy make to go.mod?

If go mod vendor succeeded, it seems like a bug if there's an "inconsistent vendoring" error afterward. Either go mod vendor should fail, or it should ensure go.mod and vendor/modules.txt are consistent.

@buyology
Copy link

@buyology buyology commented May 18, 2020

As for continuing, unfortunately gopls needs information from the go command to provide editor services and in this situation it simply can't get it.

I think it's desirable for e.g. code completion to be forgiving and "fuzzy" rather than unforgiving and strict — I'd rather have some code completion than none even if my code base is partially "broken".

If the introduction of this new check in the go tool introduced in 1.14 is not possible to bypass by the downstream tooling, I think the second best would be to at least show this error to the user. Right now vscode-go is completely quiet unless you dive into the gopls debug output. But maybe that's an issue that should be opened in vscode-go.

@evanmoses-okta
Copy link
Author

@evanmoses-okta evanmoses-okta commented May 19, 2020

Just to confirm, go mod vendor alone did not resolve the error, but go mod tidy followed by go mod vendor did? What changes did go mod tidy make to go.mod?

Yes. go mod vendor added an // indirect comment to the module in question in go.mod, but didn't remove the ## explicit annotation in vendor/modules.txt; the tooling continued to give the same error. After I ran go mod tidy, and then go mod vendor, the module in go.mod was removed and the ## explicit annotation was removed, and then the tooling was happy. For more clarity:

before

something.go:
import (
   "github.com/my-co/whatever"
)

go.mod:
require (
    github.com/my-co/whatever v0.0.0-xxxxx-abcd
)

vendor/module.txt:
# github.com/my-co/whatever v0.0.0-xxxxx-abcd
## explicit
github.com/my-co/whatever

After update that borked gopls:

something.go:
import (
   "github.com/my-co/something-else-that-transitively-includes-whatever"
)

go.mod (same):
require (
    github.com/my-co/whatever v0.0.0-xxxxx-abcd
)

vendor/module.txt (same):
# github.com/my-co/whatever v0.0.0-xxxxx-abcd
## explicit
github.com/my-co/whatever

After go mod vendor (same error from tooling)

something.go:
import (
   "github.com/my-co/something-else-that-transitively-includes-whatever"
)

go.mod (now has "// indirect"):
require (
    github.com/my-co/whatever v0.0.0-xxxxx-abcd // indirect
)

vendor/module.txt (same):
# github.com/my-co/whatever v0.0.0-xxxxx-abcd
## explicit
github.com/my-co/whatever

After go mod tidy && go mod vendor:

something.go:
import (
   "github.com/my-co/something-else-that-transitively-includes-whatever"
)

go.mod:
require (
   <no longer includes whatever>
)

vendor/module.txt (no more ## explicit):
# github.com/my-co/whatever v0.0.0-xxxxx-abcd
github.com/my-co/whatever
@bcmills
Copy link
Member

@bcmills bcmills commented May 19, 2020

The token ##explicit in your pasted examples is suspect. The actual token that the modules.txt parser looks for has a space after the ##:

if strings.HasPrefix(line, "## ") {

Could you publish a repo that shows the actual contents of the files at the named phases? I wonder if something in gopls or the editor hooks is ending up corrupting the tokens, or perhaps gopls is internally using a go.mod that differs from the one in your source tree.

@evanmoses-okta
Copy link
Author

@evanmoses-okta evanmoses-okta commented May 19, 2020

I re-typed that by hand while looking at my git history; the actual file did, in fact, have the space. This is proprietary code so I can't publish the repo, I hope that there's enough information above.

@evanmoses-okta
Copy link
Author

@evanmoses-okta evanmoses-okta commented May 19, 2020

(I edited my example to be more correct)

@bcmills
Copy link
Member

@bcmills bcmills commented May 19, 2020

The change from direct to // indirect after go mod vendor, especially combined with the proprietary code, makes me wonder whether this is the same underlying problem as #38846.

@evanmoses-okta, can you confirm whether your setup may be importing packages symlinked into the main module's source tree, and/or importing packages with normally-ignored components (such as testdata or _foo)?

@evanmoses-okta
Copy link
Author

@evanmoses-okta evanmoses-okta commented May 19, 2020

Not as far as I know. I was able to reproduce the behavior of the tooling (that is, adding // indirect after go mod vendor) in this trivial repo, although I didn't get errors from gopls: https://github.com/evanmoses-okta/gopls-error-repro

In that repo, I

  1. Created moda and consumer that depends on it, and ran go mod vendor
  2. Created modb which depends on moda
  3. Changed consumer to depend on modb instead of moda
  4. This caused go build to complain about inconsistent vendoring
  5. Ran go mod vendor, which added the // indirect comment in go.mod while leaving the ## explicit in vendor/modules.txt alone, as in this issue. This did not appear to break tooling, although obviously this is a much simpler repo and maybe there are further confounding factors.

Running go mod tidy in this repo will remove moda from consumer's go.mod and remove the ## explicit annotation in modules.txt

@gopherbot
Copy link

@gopherbot gopherbot commented May 30, 2020

Change https://golang.org/cl/235619 mentions this issue: internal/lsp: support go mod vendor as a command

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.