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/internal/imports: TestModeGetmodeVendor failing due to vendoring change #34826

Closed
jayconrod opened this issue Oct 10, 2019 · 9 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@jayconrod
Copy link
Contributor

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

$ go version
go version devel +46b7557836 Thu Oct 10 13:56:58 2019 +0000 darwin/amd64

golang.org/x/tools checked out at 0337d82405ff9ebcb2638658c98761e12f51693e

Does this issue reproduce with the latest release?

no, only tip

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/jayconrod/Library/Caches/go-build"
GOENV="/Users/jayconrod/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/jayconrod/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/jayconrod/Code/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/jayconrod/Code/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/jayconrod/go/src/golang.org/x/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/rq/x0692kqj6ml8cvrhcqh5bswc008xj1/T/go-build395310348=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

go test golang.org/x/tools/internal/imports -run=TestModeGetmodeVendor

What did you expect to see?

test passes

What did you see instead?

--- FAIL: TestModeGetmodeVendor (0.25s)
    mod_test.go:235: loading package name for rsc.io/quote: running go: exit status 1 (stderr:
        go list -m: can't list modules with -mod=vendor
        	use -mod=mod or -mod=readonly to ignore the vendor directory
        )
    mod_test.go:235: package name for rsc.io/quote = , want quote
    mod_test.go:235: scan failed: running go: exit status 1 (stderr:
        go list -m: can't list modules with -mod=vendor
        	use -mod=mod or -mod=readonly to ignore the vendor directory
        )
    mod_test.go:235: scanning for quote did not find rsc.io/quote
    mod_test.go:235: import path rsc.io/quote not found in active modules
    mod_test.go:240: package name for rsc.io/quote = , want quote
    mod_test.go:240: import path rsc.io/quote not found in active modules
FAIL
FAIL	golang.org/x/tools/internal/imports	0.291s
FAIL

https://build.golang.org/log/4ca136788f50edc4878fe80c76f1ec376539a9b2

@gopherbot gopherbot added this to the Unreleased milestone Oct 10, 2019
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Oct 10, 2019
@jayconrod jayconrod added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Soon This needs to be done soon. (regressions, serious bugs, outages) labels Oct 10, 2019
@bcmills
Copy link
Contributor

bcmills commented Oct 10, 2019

Argh, sorry for the lack of advance warning. I ran the x/tools tests with the vendor change but forgot to look at the output. 😞

@jayconrod
Copy link
Contributor Author

Caused by some combination of:

  • CL 199821 - cmd/go/internal/list: disallow 'list -m' with '-mod=vendor'
  • CL 198319 - cmd/go: automatically check and use vendored packages

(I'm putting together a CL right now that skips the test while we figure out what to do)

@gopherbot
Copy link

Change https://golang.org/cl/200299 mentions this issue: internal/imports: skip TestModeGetmodeVendor

@bcmills
Copy link
Contributor

bcmills commented Oct 10, 2019

Considering the test name, it's probably 199821.

I'm guessing that most of x/tools/internal/imports is actually interested in packages rather than modules per se. If so, the fix is probably to switch from something like

go list -f '{{with .Module}}{{.Path}} {{.Dir}}{{end}}' ...

More generally, though, go list -m is somewhat nonsensical in vendor mode: the vendor directory doesn't contain the entire module and may interleave packages from different modules.

gopherbot pushed a commit to golang/tools that referenced this issue Oct 10, 2019
Test is broken by recent cmd/go vendoring changes. Skipping the test
case while we figure out what to do.

Updates golang/go#34826

Change-Id: I31a6f18e40420970251e25836edcf32c56c10ef5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200299
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@heschi
Copy link
Contributor

heschi commented Oct 10, 2019

No, it very specifically needs modules, I'm afraid. Listing packages is too slow. I'll talk it through with Jay.

@bcmills
Copy link
Contributor

bcmills commented Oct 10, 2019

-mod=vendor does not have accurate module information — and never did, unfortunately.

(It previously reported incomplete module information in vendor mode, but we probably shouldn't build tooling around that.)

If you're detecting vendor mode explicitly, the easy way to list out the available modules is to parse the # module version and # module version => replacement lines from the vendor/modules.txt file. (But that won't get you coherent directories for individual modules, because the vendor directory intentionally flattens the modules into an interleaved directory structure.)

@bcmills bcmills added release-blocker and removed Soon This needs to be done soon. (regressions, serious bugs, outages) labels Oct 11, 2019
@bcmills bcmills modified the milestones: Unreleased, Go1.14 Oct 11, 2019
@heschi heschi self-assigned this Oct 21, 2019
@bradfitz

This comment has been minimized.

@gopherbot
Copy link

Change https://golang.org/cl/203257 mentions this issue: internal/imports: skip TestStdlibNotPrefixed

gopherbot pushed a commit to golang/tools that referenced this issue Oct 24, 2019
At tip, $GOROOT/src/go.mod is 1.14, $GOROOT/src/vendor exists, and so
vendor mode is automatically enabled. That causes golang/go#34826.

This will be fixed by my upcoming vendoring CL, but for now skip.

Updates golang/go#34826

Change-Id: I5fff51fff54cf83e6369ae76bf3b19cfb7b5ac15
Reviewed-on: https://go-review.googlesource.com/c/tools/+/203257
Run-TryBot: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/203017 mentions this issue: internal/imports: support vendoring in module mode

@golang golang locked and limited conversation to collaborators Oct 23, 2020
@rsc rsc unassigned heschi Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. 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

5 participants