Skip to content

x/build: catch problems in x/ repos gated by module go directive version sooner #67791

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

Closed
dmitshur opened this issue Jun 3, 2024 · 10 comments
Closed
Assignees
Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done. new-builder
Milestone

Comments

@dmitshur
Copy link
Member

dmitshur commented Jun 3, 2024

Issue #67783 is an example of a test failure that happened only at tip (future Go 1.23 to be) and when the go directive in x/net is increased to Go language version 1.23. Bumping x/net go directive to 1.23 will not happen on its own soon since Go 1.22 and 1.21 are still supported now, but it would be good to have at least one builder that could catch such problems sooner anyway. This is a tracking issue for that.

Edit: Discussion here shows another case where having this implemented would help, by letting vet changes be rolled out more gradually without delay in signal.

More examples of issues that this could help with:

CC @golang/release.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 3, 2024
@dmitshur dmitshur added this to the Unreleased milestone Jun 3, 2024
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Jun 3, 2024
@dmitshur dmitshur moved this to Planned in Go Release Jun 3, 2024
@findleyr
Copy link
Member

This is a great idea. I suspect x/tools will pick up some problems when its go.mod directive is updated.

@dmitshur
Copy link
Member Author

dmitshur commented Sep 6, 2024

A sketch of how this can be implemented:

  • add an input property to golangbuild, something like upgrade_go_directive
    • when set, builders run go get go@{target go directive} in each module before their usual go test ./...
  • add a builder runmod that uses the new property, something like "tiplang", and have one linux/amd64 builder for each golang.org/x repository at tip (main Go repo doesn't need it)

Using x/build as an example, it would have a new post-submit builder named:

x_build-gotip-linux-amd64-tiplang

And the top-level module test sequence would have this one extra command:

 [...] # discover modules to test, download module dependencies in advance, etc.
+go get go@1.24
 go test ./...

I think we can target doing this this at tip, one builder per x/ repo, and afterwards can consider if there's a benefit to doing it for release branches and/or more than just linux/amd64. We'll also need to find a good place where the 1.24 value is determined (config vs golangbuild), aiming to make it easier to maintain the value over time. This can be a starting point.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 6, 2024
@mknyszek mknyszek self-assigned this Jan 13, 2025
@mknyszek
Copy link
Contributor

mknyszek commented Jan 13, 2025

We'll also need to find a good place where the 1.24 value is determined (config vs golangbuild), aiming to make it easier to maintain the value over time. This can be a starting point.

golangbuild already has logic to fetch the latest release version from go.dev for perf builders, so we can reuse that and not have any configuration at all.

Is that sufficient? Or is the problem that it's always one more than whatever that is? (We can always increment it in golangbuild from the latest one.)

@dmitshur
Copy link
Member Author

dmitshur commented Jan 13, 2025

Yeah, determining it automatically without configuration, like for perf builders, sounds good. (If we later on realize we want more flexibility for some reason, we can add that flexibility afterwards; no need to do it pre-emptively.) Note that the most canonical source of the "latest tip version" is probably the internal/goversion package itself, and we also have code that reads that (e.g., here). golangbuild might be able to just read that file from disk after fetching the toolchain. (But going through the go.dev/dl/?mode=json API is probably also fine for this.)

One correction to my previous comment – I realized now that go get go@1.24 can only be used after a released version is out (or at minimum a pre-release version, e.g., go get go@1.24rc1), so we'll likely want to edit the go.mod file with the lower-level go mod edit (e.g., go mod edit -go=1.24) instead of the higher-level go get.

@dmitshur dmitshur moved this from Planned to In Progress in Go Release Jan 13, 2025
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/643316 mentions this issue: main.star: add tiplang builders

gopherbot pushed a commit to golang/build that referenced this issue Jan 17, 2025
This change adds tiplang builders for subrepos which upgrade the
repository's go.mod to the next Go version (tip toolchain's version)
before running tests.

Only for post-submit and non-special subrepos. (For now?)

For golang/go#67791.

Change-Id: If56bcaf49c27458309989ebdee4421c2382c2490
Reviewed-on: https://go-review.googlesource.com/c/build/+/643316
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@mknyszek
Copy link
Contributor

The build-side work for this is all done now, but I haven't closed the issue because I want to confirm that this build which ran in the new mode (ignore the builder name) has a real failure (it should be showing up soon in postsubmit as well).

@dmitshur
Copy link
Member Author

Looks real; it reproduces consistently with go1.24rc2 darwin/arm64. Glad to see it paying off already!

tools $ go test -count=1 ./cmd/signature-fuzzer/internal/fuzz-generator 
ok  	golang.org/x/tools/cmd/signature-fuzzer/internal/fuzz-generator	6.828s
tools $ go mod edit -go=1.24
tools $ go test -count=1 ./cmd/signature-fuzzer/internal/fuzz-generator
wraprand consistency check failed:
 checker: {f32:3 f64:12 i:76}
 caller: {f32:3 f64:12 i:77}
--- FAIL: TestIsBuildable (0.13s)
panic: bad [recovered]
	panic: bad
[…]
tools $ go mod edit -go=1.22.0                                         
tools $ go test -count=1 ./cmd/signature-fuzzer/internal/fuzz-generator
ok  	golang.org/x/tools/cmd/signature-fuzzer/internal/fuzz-generator	7.012s

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/647776 mentions this issue: main.star: set known issues for -tiplang builder in x/crypto and x/tools

gopherbot pushed a commit to golang/build that referenced this issue Feb 7, 2025
Annotate those builders with corresponding tracking issues. Since these
findings are not necessarily causing a problem today, it might be some
time before they're investigated and fixed.

This will also unblock the golang.org/x tagging workflow.

For golang/go#67791.

Change-Id: I048f08b05df546c10ff42fc267cd645eb3fb049f
Reviewed-on: https://go-review.googlesource.com/c/build/+/647776
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@dmitshur
Copy link
Member Author

dmitshur commented Feb 7, 2025

As far as I can tell everything's working as expected. I filed #71613 and #71612 for the current findings.

Is there anything more to do here @mknyszek or ready to call this done?

@mknyszek
Copy link
Contributor

mknyszek commented Feb 7, 2025

I think this is done! Closing.

@mknyszek mknyszek closed this as completed Feb 7, 2025
@github-project-automation github-project-automation bot moved this from In Progress to Done in Go Release Feb 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done. new-builder
Projects
Archived in project
Development

No branches or pull requests

4 participants