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: erroneous "go.sum is out of sync with go.mod" warning for nested module #59458

Closed
codyoss opened this issue Apr 5, 2023 · 18 comments
Assignees
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. release-blocker Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@codyoss
Copy link
Member

codyoss commented Apr 5, 2023

gopls version

Build info
----------
golang.org/x/tools/gopls v0.11.0
    golang.org/x/tools/[email protected] h1:/nvKHdTtePQmrv9XN3gIUN9MOdUrKzO/dcqgbG6x8EY=
    github.com/BurntSushi/[email protected] h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=
    github.com/google/[email protected] h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
    github.com/sergi/[email protected] h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
    golang.org/x/[email protected] h1:QfTh0HpN6hlw6D3vu8DAwC8pBIwikq0AI1evdm+FksE=
    golang.org/x/exp/[email protected] h1:fl8k2zg28yA23264d82M4dp+YlJ3ngDcpuB1bewkQi4=
    golang.org/x/[email protected] h1:LapD9S96VoQRhi/GrNTqeBJFrUjs5UHCAtTlgwA5oZA=
    golang.org/x/[email protected] h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o=
    golang.org/x/[email protected] h1:ljd4t30dBnAvMZaQCevtY0xLLD0A+bRZXbgLMLU1F/A=
    golang.org/x/[email protected] h1:BrVqGRd7+k1DiOgtnFvAkoQEWQvBc25ouMJM6429SFg=
    golang.org/x/[email protected] h1:7/HkGkN/2ktghBCSRRgp31wAww4syfsW52tj7yirjWk=
    golang.org/x/[email protected] h1:qptQiQwEpETwDiz85LKtChqif9xhVkAm8Nhxs0xnTww=
    honnef.co/go/[email protected] h1:oDx7VAwstgpYpb3wv0oxiZlxY+foCpRAwY7Vk6XpAgA=
    mvdan.cc/[email protected] h1:JVf4NN1mIpHogBj7ABpgOyZc65/UUOkKQFkoURsz4MM=
    mvdan.cc/xurls/[email protected] h1:tzxjVAj+wSBmDcF6zBB7/myTy3gX9xvi8Tyr28AuQgc=
go: go1.20.3

go env

GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/codyoss/Library/Caches/go-build"
GOENV="/Users/codyoss/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/codyoss/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/codyoss/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.20.3"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/codyoss/oss/google-cloud-go/internal/postprocessor/go.mod"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/fk/c2gx4z1147j96_g5g1f2cll400nsl6/T/go-build4044368231=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Open my editor, vscode to do some work.

What did you expect to see?

A working editor

What did you see instead?

Screenshot 2023-04-05 at 11 01 50 AM

Error loading workspace: 1 modules have errors: cloud.google.com/go/internal/postprocessor:pattern cloud.google.com/go/internal/postprocessor/...: cloud.google.com/[email protected]: missing go.sum entry

Editor and settings

  • VS Code
  • Defaults

Notes

  • The module does not actually need to be tidied. Running go mod tidy produces no diff
  • You should be able to repro this by opening vscode at this root: https://github.com/googleapis/google-cloud-go/tree/main/internal/postprocessor
  • This is a large monorepo with 100+ modules in it
  • I think I have seen this with other modules that are rooted in the internal folder as well, so the location of the module I believe is relevant. I have not noticed this errors in other modules in the project like cloud.google.com/go/storage.
  • In the pic provided it shows this was a go list command that produced this error, although I was not sure how to get the exact command. This may actually be a go list issue though
  • Having this error completely breaks codenav in the editor like jumping to refs
  • I can run go mod vendor and my editor works fine. When I do this it does not vendor the module that the editor is complains about cloud.google.com/go, rightfully so
