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/go/packages: overlays cause loading of unneeded dependencies #32538

Closed
stamblerre opened this issue Jun 10, 2019 · 2 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@stamblerre
Copy link
Contributor

Forked from #32457.

The similar issue reproduces in golangci-lint: after updating x/tools from 685fecacd0a0 to 755ce86c7629 go/packages started loading dependencies when not needed.
As I understand the regression was introduced in this overlays commit.
And this code adds all dependencies into needPkgsSet and after this to needPkgs, which are used to run go list on them:

	// Do another pass now that new packages have been created to determine the
	// set of missing packages.
	for _, pkg := range response.Packages {
		for _, imp := range pkg.Imports {
			pkgPath := toPkgPath(imp.ID)
			if _, ok := havePkgs[pkgPath]; !ok {
				needPkgsSet[pkgPath] = true
			}
		}
	}

/cc @matloob

@stamblerre stamblerre added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 10, 2019
@matloob
Copy link
Contributor

matloob commented Jun 12, 2019

Unfortunately, it's not always possible to avoid loading extra dependencies, and go/packages can't tell if they're unneeded unless the transitive set of dependencies is not changed at all.

I'll send a CL to avoid loading extra dependencies if the transitive set of dependencies is not changed. But I don't think this problem can be solved satisfactorily until go list starts handling overlays itself...

@gopherbot
Copy link

Change https://golang.org/cl/181917 mentions this issue: go/packages: avoid loading some packages in when computing overlays

@dmitshur dmitshur changed the title go/packages: overlays cause loading of unneeded dependencies x/tools/go/packages: overlays cause loading of unneeded dependencies Jun 17, 2019
@gopherbot gopherbot added this to the Unreleased milestone Jun 17, 2019
gopherbot pushed a commit to golang/tools that referenced this issue Jun 27, 2019
In some cases, it's safe to avoid loading additional packages
when computing overlays. However it's not always safe to do so.
Avoid some unnecessary loads when it's completely safe to do so.

Updates golang/go#32538

Change-Id: Ie12204735940a540c9b3f29742f8479bcab5f077
Reviewed-on: https://go-review.googlesource.com/c/tools/+/181917
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@matloob matloob closed this as completed Aug 28, 2019
@golang golang locked and limited conversation to collaborators Aug 27, 2020
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.
Projects
None yet
Development

No branches or pull requests

3 participants