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: error when using vendoring with 1.14 #37629

Open
johanbrandhorst opened this issue Mar 3, 2020 · 13 comments
Open

x/tools/gopls: error when using vendoring with 1.14 #37629

johanbrandhorst opened this issue Mar 3, 2020 · 13 comments
Labels
Milestone

Comments

@johanbrandhorst
Copy link
Member

@johanbrandhorst johanbrandhorst commented Mar 3, 2020

What did you do?

I was using gopls as usual with VS Code.

My environment uses vendoring (implicitly) with the new 1.14 automatic vendor detection.

What did you expect to see?

No errors.

What did you see instead?

An error pop up:

Your workspace is misconfigured: go [-m -json all]: exit status 1: go list -m: can't compute 'all' using the vendor directory (Use -mod=mod or -mod=readonly to bypass.)

It seems some command used by gopls is causing issues when -mod=vendor is set implicitly by the go tool.

Build info

golang.org/x/tools/gopls v0.3.3
    golang.org/x/tools/gopls@v0.3.3 h1:mTFqRDJQmpSsgDDWvbtGnSva1z9uX2XcDszSWa6DhBQ=
    github.com/BurntSushi/toml@v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
    github.com/sergi/go-diff@v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=
    golang.org/x/mod@v0.1.1-0.20191105210325-c90efee705ee h1:WG0RUwxtNT4qqaXX3DPA8zHFNm/D9xaBpxzHt1WcA/E=
    golang.org/x/sync@v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
    golang.org/x/tools@v0.0.0-20200227200655-6862ededa516 h1:OX66ZzpltgCOuBSGdaeT77hS2z3ub2AB+EuGxvGRBLE=
    golang.org/x/xerrors@v0.0.0-20191011141410-1b5146add898 h1:/atklqdjdhuosWIl6AIbOeHJjicWYPqR9bpxqxYG2pA=
    honnef.co/go/tools@v0.0.1-2020.1.3 h1:sXmLre5bzIR6ypkjXCDI3jHPssRhc8KD/Ome589sc3U=
    mvdan.cc/xurls/v2@v2.1.0 h1:KaMb5GLhlcSX+e+qhbRJODnUUBvlw01jt4yrjFIHAuA=

Go info

go version go1.14 linux/amd64

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/<redacted>/.cache/go-build"
GOENV="/home/<redacted>/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY="<redacted>"
GONOSUMDB="<redacted>"
GOOS="linux"
GOPATH="/home/<redacted>/go"
GOPRIVATE="<redacted>"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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-build121565687=/tmp/go-build -gno-record-gcc-switches"
@gopherbot gopherbot added this to the Unreleased milestone Mar 3, 2020
@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Mar 3, 2020

Thanks for reporting! Can you share the full output of gopls -rpc.trace -v check path/to/file.go?

@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.4.0 Mar 3, 2020
@johanbrandhorst

This comment has been minimized.

Copy link
Member Author

@johanbrandhorst johanbrandhorst commented Mar 3, 2020

Thanks for reporting! Can you share the full output of gopls -rpc.trace -v check path/to/file.go?

I'm not able to reproduce it reliably - I'm not quite sure what triggers it, but I will try to get a trace.

@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Mar 3, 2020

Based on the -json and the fact that the misconfiguration error comes up after we try to load the workspace, I would guess that the issue is the go list call in go/packages.

@bcmills: How should go/packages work around this - should we set the -mod=mod flag?

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Mar 3, 2020

@stamblerre, as far as I can tell that use in go/packages is only used in the implementation of determineRootDirs, and determineRootDirs itself is only used in getPkgPath.

So I would recommend raising the abstraction level up a bit: go list accepts a directory argument, so (barring any remaining bugs) you can invoke go list $DIR to resolve the package path corresponding to filesystem directory $DIR:

~/src/golang.org/x/tools$ go1.13.8 list -f '{{.Dir}}' golang.org/x/xerrors
/usr/local/google/home/bcmills/pkg/mod/golang.org/x/xerrors@v0.0.0-20191011141410-1b5146add898