Vendor modules.txt
# cloud.google.com/go/internal/gensnippets v0.0.0-20230405031606-77da60aad056
## explicit; go 1.19
cloud.google.com/go/internal/gensnippets
cloud.google.com/go/internal/gensnippets/metadata
# cloud.google.com/go/internal/godocfx v0.0.0-20230404205613-6e536a548996
## explicit; go 1.19
cloud.google.com/go/internal/godocfx/pkgload
# github.com/Microsoft/go-winio v0.5.2
## explicit; go 1.13
github.com/Microsoft/go-winio
github.com/Microsoft/go-winio/pkg/guid
# github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8
## explicit; go 1.13
github.com/ProtonMail/go-crypto/bitcurves
github.com/ProtonMail/go-crypto/brainpool
github.com/ProtonMail/go-crypto/eax
github.com/ProtonMail/go-crypto/internal/byteutil
github.com/ProtonMail/go-crypto/ocb
github.com/ProtonMail/go-crypto/openpgp
github.com/ProtonMail/go-crypto/openpgp/aes/keywrap
github.com/ProtonMail/go-crypto/openpgp/armor
github.com/ProtonMail/go-crypto/openpgp/ecdh
github.com/ProtonMail/go-crypto/openpgp/ecdsa
github.com/ProtonMail/go-crypto/openpgp/eddsa
github.com/ProtonMail/go-crypto/openpgp/elgamal
github.com/ProtonMail/go-crypto/openpgp/errors
github.com/ProtonMail/go-crypto/openpgp/internal/algorithm
github.com/ProtonMail/go-crypto/openpgp/internal/ecc
github.com/ProtonMail/go-crypto/openpgp/internal/encoding
github.com/ProtonMail/go-crypto/openpgp/packet
github.com/ProtonMail/go-crypto/openpgp/s2k
# github.com/acomagu/bufpipe v1.0.4
## explicit; go 1.12
github.com/acomagu/bufpipe
# github.com/cloudflare/circl v1.1.0
## explicit; go 1.15
github.com/cloudflare/circl/dh/x25519
github.com/cloudflare/circl/dh/x448
github.com/cloudflare/circl/ecc/goldilocks
github.com/cloudflare/circl/internal/conv
github.com/cloudflare/circl/internal/sha3
github.com/cloudflare/circl/math
github.com/cloudflare/circl/math/fp25519
github.com/cloudflare/circl/math/fp448
github.com/cloudflare/circl/math/mlsbset
github.com/cloudflare/circl/sign
github.com/cloudflare/circl/sign/ed25519
github.com/cloudflare/circl/sign/ed448
# github.com/emirpasic/gods v1.18.1
## explicit; go 1.2
github.com/emirpasic/gods/containers
github.com/emirpasic/gods/lists
github.com/emirpasic/gods/lists/arraylist
github.com/emirpasic/gods/trees
github.com/emirpasic/gods/trees/binaryheap
github.com/emirpasic/gods/utils
# github.com/go-git/gcfg v1.5.0
## explicit
github.com/go-git/gcfg
github.com/go-git/gcfg/scanner
github.com/go-git/gcfg/token
github.com/go-git/gcfg/types
# github.com/go-git/go-billy/v5 v5.4.1
## explicit; go 1.13
github.com/go-git/go-billy/v5
github.com/go-git/go-billy/v5/helper/chroot
github.com/go-git/go-billy/v5/helper/polyfill
github.com/go-git/go-billy/v5/memfs
github.com/go-git/go-billy/v5/osfs
github.com/go-git/go-billy/v5/util
# github.com/go-git/go-git/v5 v5.6.1
## explicit; go 1.13
github.com/go-git/go-git/v5
github.com/go-git/go-git/v5/config
github.com/go-git/go-git/v5/internal/revision
github.com/go-git/go-git/v5/internal/url
github.com/go-git/go-git/v5/plumbing
github.com/go-git/go-git/v5/plumbing/cache
github.com/go-git/go-git/v5/plumbing/color
github.com/go-git/go-git/v5/plumbing/filemode
github.com/go-git/go-git/v5/plumbing/format/config
github.com/go-git/go-git/v5/plumbing/format/diff
github.com/go-git/go-git/v5/plumbing/format/gitignore
github.com/go-git/go-git/v5/plumbing/format/idxfile
github.com/go-git/go-git/v5/plumbing/format/index
github.com/go-git/go-git/v5/plumbing/format/objfile
github.com/go-git/go-git/v5/plumbing/format/packfile
github.com/go-git/go-git/v5/plumbing/format/pktline
github.com/go-git/go-git/v5/plumbing/hash
github.com/go-git/go-git/v5/plumbing/object
github.com/go-git/go-git/v5/plumbing/protocol/packp
github.com/go-git/go-git/v5/plumbing/protocol/packp/capability
github.com/go-git/go-git/v5/plumbing/protocol/packp/sideband
github.com/go-git/go-git/v5/plumbing/revlist
github.com/go-git/go-git/v5/plumbing/storer
github.com/go-git/go-git/v5/plumbing/transport
github.com/go-git/go-git/v5/plumbing/transport/client
github.com/go-git/go-git/v5/plumbing/transport/file
github.com/go-git/go-git/v5/plumbing/transport/git
github.com/go-git/go-git/v5/plumbing/transport/http
github.com/go-git/go-git/v5/plumbing/transport/internal/common
github.com/go-git/go-git/v5/plumbing/transport/server
github.com/go-git/go-git/v5/plumbing/transport/ssh
github.com/go-git/go-git/v5/storage
github.com/go-git/go-git/v5/storage/filesystem
github.com/go-git/go-git/v5/storage/filesystem/dotgit
github.com/go-git/go-git/v5/storage/memory
github.com/go-git/go-git/v5/utils/binary
github.com/go-git/go-git/v5/utils/diff
github.com/go-git/go-git/v5/utils/ioutil
github.com/go-git/go-git/v5/utils/merkletrie
github.com/go-git/go-git/v5/utils/merkletrie/filesystem
github.com/go-git/go-git/v5/utils/merkletrie/index
github.com/go-git/go-git/v5/utils/merkletrie/internal/frame
github.com/go-git/go-git/v5/utils/merkletrie/noder
github.com/go-git/go-git/v5/utils/sync
# github.com/golang/protobuf v1.5.3
## explicit; go 1.9
github.com/golang/protobuf/proto
# github.com/google/go-cmp v0.5.9
## explicit; go 1.13
github.com/google/go-cmp/cmp
github.com/google/go-cmp/cmp/internal/diff
github.com/google/go-cmp/cmp/internal/flags
github.com/google/go-cmp/cmp/internal/function
github.com/google/go-cmp/cmp/internal/value
# github.com/google/go-github/v50 v50.2.0
## explicit; go 1.17
github.com/google/go-github/v50/github
# github.com/google/go-querystring v1.1.0
## explicit; go 1.10
github.com/google/go-querystring/query
# github.com/imdario/mergo v0.3.13
## explicit; go 1.13
github.com/imdario/mergo
# github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99
## explicit
github.com/jbenet/go-context/io
# github.com/kevinburke/ssh_config v1.2.0
## explicit
github.com/kevinburke/ssh_config
# github.com/pjbgf/sha1cd v0.3.0
## explicit; go 1.19
github.com/pjbgf/sha1cd
github.com/pjbgf/sha1cd/internal
github.com/pjbgf/sha1cd/ubc
# github.com/sergi/go-diff v1.1.0
## explicit; go 1.12
github.com/sergi/go-diff/diffmatchpatch
# github.com/skeema/knownhosts v1.1.0
## explicit; go 1.17
github.com/skeema/knownhosts
# github.com/xanzy/ssh-agent v0.3.3
## explicit; go 1.16
github.com/xanzy/ssh-agent
# golang.org/x/crypto v0.7.0
## explicit; go 1.17
golang.org/x/crypto/blowfish
golang.org/x/crypto/cast5
golang.org/x/crypto/chacha20
golang.org/x/crypto/curve25519
golang.org/x/crypto/curve25519/internal/field
golang.org/x/crypto/ed25519
golang.org/x/crypto/hkdf
golang.org/x/crypto/internal/alias
golang.org/x/crypto/internal/poly1305
golang.org/x/crypto/sha3
golang.org/x/crypto/ssh
golang.org/x/crypto/ssh/agent
golang.org/x/crypto/ssh/internal/bcrypt_pbkdf
golang.org/x/crypto/ssh/knownhosts
# golang.org/x/mod v0.8.0
## explicit; go 1.17
golang.org/x/mod/semver
# golang.org/x/net v0.8.0
## explicit; go 1.17
golang.org/x/net/context
golang.org/x/net/internal/socks
golang.org/x/net/proxy
# golang.org/x/oauth2 v0.6.0
## explicit; go 1.17
golang.org/x/oauth2
golang.org/x/oauth2/internal
# golang.org/x/sys v0.6.0
## explicit; go 1.17
golang.org/x/sys/cpu
golang.org/x/sys/execabs
golang.org/x/sys/internal/unsafeheader
golang.org/x/sys/unix
golang.org/x/sys/windows
# golang.org/x/tools v0.6.0
## explicit; go 1.18
golang.org/x/tools/go/gcexportdata
golang.org/x/tools/go/internal/packagesdriver
golang.org/x/tools/go/packages
golang.org/x/tools/internal/event
golang.org/x/tools/internal/event/core
golang.org/x/tools/internal/event/keys
golang.org/x/tools/internal/event/label
golang.org/x/tools/internal/gcimporter
golang.org/x/tools/internal/gocommand
golang.org/x/tools/internal/packagesinternal
golang.org/x/tools/internal/pkgbits
golang.org/x/tools/internal/tokeninternal
golang.org/x/tools/internal/typeparams
golang.org/x/tools/internal/typesinternal
# google.golang.org/appengine v1.6.7
## explicit; go 1.11
google.golang.org/appengine/internal
google.golang.org/appengine/internal/base
google.golang.org/appengine/internal/datastore
google.golang.org/appengine/internal/log
google.golang.org/appengine/internal/remote_api
google.golang.org/appengine/internal/urlfetch
google.golang.org/appengine/urlfetch
# google.golang.org/genproto v0.0.0-20230403163135-c38d8f061ccd
## explicit; go 1.19
google.golang.org/genproto/googleapis/gapic/metadata
# google.golang.org/protobuf v1.30.0
## explicit; go 1.11
google.golang.org/protobuf/encoding/protojson
google.golang.org/protobuf/encoding/prototext
google.golang.org/protobuf/encoding/protowire
google.golang.org/protobuf/internal/descfmt
google.golang.org/protobuf/internal/descopts
google.golang.org/protobuf/internal/detrand
google.golang.org/protobuf/internal/encoding/defval
google.golang.org/protobuf/internal/encoding/json
google.golang.org/protobuf/internal/encoding/messageset
google.golang.org/protobuf/internal/encoding/tag
google.golang.org/protobuf/internal/encoding/text
google.golang.org/protobuf/internal/errors
google.golang.org/protobuf/internal/filedesc
google.golang.org/protobuf/internal/filetype
google.golang.org/protobuf/internal/flags
google.golang.org/protobuf/internal/genid
google.golang.org/protobuf/internal/impl
google.golang.org/protobuf/internal/order
google.golang.org/protobuf/internal/pragma
google.golang.org/protobuf/internal/set
google.golang.org/protobuf/internal/strs
google.golang.org/protobuf/internal/version
google.golang.org/protobuf/proto
google.golang.org/protobuf/reflect/protodesc
google.golang.org/protobuf/reflect/protoreflect
google.golang.org/protobuf/reflect/protoregistry
google.golang.org/protobuf/runtime/protoiface
google.golang.org/protobuf/runtime/protoimpl
google.golang.org/protobuf/types/descriptorpb
# gopkg.in/warnings.v0 v0.1.2
## explicit
gopkg.in/warnings.v0
# gopkg.in/yaml.v3 v3.0.1
## explicit
gopkg.in/yaml.v3
@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 Apr 5, 2023
@gopherbot gopherbot added this to the Unreleased milestone Apr 5, 2023
@codyoss codyoss changed the title x/tools/gopls: x/tools/gopls: go.sum is out of sync with go.mod Apr 5, 2023
@findleyr
Copy link
Contributor

findleyr commented Apr 5, 2023

Thank you for the report. I can reproduce with gopls, and with the Go command:

> go list cloud.google.com/go/internal/postprocessor/...
pattern cloud.google.com/go/internal/postprocessor/...: cloud.google.com/[email protected]: missing go.sum entry
> go mod tidy
> go list cloud.google.com/go/internal/postprocessor/...
pattern cloud.google.com/go/internal/postprocessor/...: cloud.google.com/[email protected]: missing go.sum entry

@bcmills @matloob is this a Go command bug?

@findleyr findleyr changed the title x/tools/gopls: go.sum is out of sync with go.mod cmd/go, x/tools/gopls: go.sum is out of sync with go.mod Apr 5, 2023
@bcmills
Copy link
Contributor

bcmills commented Apr 5, 2023

This is not a go command bug per se. go mod tidy is correct that no checksum is needed for cloud.google.com/go, because that module is not directly required in the go.mod file and does not provide any package needed to build or test any package imported by github.com/googleapis/google-cloud-go/internal/postprocessor:

~/src/github.com/googleapis/google-cloud-go/internal/postprocessor$ go list -f '{{with .Module}}{{.Path}}{{end}}' -deps -test all | sort -u | grep cloud.google
cloud.google.com/go/internal/gensnippets
cloud.google.com/go/internal/godocfx
cloud.google.com/go/internal/postprocessor

