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/internal/lsp: TryBot failure in upgradedep #39504

Closed
jayconrod opened this issue Jun 10, 2020 · 9 comments
Closed

x/tools/internal/lsp: TryBot failure in upgradedep #39504

jayconrod opened this issue Jun 10, 2020 · 9 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.4.2

Comments

@jayconrod
Copy link
Contributor

When testing CL 237017, I saw TryBot failures on two tests:

  • TestLSP/Modules/testdata/upgradedep/CodeLens/go.mod
  • TestLSP/Modules/testdata/upgradedep/Link/go.mod

I couldn't reproduce this locally (darwin, amd64) or in a VM (linux, amd64). I was able to reproduce it through gomote on linux-amd64. The failure doesn't happen every time; I had to use -count=5 or so.

The failing command is:

go list -modfile=/workdir/tmp/go.lsp.218695923.mod -mod readonly -u -m all

The .mod file is the same as the one in testdata. There's no .sum file in testdata, so I'm not really sure how that gets populated when this test passes.

I'd actually expect the command above to fail when go.sum is absent or incomplete. It should write sums for .mod files loaded during MVS. It should fail because of -mod readonly.

Given that this happens irregularly, I suspect the go list -u -m command above is racing with whatever sets up the test workspace and populates the go.sum file.

cc @stamblerre @ianthehat
@jayconrod jayconrod added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. modules gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Jun 10, 2020
@gopherbot gopherbot added this to the Unreleased milestone Jun 10, 2020
@heschi
Copy link
Contributor

heschi commented Jun 10, 2020

There are somewhat similar flakes on the builders across multiple tests: https://build.golang.org/log/e954e51ef512c8b2149f6233c26f33ca3fa31294
https://build.golang.org/log/471447bf31a433d96ae5b53e4e015c9c8c1f8b3f

@stamblerre
Copy link
Contributor

Thanks - I didn't realize these tests were already flaky. Converting to regtests sounds like the right path to me - I'll try to do that today.

@heschi
Copy link
Contributor

heschi commented Jun 10, 2020

They're not -- those are completely unrelated tests failing with what looks like a similar root cause to me.

@stamblerre
Copy link
Contributor

Oh thanks for clarifying, should've checked the logs. Maybe it is because our testdata has a go.mod but not go.sum. Still, the initial workspace load should handle setting up the temporary go.sum and that should definitely run before any of these tests start.

@pjweinb
Copy link

pjweinb commented Jun 10, 2020

Me too: modest changes in unrelated code, and ran trybots 3 times, and each time 1/10 failed in TestCommandLine, each time a different trybot.

@pjweinb
Copy link

pjweinb commented Jun 10, 2020

here's references to the logs:
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/e92be18f/windows-386-2008_d2cf68fe.log
Failed on linux-amd64 (Go 1.14.x): https://storage.googleapis.com/go-build-log/83b181c6/linux-amd64_7bcc844b.log
Failed on openbsd-amd64-64: https://storage.googleapis.com/go-build-log/e92be18f/openbsd-amd64-64_121cdfa3.log

@gopherbot
Copy link

Change https://golang.org/cl/237419 mentions this issue: internal/lsp: await the initial workspace load in ModHandle

@stamblerre
Copy link
Contributor

We're fairly certain that this is caused by our reuse of the same temporary go.mod file for every go command call. I'll put up a follow-up CL that switches to use one temporary go.mod file per invocation.

@findleyr findleyr mentioned this issue Jun 10, 2020
Closed
gopherbot pushed a commit to golang/tools that referenced this issue Jun 12, 2020
@stamblerre
internal/lsp: await the initial workspace load in ModHandle
3c1b287 
ModHandle races with the initial workspace load if the go.mod file does
not yet exist. We should await for the initial workspace load to
complete before proceeding with update codelenses, etc.

Part of trying to figure out the flakes in golang/go#39504.
Also a few staticcheck fixes, and fix the Windows line endings in
fill_struct.go, because `git gofmt` complains.

Change-Id: Ide21a47137390792d1afb924740cff0bb6f0b764
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237419
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/237517 mentions this issue: internal/lsp: use a new temporary go.mod for every go list call

@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.5.0 Jun 17, 2020
@stamblerre stamblerre modified the milestones: gopls/v0.5.0, gopls/v0.4.2 Jun 18, 2020
@golang golang locked and limited conversation to collaborators Jun 18, 2021
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 NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.4.2
Development

No branches or pull requests
5 participants
@jayconrod @pjweinb @stamblerre @gopherbot @heschi