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.

Sign up for GitHub

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

cmd/go: awkward "list -json" error reporting for a vendor subdirectory that does not contain a Go package #58418

Open
nmiyake opened this issue Feb 8, 2023 · 5 comments
Open

cmd/go: awkward "list -json" error reporting for a vendor subdirectory that does not contain a Go package #58418

nmiyake opened this issue Feb 8, 2023 · 5 comments
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@nmiyake
Copy link
Contributor

nmiyake commented Feb 8, 2023 •
edited

What version of Go are you using (go version)?

$ go version
go version go1.20 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOOS="darwin"
GOARCH="amd64"

What did you do?

Run the following to create a module that vendors its dependencies and then run go list -e -json:

mkdir go-list-repro && cd $_
echo "module go-list-repro" > go.mod
echo 'package main; import _ "golang.org/x/tools/go/packages"; func main(){}' > main.go
go mod tidy
go mod vendor
go list -e -json golang.org/x/tools

What did you expect to see?

The JSON struct information for the requested module should contain the Dir field that indicates the directory for the module (the path should be in the vendor directory).

What did you see instead?

The JSON struct printed as part of the output does not have a Dir field:

go: finding module for package golang.org/x/tools
{
	"ImportPath": "golang.org/x/tools",
	"Match": [
		"golang.org/x/tools"
	],
	"Incomplete": true,
	"Error": {
		"ImportStack": [],
		"Pos": "",
		"Err": "cannot query module due to -mod=vendor\n\t(Go version in go.mod is at least 1.14 and vendor directory exists.)"
	}
}

This is the output of the command in the same directory using Go 1.19:

{
	"Dir": "/Volumes/git/go/src/github.com/nmiyake/go-list-repro/vendor/golang.org/x/tools",
	"ImportPath": "golang.org/x/tools",
	"Target": "/Volumes/git/go/pkg/darwin_amd64/github.com/nmiyake/go-list-repro/vendor/golang.org/x/tools.a",
	"Root": "/Volumes/git/go",
	"Match": [
		"golang.org/x/tools"
	],
	"Incomplete": true,
	"Error": {
		"ImportStack": [
			"golang.org/x/tools"
		],
		"Pos": "",
		"Err": "no Go files in /Volumes/git/go/src/github.com/nmiyake/go-list-repro/vendor/golang.org/x/tools"
	}
}

For context, the code I wrote that depends on this is doing the following:

  • "Given a local module that vendors its dependencies, find all of the paths to the imported modules in the vendor directory"

The output of go list through Go 1.19 allowed this to be done, but the behavior change in Go 1.20 breaks this workflow, and I'm unsure why the output changed in this case.

The error "cannot query module due to -mod=vendor\n\t(Go version in go.mod is at least 1.14 and vendor directory exists.)" is also misleading, since if the query is changed to be a package (golang.org/x/tools/go/packages), the list operation succeeds and prints the package information (even though the same condition of "Go version in go.mod is at least 1.14 and vendor directory exists." is still true).
@nmiyake nmiyake mentioned this issue Feb 8, 2023
Closed
@seankhliao seankhliao added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Feb 8, 2023
@seankhliao
Copy link
Member

cc @bcmills @matloob

@bcmills
Copy link
Contributor

bcmills commented Feb 8, 2023

The Dir field does not apply to modules, only to packages, and golang.org/x/tools is not a package (it has no .go source files).

The removal of the Target field may have been a side effect of the fix for #37015, which intentionally fixed a bug that caused the Target field to be inconsistently populated in module mode.

@bcmills bcmills mentioned this issue Feb 8, 2023
Closed
@nmiyake
Copy link
Contributor Author

nmiyake commented Feb 8, 2023

Yes, understood that the target here is not a package, but it looks like the handling changed in a way that appears incorrect -- so this is a more scoped case of "behavior when calling go list on a module/package path that does not contain go files that is vendored".

The output in Go 1.19 correctly stated "Err": "no Go files in /Volumes/git/go/src/github.com/nmiyake/go-list-repro/vendor/golang.org/x/tools" as the error, and still populated the fields like Dir (which answered the question "what is the directory on disk of this module/the package that would exist if a Go file were in this directory). I understand that the Dir field being populated in the error case is not necessarily a behavior that is guaranteed/worth restoring, so while I did find it useful, I understand if that behavior will not be restored.

In contrast, the error output in 1.20 of "cannot query module due to -mod=vendor\n\t(Go version in go.mod is at least 1.14 and vendor directory exists.)" does seem incorrect.

@bcmills
Copy link
Contributor

bcmills commented Feb 8, 2023

vendor/modules.txt lists the directories that contain Go packages, and that list does not include the named package.

We shouldn't even be spending the I/O operations to read the directory if we don't expect to find a package there (although we may mistakenly be doing so today). If we haven't read (or shouldn't have read) the directory, then we also shouldn't claim anything about whether that directory contains Go source files.

So I don't think the no Go files error was correct, although I do agree that the cannot query module error message is also not particularly helpful in vendor mode. 🤔

@bcmills bcmills added this to the Backlog milestone Feb 8, 2023
@bcmills bcmills changed the title cmd/go: "list -json" does not populate "Dir" field for module in vendor directory (Go 1.20 regression) cmd/go: awkward "list -json" error reporting for a vendor subdirectory that does not contain a Go package Feb 8, 2023
@nmiyake
Copy link
Contributor Author

nmiyake commented Feb 8, 2023

Thank you for the context -- in conjunction with looking at the change that introduced the behavior, I understand the mechanics better now.

The behavior difference is due to the importFromModules change in src/cmd/go/internal/modload/import.go, with the crux of it at https://go-review.googlesource.com/c/go/+/434095/11/src/cmd/go/internal/modload/import.go#326.

Both before and after the change, the following is executed for the vendor directory (so I/O is happening):

vendorDir, vendorOK, _ := dirInModule(path, "", filepath.Join(modRoot, "vendor"), false)

Prior to the change, the vendorDir value was always returned after being computed while, after the change, if vendorOK is not true, "" is returned as the directory rather than vendorDir.

I'll leave this issue open since I do think there are still things that could be improved about the current output, but also understand that the behavior that I was consuming was an incidental one that happened to work but was not technically correct behavior, so will find a workaround for that on my end. Thank you.

@nmiyake nmiyake mentioned this issue Feb 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@nmiyake @bcmills @seankhliao