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

Jump to bottom

go/packages: returned packages are in incorrect order #30826

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

go/packages: returned packages are in incorrect order #30826

josharian opened this issue Mar 14, 2019 · 2 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.

package main

import (
	"fmt"

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

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

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

When I run this, I get:

$ go run bug.go 
fmt
golang.org/x/tools/cmd/compilebench

I expected:

$ go run bug.go 
golang.org/x/tools/cmd/compilebench
fmt

Load is documented to return the packages in the order they were passed to Load. (And this is important; if given relative package names, I have no other way of correlating them to the packages I receive back from Load.)

This does not reproduce if Mode is LoadFiles; it reproduces with all other modes.

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 josharian mentioned this issue Mar 14, 2019
Closed
@ianthehat
Copy link

The input to Load is "patterns", and the documentation says "in general it is not possible to determine which packages correspond to which patterns"
I don't believe there is even an implied promise about the order matching, so I am surprised the documentation made you think that, we should probably fix the documentation.
I think if you need to do this, I think you need to make multiple independent calls to load instead.

@josharian
Copy link
Contributor Author

I think if you need to do this, I think you need to make multiple independent calls to load instead.

That's unfortunate. I guess I understand. It's a cumbersome and expensive way to resolve a relative path. I might be remembering the order-preservation promise from go/loader.

@josharian josharian mentioned this issue Mar 28, 2019
Closed
josharian added a commit to josharian/go-fuzz that referenced this issue Mar 31, 2019
@josharian
go-fuzz-build: improve pkg resolution
f6dbad7 
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
@josharian @dvyukov
go-fuzz-build: improve pkg resolution
fc0bf08 
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
@josharian @dvyukov
go-fuzz-build: improve pkg resolution
30ad47f 
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.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@josharian @ianthehat @gopherbot