go: lower Go requirement to Go 1.18 #37
Conversation
With commit 6864912 ("Isolate the testing import in test code"), the testing hooks were only ever called in binaries compiled in test mode, meaning that checking if we are in a test doesn't really make sense. This is also necessary to remove because testing.Testing() was added in Go 1.21, which is too new for some of our users. Signed-off-by: Aleksa Sarai <[email protected]>
|
@cyphar this is amazing! Thank you for investing time in this. Much appreciated. Especially on behalf of people who have to maintain older releases. For me, 1.19 would also be OK, should there be something you would want from 1.19 here. |
|
Unfortunately all the stuff we need to backport is from 1.20 and 1.21 (most of the useful generics stdlib stuff was added in 1.21). Go 1.18 vs 1.19 doesn't really change anything AFAIK. We could use |
|
I also can't promise this is going to work -- the current failures are because of the lack of multiple |
@cyphar that sounds like a pain to solve, indeed... I think, we have no choice but to draw a line here. Generics is one, but the lack of multiple %w support is probably too much. I am very thankful you've tried. Much appreciated! ❤️ |
|
Luckily one of the multiple errors I'm wrapping is purely internal and is used for tests, so it might be okay. Let me give it a shot and I'll let you know. |
cyphar
force-pushed
the
go-downgrade
branch
6 times, most recently
from
99b9e7f to
5185e66
Compare
We make heavy use of generics, so we can't really go earlier than Go 1.18, but we can backport some of the stdlib helpers from later Go versions to keep the minimum Go version lower. While such old Go versions are no longer maintained, some downstreams want to use the new API on very old release branches where updating the Go version is impractical. So let's make their lives easier. Signed-off-by: Aleksa Sarai <[email protected]>
The newest feature from golang.org/x/sys we aboslutely need is the fsconfig(2) wrappers which were added in v0.18.0. We also would like the definition of STATX_MNT_ID_UNIQUE but since it is a fixed constant, we don't need to bump the dependency all the way up to v0.20.0. Note that this is being done to avoid forcing downstreams to upgrade their golang.org/x/sys dependency unnecessarily. This does not mean that all users of filepath-securejoin will use an outdated version -- Go only uses a single version of any module for compiling entire programs, and its use of Minimal Version Selection[1] means that if a user of filepath-securejoin requires a newer version they will get the newer version (and filepath-securejoin would use the same version too). Signed-off-by: Aleksa Sarai <[email protected]>
In particular, we want to test our minimum (Go 1.18), as well as the versions where have explicit compatibility handling (Go 1.20 and 1.21) in order to make sure that builds with any mix of compatibility shims work correctly. One slight wrinkle is that -test.gocoverdir was only added in Go 1.20, so for older Go versions we need to just skip generating coverage (in theory you can collate coverage runs from older Go versions but the format is different to the post-1.20 format and so would be too complicated to deal with). Signed-off-by: Aleksa Sarai <[email protected]>
This is not a guarantee that things will work, but it will give us a hint about any obvious issues. Signed-off-by: Aleksa Sarai <[email protected]>
cyphar
force-pushed
the
go-downgrade
branch
3 times, most recently
from
0cdb62c to
3419ad0
Compare
actions/setup-go supports specifying the latest and latest-1 versions this way, which is preferable to updating the workflow each time there's an update. We keep the versions we care about for compatibility purposes, of course. Signed-off-by: Aleksa Sarai <[email protected]>
|
@cyphar this is amazing! Thank you for persevering with this! ❤️ |
| - "oldstable" | ||
| - "stable" |
There was a problem hiding this comment.
This is somewhat problematic, here's why.
A cache that actions/setup-go creates adds a Go version to the source data for the cache key (or maybe to the cache key itself -- I don't remember exactly). Now, if you change the go version manually (say from 1.22 to 1.23), the existing cache is instantly invalidated. But a string like "stable" or "oldstable" never changes, so when a new minor Go version is released, your CI jobs can still be stuck using an old one from the cache.
The same problem exists when a new Go patch version is released, and it is mitigated by setting the check-latest: true input for action/setup-go. I believe it also helps when you use stable/oldstable, and in this case it becomes a must-have, if you really want to test Go 1.24.0 once it comes out, not when a cache expires.
I might not be entirely correct here but at the very least it makes sense to keep an eye on your CI runs.
There was a problem hiding this comment.
Ah, damn. I assumed this was more like a thin alias that wouldn't cause those kinds of issues. I'll add check-latest: true, hopefully that mitigates the risk.
There was a problem hiding this comment.
It seems that the aliases are supposed to imply check-latest: true according to the docs:
Note: using these aliases will result in same version as using corresponding minor release with check-latest input set to true
But I'll add it anyway.
We make heavy use of generics, so we can't really go earlier than Go
1.18, but we can backport some of the stdlib helpers from later Go
versions to keep the minimum Go version lower.
While such old Go versions are no longer maintained, some downstreams
want to use the new API on very old release branches where updating the
Go version is impractical. So let's make their lives easier.
Signed-off-by: Aleksa Sarai [email protected]