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

x/tools/gopls: build mod diagnostics with GOWORK=off #50862

Closed
findleyr opened this issue Jan 27, 2022 · 6 comments
Closed

x/tools/gopls: build mod diagnostics with GOWORK=off #50862

findleyr opened this issue Jan 27, 2022 · 6 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. modules Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.9.0

Comments

@findleyr
Copy link
Member

gopls relies on errors produced by the go command to suggest fixes to our users, but when using go.work, we don't get all of these errors. We can work around this by running go commands with -workfile=off to build our diagnostics.

Example (based on gopls/internal/regtest/workspace.TestUseGoWork):

[workspace]> ls -R
.:
go.work  moda  modb  tags

./moda:
a

./moda/a:
a.go  go.mod  go.sum

./modb:
b.go  go.mod  go.sum
[workspace]> cat ./modb/go.mod
module b.com

require golang.org/x/tools v0.1.9
[workspace]> cat ./moda/a/go.mod
module a.com

require b.com v1.2.3
[workspace]> cd modb
[modb]> GOPROXY=off go list all
../moda/a/a.go:4:2: b.com@v1.2.3: module lookup disabled by GOPROXY=off
[modb]> GOPROXY=off go list -workfile=off all
go: golang.org/x/tools@v0.1.9: module lookup disabled by GOPROXY=off
go: golang.org/x/tools@v0.1.9: module lookup disabled by GOPROXY=off

CC @matloob
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jan 27, 2022
@gopherbot gopherbot added this to the Unreleased milestone Jan 27, 2022
@findleyr findleyr modified the milestones: Unreleased, gopls/v0.8.0 Jan 27, 2022
@findleyr findleyr assigned matloob and findleyr and unassigned matloob Jan 27, 2022
@bcmills
Copy link
Contributor

bcmills commented Jan 27, 2022

b.com@v1.2.3: module lookup disabled by GOPROXY=off

That error message looks deeply wrong to me. module lookup disabled should imply that we are looking up an imported package, but b.com@v1.2.3 implies that the module containing the package is already known. We should either get a module@version in the error or a module lookup disabled, but never both in the same error.

@bcmills
Copy link
Contributor

bcmills commented Jan 27, 2022

(That is: I agree that there is at least one bug here, but it may be more fundamental than workspace mode. 😅)

@findleyr
Copy link
Member Author

Some more interesting behavior! For some reason -e -json behaves quite differently with -workfile=off.

[modb]> GOPROXY=off go list -e -json b.com/...
{
        "Dir": "/home/rfindley/tmp/workspace/modb",
        "ImportPath": "b.com",
        "Name": "b",
        "Root": "/home/rfindley/tmp/workspace/modb",
        "Module": {
                "Path": "b.com",
                "Main": true,
                "Dir": "/home/rfindley/tmp/workspace/modb",
                "GoMod": "/home/rfindley/tmp/workspace/modb/go.mod"
        },
        "Match": [
                "b.com/..."
        ],
        "Stale": true,
        "StaleReason": "build ID mismatch",
        "GoFiles": [
                "b.go"
        ]
}
{
        "ImportPath": "b.com/...",
        "Match": [
                "b.com/..."
        ],
        "Incomplete": true,
        "Error": {
                "ImportStack": [],
                "Pos": "",
                "Err": "b.com@v1.2.3: module lookup disabled by GOPROXY=off"
        }
}
[modb]> GOPROXY=off go list -e -json -workfile=off b.com/...
go: golang.org/x/tools@v0.1.9: module lookup disabled by GOPROXY=off
go: golang.org/x/tools@v0.1.9: module lookup disabled by GOPROXY=off

@findleyr findleyr modified the milestones: gopls/v0.8.0, gopls/v0.8.1 Feb 17, 2022
@findleyr findleyr changed the title x/tools/gopls: build mod diagnostics with -workfile=off x/tools/gopls: build mod diagnostics with GOWORK=off Mar 12, 2022
@findleyr findleyr modified the milestones: gopls/v0.8.1, gopls/v0.8.2 Mar 14, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/405259 mentions this issue: internal/lsp/cache: extract module load errors when go.work is used

@findleyr
Copy link
Member Author

@bcmills @matloob is it valid to depend on this behavior from go list, that errors will be embedded in synthetic package results corresponding to the query string?

i.e. this result:

{
        "ImportPath": "b.com/...",
        "Match": [
                "b.com/..."
        ],
        "Incomplete": true,
        "Error": {
                "ImportStack": [],
                "Pos": "",
                "Err": "b.com@v1.2.3: module lookup disabled by GOPROXY=off"
        }
}

It works, and solves our problem, but feel rather incorrect.

@bcmills
Copy link
Contributor

bcmills commented May 12, 2022

Hmm. In general if we can't determine the import path(s) for a pattern we should report an error for that pattern, so that should be safe to rely on.

I agree that it feels a bit iffy, but I think the only better alternative might be to omit the ImportPath field entirely for the error, and I'm not sure that's any better for tools. 😅

@findleyr findleyr modified the milestones: gopls/v0.8.4, gopls/v0.9.0 May 18, 2022
@golang golang locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. modules Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.9.0
Development

No branches or pull requests
4 participants
@bcmills @gopherbot @matloob @findleyr