However, the command go list cloud.google.com/go/internal/postprocessor/... does require that checksum, because it has a well-defined version for cloud.google.com/go and needs to check whether that version of that module contains any packages matching the pattern cloud.google.com/go/internal/postprocessor/....

So, I would say that this is a bug in the query that gopls is using to load packages: it should be using a module-relative query (such as ./..., with the command's working directory in cloud.google.com/go/internal/postprocessor), rather than a wildcard query that could span multiple modules.

@findleyr
Copy link
Contributor

findleyr commented Apr 5, 2023

However, the command go list cloud.google.com/go/internal/postprocessor/... does require that checksum, because it has a well-defined version for cloud.google.com/go and needs to check whether that version of that module contains any packages matching the pattern cloud.google.com/go/internal/postprocessor/....

So you're saying that this query also checks for the package ./internal/postprocessor inside the cloud.google.com/go module, right?

it should be using a module-relative query (such as ./..., with the command's working directory in cloud.google.com/go/internal/postprocessor

So in the presence of a go.work file using dirA, dirB, dirC, we'd have to call go/packages.Load("./...") three times, once from each dir, right?

@bcmills
Copy link
Contributor

bcmills commented Apr 5, 2023

So you're saying that this query also checks for the package ./internal/postprocessor inside the cloud.google.com/go module, right?

Not that package specifically, because it already knows that that one is in the main module. But that pattern would also match packages like cloud.google.com/go/internal/postprocessor/subpkg, so it has to check whether any subpackages like that exist in the selected version of cloud.google.com/go.

So in the presence of a go.work file using dirA, dirB, dirC, we'd have to call go/packages.Load("./...") three times, once from each dir, right?

Hmm... You could do that, yes. You could also give directory paths instead of import paths, I think: go list dirA/... dirB/... dirC/..., which will stop at module boundaries assuming that dirA, dirB, and dirC are absolute paths.

I thought we also had a proposal somewhere about allowing ./... to cross module boundaries when both modules are in the workspace, but I can't find it at the moment. 🤷‍♂️

See also:

@findleyr
Copy link
Contributor

findleyr commented Apr 5, 2023

You could also give directory paths instead of import paths, I think: go list dirA/... dirB/... dirC/..., which will stop at module boundaries assuming that dirA, dirB, and dirC are absolute paths.

We have the absolute absolute paths available, so it sounds like this is the way to go.

I thought we also had a proposal somewhere about allowing ./... to cross module boundaries when both modules are in the workspace,

FWIW that wouldn't help us here, because we don't really want to get into the business of finding a common root for otherwise unrelated dirs (and it shouldn't matter where the GOWORK value lives).

Thanks for the explanation. This is a long standing gopls bug: we've loaded <modulePath>/... for as long as I can remember.

@findleyr findleyr modified the milestones: Unreleased, gopls/v0.12.0 Apr 5, 2023
@dmitshur
Copy link
Contributor

dmitshur commented Apr 6, 2023

I thought we also had a proposal somewhere about allowing ./... to cross module boundaries when both modules are in the workspace, but I can't find it at the moment.

Maybe #51283 is what you were thinking of, though it's more of a tracking issue with Thinking label than a proposal.

@hyangah
Copy link
Contributor

hyangah commented Apr 12, 2023

I thought we also had a proposal somewhere about allowing ./... to cross module boundaries when both modules are in the workspace, but I can't find it at the moment.

Maybe this #50745 (for go.work) or #59480 (for general multi-module repo support)

@findleyr findleyr self-assigned this Apr 17, 2023
@findleyr findleyr changed the title cmd/go, x/tools/gopls: go.sum is out of sync with go.mod x/tools/gopls: erroneous "go.sum is out of sync with go.mod" warning for nested module Apr 18, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/485840 mentions this issue: gopls/internal/lsp/cache: load modules by dir, not module path

@findleyr
Copy link
Contributor

@bcmills I'm working on fixing this in CL 485840, linked above.

This fix is failing on windows only:
https://storage.googleapis.com/go-build-log/903a25ae/windows-amd64-2016_fee7a6f8.log

Params: {"token":"2888608224459394542","value":{"kind":"begin","title":"Error loading workspace","message":"3 modules have errors: \tC:\\Users\\gopher\\AppData\\Local\\Temp\\1\\gopls-regtest-4152842788\\TestUseGoWorkOutsideTheWorkspace\\default\\work\\other\\c:pattern C:\\Users\\gopher\\AppData\\Local\\Temp\\1\\gopls-regtest-4152842788\\TestUseGoWorkOutsideTheWorkspace\\default\\work\\other\\c\\...: directory prefix ..\\other\\c does not contain modules listed in go.work or their selected dependencies \tC:\\Users\\gopher\\AppData\\Local\\Temp\\1\\gopls-regtest-4152842788\\TestUseGoWorkOutsideTheWorkspace\\default\\work\\work\\a:pattern C:\\Users\\gopher\\AppData\\Local\\Temp\\1\\gopls-regtest-4152842788\\TestUseGoWorkOutsideTheWorkspace\\default\\work\\work\\a\\...: directory prefix a does not contain modules listed in go.work or their selected dependencies \tC:\\Users\\gopher\\AppData\\Local\\Temp\\1\\gopls-regtest-4152842788\\TestUseGoWorkOutsideTheWorkspace\\default\\work\\work\\b:pattern C:\\Users\\gopher\\AppData\\Local\\Temp\\1\\gopls-regtest-4152842788\\TestUseGoWorkOutsideTheWorkspace\\default\\work\\work\\b\\...: directory prefix b does not contain modules listed in go.work or their selected dependencies "}}

Test is defined here:
https://cs.opensource.google/go/x/tools/+/master:gopls/internal/regtest/workspace/fromenv_test.go;l=15;drc=bd48b9a5d7872f3d6b457d49038c4657c7443740

This is a strange failure mode. On one hand the go command appears to be correctly computing the relative directory paths (see the "directory prefix"), yet the query fails.

@bcmills
Copy link
Contributor

bcmills commented Apr 18, 2023

The GOWORK environment variable in that test appears to be incorrect:
https://cs.opensource.google/go/x/tools/+/master:gopls/internal/regtest/workspace/fromenv_test.go;l=52;drc=bd48b9a5d7872f3d6b457d49038c4657c7443740

It is using slashes to separate the path elements, where it should be using filepath.Join. Cleaned paths underneath that directory will use backslashes instead, but the backslashes won't include slashes as a prefix.

@findleyr
Copy link
Contributor

findleyr commented Apr 18, 2023

@bcmills thanks, I'll see if I can fix the failure by using the correct separators.

However, I'll also note that this test was previously succeeding. It seems inconsistent that loading <modulePath>/... would (presumably) work, yet loading by directory does not.

@bcmills
Copy link
Contributor

bcmills commented Apr 18, 2023

It seems inconsistent that loading /... would (presumably) work, yet loading by directory does not.

Agreed! We probably have a path-cleaning bug in cmd/go somewhere. (Perhaps we should be calling filepath.Clean here?)

@findleyr
Copy link
Contributor

Aha, so I have tests passing now.

I don't think the problem was the GOWORK environment variable. Rather, it appears to be necessary to use \-separated paths in the use directives.

We have other tests for GOWORK that don't fail. The only difference is that in this case, the GOWORK value is outside the workspace directory.

@bcmills
Copy link
Contributor

bcmills commented Apr 18, 2023

Ah, right. Slash-separated use paths should be fine as long as they are relative, but absolute use paths mixing slashes and backslashes are... weird.

@findleyr
Copy link
Contributor

Ah, right. Slash-separated use paths should be fine as long as they are relative, but absolute use paths mixing slashes and backslashes are... weird.

You're right: the real problem is that the absolute paths use a mix of \ and /. Strange that they ever worked...

In any case, since this is such a weird edge case that wouldn't reasonably arise in practice, I don't think it's worth considering this a Go command bug.

@bcmills
Copy link
Contributor

bcmills commented Apr 18, 2023

since this is such a weird edge case that wouldn't reasonably arise in practice

Um... I didn't say that. 😅

@codyoss
Copy link
Member Author

codyoss commented Apr 20, 2023

Thank you 😄

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/487136 mentions this issue: gopls/internal/lsp/source: more ITV filtering

gopherbot pushed a commit to golang/tools that referenced this issue Apr 21, 2023
This change does additional filtering of intermediate
test variants for the cases where we invoke type
checking indirectly (through filecached derivatives)
not via Snapshot.TypeCheck.

Also:
- document the head-spinning subtlety of ITVs.
- change RemoveIntermediateTestVariants to use a pointer
  to a slice so that it is impossible to forget to use
  the result. D'oh!

Fixes golang/go#59458

Change-Id: I156385456cefeb9dc3a84f23db3b2fa5ef9f244a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/487136
Auto-Submit: Alan Donovan <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Alan Donovan <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
@golang golang locked and limited conversation to collaborators Apr 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. release-blocker Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

6 participants