golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.87k stars 17.52k forks source link

cmd/go/internal/list: insufficient error tolerance on listing packages #59785

Open frankli0324 opened 1 year ago

frankli0324 commented 1 year ago

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

$ go version
go version go1.20.3 darwin/amd64 (unrelated)

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
unrelated, preferred not to disclose

What did you do?

I used vscode-go for browsing a repository that I don't have permissions to access it's dependencies. the extension fails to load the workspace complaining that I have no permissions to go get related repos, so vscode can't even index the existing sources that I have. After digging in a little deeper, I found that the command giving the complain is:

https://github.com/golang/vscode-go/blob/master/src/goPackages.ts#L60

go list -e -f '{{.Name}};{{.ImportPath}};{{.Dir}}' std all

What did you expect to see?

I expected go list to tolerate the errors as what it's told to by passing the -e argument.

The -e flag changes the handling of erroneous packages, those that cannot be found or are malformed.

What did you see instead?

instead, it tried to download the missing packages (due to permission errors) and failed if download fails.

Current Workaround

For anyone who ends up in a similar situation, maybe you could try go mod tidy -e and then disable the go modules by setting:

{
    "go.toolsEnvVars": {
        "GO111MODULE": "off"
    }
}

in .vscode/settings.json. The modload package seems to work with go packages.

frankli0324 commented 1 year ago

I'm unfamiliar with golang source but I suspect that it's the AutoVCS flag passed to load.PackagesAndErrors which raises the error

// src/cmd/go/internal/list/list.go#L605-L612
pkgOpts := load.PackageOpts{
    IgnoreImports:      *listFind,
    ModResolveTests:    *listTest,
    AutoVCS:            true,
    SuppressBuildInfo:  !*listExport && !listJsonFields.needAny("Stale", "StaleReason"),
    SuppressEmbedFiles: !*listExport && !listJsonFields.needAny("EmbedFiles", "TestEmbedFiles", "XTestEmbedFiles"),
}
pkgs := load.PackagesAndErrors(ctx, pkgOpts, args)
frankli0324 commented 1 year ago

I dug deeper and found that it's not only about AutoVCS, but:

when not listing modules (without -mod flag), if go modules is enabled (which is default now), go list doesn't tell load.PackagesAndErrors to execute modload.LoadPackages allowing errors, which results in process termination when an error is occurred

I think that modload.LoadPackages is also used when executing go mod tidy. It also accepts an AllowError option.

frankli0324 commented 1 year ago

After getting around this by recompiling the go binary with listE properly propagated to modload, I noticed that gopls attempted to invoke go list without -e a few times:

go list -modfile=/var/folders/1b/jvpybz8s7lq9hb5llmfwy0_c0000gn/T/go.eb3c3e8094d54e5efda22499fbea51c5ebf8c8ed08419e55fbb934c47a48f885.3405178131.mod -mod=mod -f {{context.GOARCH}} {{context.Compiler}} -- unsafe
go list -mod=readonly -m -f {{.Path}} {{.Dir}} {{.GoMod}} {{.GoVersion}} {{range context.ReleaseTags}}{{if eq . "go1.14"}}{{.}}{{end}}{{end}}

they all exited with code 1 due to the repo permission issue. should I file a issue for gopls as well?

dmitshur commented 1 year ago

CC @bcmills, @matloob.

frankli0324 commented 1 year ago

@bcmills @matloob @dmitshur sorry to disturb but have you digged into/had a look at this?

frankli0324 commented 1 year ago

does this really need more than a month to investigate? 🤔 at least some progress would be nice please?

bcmills commented 1 year ago

This issue is in the backlog, and Go is an open-source project. If you would like to help move it along, I would be glad to review a change to fix it.

https://go.dev/doc/contribute