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

go/packages: returned packages are deduplicated #30828

Closed
josharian opened this issue Mar 14, 2019 · 3 comments
Closed

go/packages: returned packages are deduplicated #30828

josharian opened this issue Mar 14, 2019 · 3 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.

Comments

@josharian
Copy link
Contributor

Using Go 1.12.

// +build ignore

package main

import (
	"fmt"

	"golang.org/x/tools/go/packages"
)

func main() {
	cfg := &packages.Config{
		Mode: packages.LoadImports,
	}

	initial, err := packages.Load(cfg, "fmt", "fmt")
	if err != nil {
		panic(err)
	}
	for _, x := range initial {
		fmt.Println(x.PkgPath)
	}
}

Got:

$ go run bug.go
fmt

Want:

$ go run bug.go
fmt
fmt

Related: #30826. This is important to me for the same reasons.

(For the record, neither of these bugs is theoretical. I hit both of them working on go-fuzz; this one I hit while trying to work around #30826.)

cc @matloob

@josharian josharian added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 14, 2019
@josharian
Copy link
Contributor Author

@ianthehat's comments on #30826 are apropos here as well.

@matloob
Copy link
Contributor

matloob commented Mar 14, 2019

Hi -- just want to make sure that you intended to close this bug.

@matloob
Copy link
Contributor

matloob commented Mar 14, 2019

Oh, just read #30826... Yeah, you'd have to make two separate calls to Load for this usecase. Once we get the changes to LoadMode in, that should be relatively cheap to do.

josharian added a commit to josharian/go-fuzz that referenced this issue Mar 31, 2019
Prior to this change, go-fuzz-build assumed that the
first package in the slice returned by packages.Load
corresponded to the package requested by the user.
This assumption was faulty, although it worked accidentally.

This change adds another package.Load call to explicitly
resolve the package to be fuzzed, and does more careful validation of it.

While we're here, do some minor cleanup:

* Request cheaper loading of std. All we need is package paths.
* Simplify processing of the result of loading std.
* Store the fuzz package, rather than its package path, in Context.
  This will be helpful in other planned work (e.g. adding fuzz.F).

Updates golang/go#30826 and golang/go#30828
dvyukov pushed a commit to dvyukov/go-fuzz that referenced this issue Apr 1, 2019
Prior to this change, go-fuzz-build assumed that the
first package in the slice returned by packages.Load
corresponded to the package requested by the user.
This assumption was faulty, although it worked accidentally.

This change adds another package.Load call to explicitly
resolve the package to be fuzzed, and does more careful validation of it.

While we're here, do some minor cleanup:

* Request cheaper loading of std. All we need is package paths.
* Simplify processing of the result of loading std.
* Store the fuzz package, rather than its package path, in Context.
  This will be helpful in other planned work (e.g. adding fuzz.F).

Updates golang/go#30826 and golang/go#30828
bradleyjkemp pushed a commit to bradleyjkemp/simple-fuzz that referenced this issue Oct 23, 2019
Prior to this change, go-fuzz-build assumed that the
first package in the slice returned by packages.Load
corresponded to the package requested by the user.
This assumption was faulty, although it worked accidentally.

This change adds another package.Load call to explicitly
resolve the package to be fuzzed, and does more careful validation of it.

While we're here, do some minor cleanup:

* Request cheaper loading of std. All we need is package paths.
* Simplify processing of the result of loading std.
* Store the fuzz package, rather than its package path, in Context.
  This will be helpful in other planned work (e.g. adding fuzz.F).

Updates golang/go#30826 and golang/go#30828
@golang golang locked and limited conversation to collaborators Mar 13, 2020
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

No branches or pull requests

3 participants