~/src/golang.org/x/tools$ go1.13.8 list /usr/local/google/home/bcmills/pkg/mod/golang.org/x/xerrors@v0.0.0-20191011141410-1b5146add898
golang.org/x/xerrors
@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Mar 3, 2020

Ah, wait, that doesn't work because getPkgPath is trying to infer the hypothetical package path for a directory that does not (yet) actually contain a Go package.

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Mar 3, 2020

Thinking about this some more: is there a use-case for overlays outside of the main module? The module cache is generally read-only anyway, so folks shouldn't be adding files there.

I guess you might want to handle overlays for modules substituted in via replace directives..? But that won't work in vendor mode anyway, because the go command won't read the replacement directories: they really don't have a package path.

So that leaves:

  1. Directories in the main module's vendor tree. You already know where the main module is, so you just need to detect that $DIR is within a subdirectory of its vendor tree, lop off the prefix, done. (No root search required.)

  2. Directories in the main module. go list -m -f '{{.Dir}}' within $DIR to verify that it's actually still in the main module, lop off the prefix containing the main module directory, filepath.ToSlash, and done.

  3. Directories in modules for which the main module contains a replace directive. But we currently require a go.mod file for replaced modules, and the replacement module's go.mod file is supposed to match the path that it replaces. So you can go list -m -f {{.Path}} {{.Dir}} within $DIR, then go list -m {{with .Replace}}{{.Dir}}{{end}} that module path from within the main module (to verify that that module really is the replacement for the module), and again lop off the directory prefix and filepath.ToSlash as above.

@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Mar 3, 2020

Thanks for the detailed analysis, @bcmills! I'll leave the decision-making up to @matloob, since this is not an area of go/packages that I'm particularly familiar with, but your suggestions sound good to me.

@matloob

This comment has been minimized.

Copy link
Contributor

@matloob matloob commented Mar 4, 2020

I'm trying to understand what's going on here. Some questions:

@bcmills, you're asking if there's a use-case for overlays outside the main module because you're trying to figure out if we can get rid of the go list -m call that determines module roots for determining import paths, right? If so, the suggestion is to determine which module each directory is by doing a go list -m call for each of them, right? If so, I'm wondering if the go list -m -f '{{.Dir}} invocation for #​2 will work if the directory doesn't exist.

My second question is why doesn't go list -m all work with -mod=vendor?

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Mar 4, 2020

you're trying to figure out if we can get rid of the go list -m call that determines module roots for determining import paths, right?

Yep.

If so, the suggestion is to determine which module each directory is by doing a go list -m call for each of them, right?

Correct, with the exception of subdirectories of the main module's vendor directory.

If so, I'm wondering if the go list -m -f '{{.Dir}} invocation for #​2 will work if the directory doesn't exist.

It will not, but it should be ok in general to run go list -m -f '{{.Dir}}' on the deepest parent of the proposed directory that actually does exist.

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Mar 4, 2020

My second question is why doesn't go list -m all work with -mod=vendor?

vendor/modules.txt only includes two sets of modules:

  1. modules that provide packages transitively imported by the main module
  2. (as of Go 1.14) modules named explicitly in the main module's go.mod file.

In contrast, at least up through Go 1.14, go list -m all lists all modules transitively required by the main module's go.mod file. In general, that can be an arbitrarily larger superset of the two sets listed in vendor/modules.txt.

That means that the output of go list -m all would vary depending on the -mod flag, and since the default value for the -mod flag changes automatically in Go 1.14, I felt that could be very confusing. So instead, it emits an error, and the user can make an explicit choice to either set -mod=mod explicitly or find a different query that works more consistently with -mod=vendor.

@matloob

This comment has been minimized.

Copy link
Contributor

@matloob matloob commented Mar 4, 2020

Thanks for the explanation. I'll start by implementing #​2. If we need support for directories in the vendor tree or those that are replaced, we can implement #​1 and #​3 if requested by users. (My intention is to keep the implementation of overlays as simple and restricted as possible.

@karuppiah7890

This comment has been minimized.

Copy link

@karuppiah7890 karuppiah7890 commented Mar 11, 2020

Is there any temporary workaround for this issue?

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
6 participants
You can’t perform that action at this time.