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

tools: replace use of 'gopkgs' with 'go list all' #258

Closed
marwan-at-work opened this issue Jun 25, 2020 · 11 comments
Closed

tools: replace use of 'gopkgs' with 'go list all' #258

marwan-at-work opened this issue Jun 25, 2020 · 11 comments
Assignees
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@marwan-at-work
Copy link
Contributor

Currently, if you cmd+shift+P and choose Go: Add Import. The list of imported file looks incorrect. For example, packages within the same module do not show up.

To reproduce, go into a module repo such as golang.org/x/tools and open a Go file such as internal/lsp/source/rename.go.

Try to run the Go: Add Import and notice the list does not include most of the packages that you expect within the tools repo. Or even sometimes you get this error:

Could not find packages. Ensure `gopkgs -format {{.Name}};{{.ImportPath}}` runs successfully.

As a solution, I don't think we have to call out into gopkgs anymore as we can just call go list directly to get all importable paths within a directory.

@hyangah hyangah added NeedsFix The path to resolution is known, but the work has not been done. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 25, 2020
marwan-at-work added a commit to marwan-at-work/vscode-go-1 that referenced this issue Jun 25, 2020
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/240001 mentions this issue: Fix Add Import action by using go list to suggest import paths

@hyangah
Copy link
Contributor

hyangah commented Jun 25, 2020

Thanks @marwan-at-work for investigation and offering the fix.

I want to know if I understand the issue right - since I can't reproduce the issue myself yet. Here is the screenshot.

Screen Shot 2020-06-25 at 5 43 47 PM

What's the gopkgs version?
The version I am using is

$ go version -m ~/go/bin/gopkgs
/Users/hakim/go/bin/gopkgs: go1.14.3
path github.com/uudashr/gopkgs/v2/cmd/gopkgs
mod github.com/uudashr/gopkgs/v2 v2.1.2 h1:A0+QH6wqNRHORJnxmqfeuBEsK4nYQ7pgcOHhqpqcrpo=
dep github.com/karrick/godirwalk v1.12.0 h1:nkS4xxsjiZMvVlazd0mFyiwD4BR9f3m6LXGhM2TUx3Y=
dep github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I=

@marwan-at-work
Copy link
Contributor Author

@hyangah

marwansulaiman@Marwans-MBP go-proxy % go version -m `which gopkgs`
/Users/marwansulaiman/go/bin/gopkgs: go1.14.2
	path	github.com/uudashr/gopkgs/v2/cmd/gopkgs
	mod	github.com/uudashr/gopkgs/v2	v2.1.2	h1:A0+QH6wqNRHORJnxmqfeuBEsK4nYQ7pgcOHhqpqcrpo=
	dep	github.com/karrick/godirwalk	v1.12.0	h1:nkS4xxsjiZMvVlazd0mFyiwD4BR9f3m6LXGhM2TUx3Y=
	dep	github.com/pkg/errors	v0.8.1	h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I=

and here's what i get when trying to run the command:

Screen Shot 2020-06-25 at 6 56 39 PM

@hyangah
Copy link
Contributor

hyangah commented Jun 26, 2020

@marwan-at-work thanks - interesting.

So, does gopkgs -format '{{.Name}};{{.ImportPath}};{{.Dir}}' -workDir <the directory> give you an error or the result missing x/tools/.. paths? Strange.

Also confirm that you do not have any special settings enabled, and the extension is using the gopkgs which gopkgs returns. (fyi Go: Locate Go Tools command should give the absolute path to each tool binary).

In general, I think switching to go list is a good idea (one less tool to depend on, and staying consistent with go build). But we need to understand the difference between them (performance and correctness wise). cc/ @uudashr

@hyangah hyangah changed the title "Go: Add Import" command is not working correctly Replace use of 'gopkgs' with 'go list all' Jul 1, 2020
@hyangah hyangah added this to the v0.16.0 milestone Jul 1, 2020
@uudashr
Copy link
Contributor

uudashr commented Jul 3, 2020

Hi there, the reason why we create and use gopkgs is because of performance. At back day, there are no modules, so all the dependencies are goes to gopath and the package on gopath are always increasing. And lot of people using vendors which add more packages to scan.

Nowadays after cleanup by go installation, using go list all seems faster, due to the less packages on the gopath to scan.

On my machine there are 393 packages inside gopath and goroot

gopkgs  0.03s user 0.08s system 88% cpu 0.124 total
go list all  0.53s user 0.31s system 195% cpu 0.432 total

go list all still took longer, but seems still acceptable for user to wait.

@hyangah hyangah removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 22, 2020
@hyangah hyangah modified the milestones: v0.16.0, v0.17.0 Jul 30, 2020
@hyangah hyangah modified the milestones: v0.17.0, v0.18.0 Sep 15, 2020
@hyangah hyangah modified the milestones: v0.18.0, Backlog Oct 21, 2020
@hyangah hyangah changed the title Replace use of 'gopkgs' with 'go list all' tools: replace use of 'gopkgs' with 'go list all' Dec 10, 2020
@hyangah hyangah modified the milestones: Backlog, v0.28.0 Aug 3, 2021
@hyangah hyangah modified the milestones: v0.28.0, v0.29.0 Sep 16, 2021
@hyangah hyangah modified the milestones: v0.29.0, On Deck Oct 26, 2021
@jamalc jamalc self-assigned this Jan 11, 2022
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/379235 mentions this issue: src: replace use of gopkgs with go list

@marwan-at-work
Copy link
Contributor Author

I think golang/go#48545 should close out this issue as well? cc: @hyangah

@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/379494 mentions this issue: src/goCodeAction.ts,src/goImport.ts: replace listPackages with golist

gopherbot pushed a commit that referenced this issue Jan 25, 2022
For #258

Change-Id: I37b00f29bf9bc2d3fee789c3e187a8d8f1efd847
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/379235
Trust: Jamal Carvalho <jamal@golang.org>
Run-TryBot: Jamal Carvalho <jamal@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/382475 mentions this issue: vscode-go: remove references to gopkgs

gopherbot pushed a commit that referenced this issue Feb 3, 2022
Removed remaining references in the codebase to gopkgs.

For #258.

Change-Id: I9257b87316f193f960bd121bfdcc5cd890ac73d4
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/382475
Trust: Jamal Carvalho <jamal@golang.org>
Run-TryBot: Jamal Carvalho <jamal@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
@jamalc
Copy link
Contributor

jamalc commented Feb 3, 2022

This is finished with CL 382457.

@pavlelee
Copy link
Contributor

go list -f "{{.Name}};{{.ImportPath}}" all is incomplete in go mod, nearly two-thirds less.

go list -f "{{.Name}};{{.ImportPath}}" all
gopkgs -format '{{.Name}};{{.ImportPath}};{{.Dir}}' -workDir ./

@golang golang locked and limited conversation to collaborators Mar 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants