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.

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

Establish minimum Go version of 1.21, with toolchain of Go 1.22 #522

Merged
merged 1 commit into from
Aug 9, 2024

Conversation

brandur
Copy link
Contributor

@brandur brandur commented Aug 9, 2024

This one related to #520, in which it turns out that our Go 1.21 build
step has actually been automatically upgrading itself to Go 1.22, so
under the radar that we didn't notice.

Here, leverage a Go "toolchain" to establish a preferred version of Go
for the project, but keeping a go directive in go.mods that's at the
1.21 lower bound that we're trying to support. The presence of a
toolchain directive prevents go mod tidy from upgrading the go
directive to the latest version of Go installed.

I found the easiest way to add the toolchain directive to be with go get [2] like:

go get go@1.21 toolchain@1.22.5

Maintaining this correctly is going to be a little tricky. This is one
of those classic Go features that kind of works, but various Go commands
will perform all kinds of confusing behavior like stripping directives
out of your file if there's anything even a tiny bit wrong, and with no
explanation whatsoever.

I found that to get this working I had to started with the "inner"
dependencies like rivershared and rivertype, go get Go/toolchain
there first, then work my way up the stack all the way up to the main
project.

I'm going to try and follow this up with some tooling to help make this
easier for ourselves, and a build check that verifies in CI that nothing
gets accidentally regressed as we make changes because this is very
easy to do accidentally.

Fixes #520.

[1] https://go.dev/doc/toolchain
[2] https://go.dev/doc/toolchain#get

@brandur brandur force-pushed the brandur-min-go-1.21 branch 2 times, most recently from 9dd8c3a to b1cb50c Compare August 9, 2024 01:11
// replace github.com/riverqueue/river/rivertype => ../../rivertype
replace github.com/riverqueue/river/riverdriver/riverpgxv5 => ../../riverdriver/riverpgxv5

replace github.com/riverqueue/river/rivertype => ../../rivertype
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This'll need the two stage release. Without these replace directives, go mod tidy detects that Go 1.21 can't be supported and therefore removes the toolchain directive and changes go from 1.21 to 1.22.

replace directives to be removed during second phase of release.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this why setup-go is still upgrading us to 1.22?

Setup go version spec 1.21
Found in cache @ /opt/hostedtoolcache/go/1.21.[12](https://github.com/riverqueue/river/actions/runs/10312078359/job/28546717441#step:4:13)/x64
Added go to the path
Successfully set up Go version 1.21
go: downloading go1.22.5 (linux/amd64)
/opt/hostedtoolcache/go/1.21.12/x64/bin/go env GOMODCACHE
/opt/hostedtoolcache/go/1.21.12/x64/bin/go env GOCACHE
/home/runner/go/pkg/mod
/home/runner/.cache/go-build
Cache Size: ~1[16](https://github.com/riverqueue/river/actions/runs/10312078359/job/28546717441#step:4:17) MB (122068102 B)
/usr/bin/tar -xf /home/runner/work/_temp/3fd374b5-c372-4fe0-bb5a-72c9ec40afc9/cache.tzst -P -C /home/runner/work/river/river --use-compress-program unzstd
Received 12[20](https://github.com/riverqueue/river/actions/runs/10312078359/job/28546717441#step:4:21)68102 of 122068102 (100.0%), 116.4 MBs/sec
Cache restored successfully
Cache restored from key: setup-go-Linux-ubuntu22-go-1.22.5-7e42e876c7fc43348578ac5ddfec93c5643eb4a8c880ab849c10c6fbcbbb6cc8
go version go1.[22](https://github.com/riverqueue/river/actions/runs/10312078359/job/28546717441#step:4:23).5 linux/amd64

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that might be something else -- even Go 1.21 is capable of upgrading to a Go 1.22 toolchain, so the default GOTOOLCHAIN value of auto is causing Go to install the matrix version, but then fetch one according to toolchain in go.mod.

@brandur brandur force-pushed the brandur-min-go-1.21 branch 7 times, most recently from 391cf8c to c09a3ac Compare August 9, 2024 02:41
# than trying to fetch one according to a `toolchain` value in `go.mod`.
# This ensures that we're really running the Go version in the CI matrix
# rather than one that the Go command has upgraded to automatically.
GOTOOLCHAIN: local
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bgentry I think this is the magic invocation needed to stop CI from autoupgrading its toolchain. A value of "local" tells Go to ignore a go.mod toolchain directive and use the version in the installed Go command.

See this section of the docs:

https://go.dev/doc/toolchain#select

When GOTOOLCHAIN is set to local, the go command always runs the bundled Go toolchain.

@@ -1,16 +1,20 @@
module github.com/riverqueue/river/cmd/river

go 1.22.5
go 1.21
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bgentry I changed all of these to 1.21 instead of a specific patch version like 1.21.13.

1.21 == 1.21.0

I think this is probably right because it gives maximum flexibility for people installing the package. They should still use the latest possible patch version of course, but it's not always possible due to their local build toolchain. Specifying 1.21.0 just seems like the much easier choice rather than trying to find a minimum patch version that's most universally available.

Do you agree?

@brandur brandur requested a review from bgentry August 9, 2024 02:46
@@ -78,6 +78,8 @@ func TestReplaceNamed(t *testing.T) {
{Desc: "SliceUint64", ExpectedSQL: "SELECT ARRAY[123,124]", InputSQL: "SELECT @slice_uint64", InputArgs: map[string]any{"slice_uint64": []uint64{123, 124}}},
}
for _, tt := range testCases {
tt := tt
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something I hadn't realized is that golangci-lint is actually reading the Go version from go.mod. So as we reset the min version down to 1.21, the lint requiring that loop vars are captured kicked back in.

This one related to #520, in which it turns out that our Go 1.21 build
step has actually been automatically upgrading itself to Go 1.22, so
under the radar that we didn't notice.

Here, leverage a Go "toolchain" to establish a preferred version of Go
for the project, but keeping a `go` directive in `go.mod`s that's at the
1.21 lower bound that we're trying to support. The presence of a
`toolchain` directive prevents `go mod tidy` from upgrading the `go`
directive to the latest version of Go installed.

I found the easiest way to add the toolchain directive to be with `go
get` [2] like:

    go get go@1.21 toolchain@1.22.5

Maintaining this correctly is going to be a little tricky. This is one
of those classic Go features that kind of works, but various Go commands
will perform all kinds of confusing behavior like stripping directives
out of your file if there's anything even a tiny bit wrong, and with no
explanation whatsoever.

I found that to get this working I had to started with the "inner"
dependencies like `rivershared` and `rivertype`, `go get` Go/toolchain
there first, then work my way up the stack all the way up to the main
project.

I'm going to try and follow this up with some tooling to help make this
easier for ourselves, and a build check that verifies in CI that nothing
gets accidentally regressed as we make changes because this is _very_
easy to do accidentally.

Fixes #520.

[1] https://go.dev/doc/toolchain
[2] https://go.dev/doc/toolchain#get
@brandur brandur force-pushed the brandur-min-go-1.21 branch from c09a3ac to 0b54794 Compare August 9, 2024 02:56
@brandur
Copy link
Contributor Author

brandur commented Aug 9, 2024

Thx.

@brandur brandur merged commit f9619a0 into master Aug 9, 2024
10 checks passed
@brandur brandur deleted the brandur-min-go-1.21 branch August 9, 2024 02:59
brandur added a commit that referenced this pull request Aug 13, 2024
…in + workspace

This one's a bit of a grab bucket, but do a few refactors and additions:

* Bring the project over to use a Go workspace, thereby letting us run
  tests and programs across module directories and cut out our `replace`
  statements in `go.mod` all over the place.

* Move the River version update program for `go.mod` files over to
  `rivershared` so that it can be used by other River projects. It's
  modified so that instead of hardcoding a submodule list, it can now
  read them straight out of the project's workspace in `go.work`.

* Add a new program that's similar to this one whose job it is to check
  that the `go`/`toolchain` directives across all `go.mod`s in the
  workspace match, which will help prevent accidental regressions of
  #522. It can also be used locally to update `go`/`toolchain`
  directives to new values in case we want to do that in the future.

* Do a lot of repaving in `Makefile` so that we can use list of
  submodules from the workspace to run a series of commands instead of
  needing to maintain independent lists for each target, a practice that
  was often unreliable.
brandur added a commit that referenced this pull request Aug 13, 2024
…in + workspace

This one's a bit of a grab bucket, but do a few refactors and additions:

* Bring the project over to use a Go workspace, thereby letting us run
  tests and programs across module directories and cut out our `replace`
  statements in `go.mod` all over the place.

* Move the River version update program for `go.mod` files over to
  `rivershared` so that it can be used by other River projects. It's
  modified so that instead of hardcoding a submodule list, it can now
  read them straight out of the project's workspace in `go.work`.

* Add a new program that's similar to this one whose job it is to check
  that the `go`/`toolchain` directives across all `go.mod`s in the
  workspace match, which will help prevent accidental regressions of
  #522. It can also be used locally to update `go`/`toolchain`
  directives to new values in case we want to do that in the future.

* Do a lot of repaving in `Makefile` so that we can use list of
  submodules from the workspace to run a series of commands instead of
  needing to maintain independent lists for each target, a practice that
  was often unreliable.
brandur added a commit that referenced this pull request Aug 13, 2024
…in + workspace

This one's a bit of a grab bucket, but do a few refactors and additions:

* Bring the project over to use a Go workspace, thereby letting us run
  tests and programs across module directories and cut out our `replace`
  statements in `go.mod` all over the place.

* Move the River version update program for `go.mod` files over to
  `rivershared` so that it can be used by other River projects. It's
  modified so that instead of hardcoding a submodule list, it can now
  read them straight out of the project's workspace in `go.work`.

* Add a new program that's similar to this one whose job it is to check
  that the `go`/`toolchain` directives across all `go.mod`s in the
  workspace match, which will help prevent accidental regressions of
  #522. It can also be used locally to update `go`/`toolchain`
  directives to new values in case we want to do that in the future.

* Do a lot of repaving in `Makefile` so that we can use list of
  submodules from the workspace to run a series of commands instead of
  needing to maintain independent lists for each target, a practice that
  was often unreliable.
brandur added a commit that referenced this pull request Aug 18, 2024
…in + workspace

This one's a bit of a grab bucket, but do a few refactors and additions:

* Bring the project over to use a Go workspace, thereby letting us run
  tests and programs across module directories and cut out our `replace`
  statements in `go.mod` all over the place.

* Move the River version update program for `go.mod` files over to
  `rivershared` so that it can be used by other River projects. It's
  modified so that instead of hardcoding a submodule list, it can now
  read them straight out of the project's workspace in `go.work`.

* Add a new program that's similar to this one whose job it is to check
  that the `go`/`toolchain` directives across all `go.mod`s in the
  workspace match, which will help prevent accidental regressions of
  #522. It can also be used locally to update `go`/`toolchain`
  directives to new values in case we want to do that in the future.

* Do a lot of repaving in `Makefile` so that we can use list of
  submodules from the workspace to run a series of commands instead of
  needing to maintain independent lists for each target, a practice that
  was often unreliable.
brandur added a commit that referenced this pull request Aug 18, 2024
…in + workspace

This one's a bit of a grab bucket, but do a few refactors and additions:

* Bring the project over to use a Go workspace, thereby letting us run
  tests and programs across module directories and cut out our `replace`
  statements in `go.mod` all over the place.

* Move the River version update program for `go.mod` files over to
  `rivershared` so that it can be used by other River projects. It's
  modified so that instead of hardcoding a submodule list, it can now
  read them straight out of the project's workspace in `go.work`.

* Add a new program that's similar to this one whose job it is to check
  that the `go`/`toolchain` directives across all `go.mod`s in the
  workspace match, which will help prevent accidental regressions of
  #522. It can also be used locally to update `go`/`toolchain`
  directives to new values in case we want to do that in the future.

* Do a lot of repaving in `Makefile` so that we can use list of
  submodules from the workspace to run a series of commands instead of
  needing to maintain independent lists for each target, a practice that
  was often unreliable.
brandur added a commit that referenced this pull request Aug 18, 2024
…in + workspace (#528)

This one's a bit of a grab bucket, but do a few refactors and additions:

* Bring the project over to use a Go workspace, thereby letting us run
  tests and programs across module directories and cut out our `replace`
  statements in `go.mod` all over the place.

* Move the River version update program for `go.mod` files over to
  `rivershared` so that it can be used by other River projects. It's
  modified so that instead of hardcoding a submodule list, it can now
  read them straight out of the project's workspace in `go.work`.

* Add a new program that's similar to this one whose job it is to check
  that the `go`/`toolchain` directives across all `go.mod`s in the
  workspace match, which will help prevent accidental regressions of
  #522. It can also be used locally to update `go`/`toolchain`
  directives to new values in case we want to do that in the future.

* Do a lot of repaving in `Makefile` so that we can use list of
  submodules from the workspace to run a series of commands instead of
  needing to maintain independent lists for each target, a practice that
  was often unreliable.
tigrato pushed a commit to gravitational/river that referenced this pull request Dec 18, 2024
…rqueue#522)

This one related to riverqueue#520, in which it turns out that our Go 1.21 build
step has actually been automatically upgrading itself to Go 1.22, so
under the radar that we didn't notice.

Here, leverage a Go "toolchain" to establish a preferred version of Go
for the project, but keeping a `go` directive in `go.mod`s that's at the
1.21 lower bound that we're trying to support. The presence of a
`toolchain` directive prevents `go mod tidy` from upgrading the `go`
directive to the latest version of Go installed.

I found the easiest way to add the toolchain directive to be with `go
get` [2] like:

    go get go@1.21 toolchain@1.22.5

Maintaining this correctly is going to be a little tricky. This is one
of those classic Go features that kind of works, but various Go commands
will perform all kinds of confusing behavior like stripping directives
out of your file if there's anything even a tiny bit wrong, and with no
explanation whatsoever.

I found that to get this working I had to started with the "inner"
dependencies like `rivershared` and `rivertype`, `go get` Go/toolchain
there first, then work my way up the stack all the way up to the main
project.

I'm going to try and follow this up with some tooling to help make this
easier for ourselves, and a build check that verifies in CI that nothing
gets accidentally regressed as we make changes because this is _very_
easy to do accidentally.

Fixes riverqueue#520.

[1] https://go.dev/doc/toolchain
[2] https://go.dev/doc/toolchain#get
tigrato pushed a commit to gravitational/river that referenced this pull request Dec 18, 2024
…in + workspace (riverqueue#528)

This one's a bit of a grab bucket, but do a few refactors and additions:

* Bring the project over to use a Go workspace, thereby letting us run
  tests and programs across module directories and cut out our `replace`
  statements in `go.mod` all over the place.

* Move the River version update program for `go.mod` files over to
  `rivershared` so that it can be used by other River projects. It's
  modified so that instead of hardcoding a submodule list, it can now
  read them straight out of the project's workspace in `go.work`.

* Add a new program that's similar to this one whose job it is to check
  that the `go`/`toolchain` directives across all `go.mod`s in the
  workspace match, which will help prevent accidental regressions of
  riverqueue#522. It can also be used locally to update `go`/`toolchain`
  directives to new values in case we want to do that in the future.

* Do a lot of repaving in `Makefile` so that we can use list of
  submodules from the workspace to run a series of commands instead of
  needing to maintain independent lists for each target, a practice that
  was often unreliable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Version in go.mod could be lowered
2 participants