-
Notifications
You must be signed in to change notification settings - Fork 18k
x/tools/gopls: gopls-CI Kokoro presubmit failing on CL 315570 due to rsync
failure
#45919
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
Comments
This failure mode may have something to do with #34634, but as far as I can tell that issue was fixed in Go 1.14, so it doesn't explain why the Go 1.13 build would be passing when the Go 1.12 build isn't. |
It looks like the new link should be https://go.googlesource.com/tools/+/refs/heads/master/gopls#supported-go-versions-and-build-systems.
Another possibly related issue is #31871, which was fixed in Go 1.13. |
The documentation for this (once we resolve the broken link) is still sparse. The slightly longer explanation is that we don't have data on the distribution of Go versions among our users, and so try to provide minimal support for older go versions. The "legacy CI" build exists so that we don't break the build, and at least know when we have regressions in older versions. We don't prioritize fixes for the regressions, often skipping tests to resolve them. Sorry you ran into this. I'll improve the documentation, specifically to clarify our policy with respect to these builds.
That seems likely. In that case, the best course of action might be to pin our previous gopls release as the final release supporting 1.12.
AFAICT from internal search results, this is an error that other teams have seen since the Kokoro cluster was upgraded in early 2021. In other words, we don't have direct control over this error message. We might be able to fix it by altering our build script (which is currently just a thin wrapper around |
rsync
failurersync
failure
Does it mean removing 1.12 from the presubmit CI set? Based on #39146, I assumed gopls wouldn't go beyond supporting the latest 4 versions even it decides to go with its own version policy separate from the official go project's. The current Go version is 1.16, so I think it's time to remove 1.12. |
Ah, thank you Hana for finding that issue. I'll add that to the documentation as well. Yes, I mean to propose that we drop the presubmit CI for 1.12, and furthermore allow ourselves to break the build by using e.g. errors rather than xerrors for error wrapping. Per your link, we'd also drop 1.13 CI once 1.17 is released. We've been a bit hesitant to drop support ("if it ain't broke don't break it"), but this approach still incurs cognitive overhead: we have to hold an increasingly distant set of API additions and bug fixes in our heads. Actively dropping support also allows us to announce final support before we encounter an irreconcilable regression. |
Change https://golang.org/cl/316349 mentions this issue: |
Clarify that we have best-effort support for the last 4 major Go versions, and that support for Go releases 3 and 4 versions ago may not block releases and may be dropped if necessary. Also explain that Kokoro CI is not automatically re-run when the result is removed in Gerrit. For golang/go#45919 Change-Id: Ic1480c9276dad9502aaf885b98bb9445deeed0c5 Reviewed-on: https://go-review.googlesource.com/c/tools/+/316349 Trust: Robert Findley <rfindley@google.com> Run-TryBot: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> Reviewed-by: Rebecca Stambler <rstambler@golang.org> TryBot-Result: Go Bot <gobot@golang.org>
Change https://golang.org/cl/315570 mentions this issue: |
This change was produced using 'go mod tidy -go=1.17' with a go command built at CL 315210. This activates lazy loading for the x/tools module, and updates the go.mod file to maintain the lazy-loading invariants (namely, including an explicit requirement for every package transitively imported by the main module). I will send a separate CL to upgrade the gopls module, since it has some awkward interactions with the Go 1.12 presubmit test (golang/go#45919). Note that this does *not* prevent users with earlier go versions from successfully building packages from this module. For golang/go#36460. Change-Id: I1d2a6b28820fa6cb3d02162c14873643e4c689cf Reviewed-on: https://go-review.googlesource.com/c/tools/+/315570 Trust: Bryan C. Mills <bcmills@google.com> Run-TryBot: Bryan C. Mills <bcmills@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Alexander Rakoczy <alex@golang.org>
Ok, we discussed this at our team meeting today and consensus was that we've made effort to keep gopls working with Go 1.12 working in the past, it still works pretty well, and absent usage data would prefer not to just stop supporting it now. Notably AppEngine still supports Go 1.12. We discussed adding a pop-up warning users that Go 1.12 will be deprecated soon. Upon further consideration, the actual CI error is probably not that hard to fix -- I'll just chmod |
It took a couple attempts, but we've now successfully worked around the permissions issue. |
CL 315570 is passing the TryBots and two out of three of the
gopls-legacy
kokoro presubmits (Go 1.13 and Go 1.14).However, the Go 1.12 presubmit is failing due to what appears to be an internal error within the presubmit script. The tests appear to have all run successfully, but then the presubmit script fails due to an
rsync
error attempting to transfergopls/go.mod
somewhere.The failure log also links to a document that no longer exists (https://go.googlesource.com/tools/+/refs/heads/master/gopls/doc/user.md#supported-go-versions), and especially given that it isn't at all clear to me why we're even testing against Go 1.12 — a version that hasn't been supported by the rest of the Go project for over a year now (2020-02-25).
If this presubmit test is indicating a real problem with the CL, then it needs to be improved to more clearly explain what that problem is. Otherwise, the presubmit should be fixed (or disabled) to eliminate the spurious failure.
CC @golang/release @ianthehat @stamblerre @findleyr
Log
The text was updated successfully, but these errors were encountered: