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

cmd/go: build is slow to report misspelled package name #26874

Closed
adonovan opened this issue Aug 8, 2018 · 8 comments
Closed

cmd/go: build is slow to report misspelled package name #26874

adonovan opened this issue Aug 8, 2018 · 8 comments
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented Aug 8, 2018

The go command's support for versioned modules means that go build takes 12 seconds to report a typo in a package name.

$ pwd
/home/adonovan/got/src/golang.org/x/tools

$ time go build ./cmd/ssadump
real	0m0.185s
user	0m0.332s
sys	0m0.140s

$ time go build ./cmd/ssadum
go: finding golang.org/x/tools/cmd/ssadum latest
go: finding golang.org/x/tools/cmd latest
go: import "golang.org/x/tools/cmd/ssadum": cannot find module providing package golang.org/x/tools/cmd/ssadum

real	0m12.422s
user	0m0.200s
sys	0m0.096s
@adonovan adonovan changed the title cmd/go: typos are slow cmd/go: build is slow to report misspelled package name Aug 8, 2018
@iamoryanmoshe
Copy link
Contributor

What has changed that caused it?
Was it shorter before?

@cespare
Copy link
Contributor

cespare commented Aug 8, 2018

@oryanmoshe yes, module semantics have made this slow. On my machine (Go 1.10) it takes about 10ms if I misspell the name.

@iamoryanmoshe
Copy link
Contributor

@cespare
Thanks, I didn’t know what to look for until you said modules.. Didn't get the chance to dive into modules internals yet.

I'll look at it and see if I can spot why it's taking so long.

@bcmills
Copy link
Contributor

bcmills commented Aug 8, 2018

That's especially odd since, at least according to my understanding, patterns starting with ./ should only match packages in the current module (see discussion on #26317).

@bcmills
Copy link
Contributor

bcmills commented Aug 9, 2018

@alandonovan, are you seeing this slowness for all builds, or only those with modules enabled?

@bcmills bcmills added NeedsFix The path to resolution is known, but the work has not been done. modules labels Aug 9, 2018
@bcmills bcmills added this to the Go1.11 milestone Aug 9, 2018
@rsc
Copy link
Contributor

rsc commented Aug 10, 2018

The reason is clear from the output: it tries to do a lookup for a different module that might provide ssadum. We need to avoid that.

@gopherbot
Copy link

Change https://golang.org/cl/129061 mentions this issue: cmd/go: do not turn list ./nonexist into a network lookup

@gopherbot
Copy link

Change https://golang.org/cl/151800 mentions this issue: cmd/go: emit go list error for local non-existant packages

gopherbot pushed a commit that referenced this issue Dec 4, 2018
In CL 129061, a check was added for patterns that reference
nonexistent local directories. While this prevented unnecessary
network lookups (fixing #26874), it caused "go list -e" to exit with
an error instead of listing packages with error messages.

This change avoids the network lookup and does not exit for these
kinds of packages. Errors are still reported by
internal/load.LoadImport for packages that don't exist.

Fixes #28023

Change-Id: I0a648269e437aed3a95bfb05461a397264f3793f
Reviewed-on: https://go-review.googlesource.com/c/151800
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@golang golang locked and limited conversation to collaborators Nov 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Development

No branches or pull requests

6 participants