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

Decrease the specificity of the Go version in go.mod #370

Closed
ForestEckhardt opened this issue Jul 1, 2024 · 8 comments
Closed

Decrease the specificity of the Go version in go.mod #370

ForestEckhardt opened this issue Jul 1, 2024 · 8 comments

Comments

@ForestEckhardt
Copy link

In 9ea2328 you added a patch version the to Go version inside of go.mod. This in turn means that all consumers of this module now need to specify the exact patch version as well instead of being able to set something like go 1.21.

Would it be possible to decrease the specificity of the version given to allow for an easier upgrade path using tools like dependabot?

@dtrudg
Copy link
Member

dtrudg commented Jul 2, 2024

What can be meaningfully specified on a go.mod go directive has changed over time. Go 1.21 would be considered a development version, with no release... so with the introduction of the toolchain concept and automatic tool chain downloads, a go 1.21 line can no longer be specified alone. (until 1.22.4 where a workaround automatically appending the .0 for a toolchain download was added).

See discussion e.g. on the Go repository here:

See - golang/go#62278 (comment)

Note that there are many linked issues to other projects, where they have had to do the same... add a patch release to the go line. This is expected post Go 1.21.

@robdimsdale
Copy link

@dtrudg from that trhead you linked:

In general if you want to specify a development version in the go line, you must also give a concrete toolchain version. So either of these should work:

go 1.21.0

or

go 1.21
toolchain go1.21.0

Can you talk us through your reasoning about picking the go version line vs adding a toolchain? In our experience we've generally seen more projects adopt the latter. In our experience it has the benefit of:

  1. Ensuring that the go version is set at the point of where the tool is built (i.e as a consumer of your library, I choose the go version via toolchain)
  2. reducing noise that results from any (transitive) dependency picking a concrete value for go version.

@dtrudg
Copy link
Member

dtrudg commented Jul 2, 2024

@robdimsdale - I'll ask @tri-adam to chime in, but my understanding from the Go documentation and the thread linked is that...

  • The go value in go.mod is there for the module to specify the minimum concrete go version that can be used.
  • From go 1.21 things changed a bit, and there is no concrete version x.y, only x.y.0.
  • Specifying go 1.21 is saying that 1.21 development is okay, and the additional toolchain go1.21.0 is a workaround to say 'actually we need the 1.21.0 release'

This module is primarily used by sylabs/singularity - and there we had more issues with go.mod noise and dependabot updates before we switched from go 1.21 -> go 1.21.0, so I'm not clear that it's universally the case that the other approach works better.

We also have upstream dependencies that are now specifying an x.y.0 - e.g.

https://github.com/sigstore/sigstore/blob/main/go.mod#L3

... so it's not clear to me that we can avoid it anyway.

@robdimsdale
Copy link

@dtrudg Yeah, the transitive dependencies issue is exactly why we (https://github.com/paketo-buildpacks) are here. We don't use sif directly - in fact we hadn't heard of it until this week!

My understanding of the go vs toolchain commands is essentially that the go directive is the interface to specify the language constraint (since go doesn't introduce features in a patch) and the toolchain is the concrete implementation that you actually build the binary with. This is captured in this discussion.

However, I accept that my understanding might be wrong. And even if I'm not wrong, I accept that we can't avoid updating transitive dependencies just because they bump the go version. We just wanted to check in with you all to make sure that this bump was intentional. Which is sounds like it was.

Honestly, we're just trying to keep our dependencies up to date via dependabot in order to be good OSS citizens, and this automatic update process fails if a transitive dependency changes the version of go in go.mod. Your project just happened to be the first transitive dependency of ours which had this issue 😄

I'm happy to leave this issue open a little longer to ensure relevant parties have had a chance to weigh in. But personally, I think I'm convinced that we (paketo buildpacks) need to accept that transitive dependencies will require go bumps from time to time.

@dtrudg
Copy link
Member

dtrudg commented Jul 2, 2024

Okay - I'll see what @tri-adam says.... I must admit we (or at least I) went through a lot of somewhat confused discussion based on finding different ideas in issues when we first used a 1.21.0 on another project :-)

I would point out, though, that according to the syntax documented at the link below, the go value should be a 'valid release version', which 1.21 is not (it is only a 'language version').

It's all really confusing given that 1.20, 1.19 etc. were language and release versions... but from 1.21 the first patch release version was 1.21.0 and not just 1.21 (https://go.dev/doc/toolchain#version).

https://go.dev/ref/mod#go-mod-file-go

image

@tri-adam
Copy link
Member

tri-adam commented Jul 2, 2024

I can shed some light on why we went down this road.

When we dropped Go 1.20 support (#351), I actually did go with go 1.21 in go.mod initially. This worked, but it did result in toolchain not available error occurring in certain scenarios as described in golang/go#62278.

Ultimately, on re-reading the documentation I came to the conclusion that go 1.21 and go 1.21.0 both mandate version >= 1.21 for the Go language version. go 1.21 tolerates 1.21-rcX, whereas go 1.21.0 does not, but this did not seem like a useful difference. There were a couple of other factors that have since led me to add a patch back in to the go directive:

  • Go 1.19 releases prior to 1.19.13 and Go 1.20 releases prior to Go 1.20.8 still treated the go directive as advisory rather than mandatory. SIF >= v2.16.0 really does require features from Go 1.21, so go 1.21.0 makes things fail more obviously with old versions of Go.
  • The Go tooling itself seems to lean this way, for example:
go: creating new go.mod: module m
$ cat go.mod
module m

go 1.22.4

On this:

I think I'm convinced that we (paketo buildpacks) need to accept that transitive dependencies will require go bumps from time to time.

Yes... unfortunately I don't see a way out of that. We are facing this ourselves at the moment with Dependabot having pulled in a dependency which has bumped to Go 1.22.0 (#369). Having committed to supporting the current to stable releases of Go in this module, we have to either hold off on updating that module or change our policy around Go version compatibility...

With all that said... @robdimsdale I'm curious what effects you are seeing. Could you drop a link to a Dependabot PR where we're generating extra noise? That certainly wasn't our intention, and I'd be curious to learn about any downstream effects.

@robdimsdale
Copy link

robdimsdale commented Jul 2, 2024

Sure thing - an example failure is this PR, which fails when trying to run tests with the following:

go: updates to go.mod needed; to update it:
	go mod tidy

Running a go mod tidy bumps the go line from go 1.21 to 1.21.0 and this fixes the issue.

The more I think about this, the more I think it's probably just a deficiency in github's dependabot. I don't really mind what patch version of go we have in our projects (modulo my misinterpretation that I articulated above). We mostly create binaries (vs libraries) so I don't even mind what minor version of go the go.mod has. Especially because we've adopted the toolchain directive which ensures we're actually building our binaries with the latest version of the go toolchain. I just don't want to have to deal with dozenss of dependabot failures.

(for extra context, part of why dependabot failures like this are annoying to us is because we manage dozens of repositories in the OSS and hundreds more like this in closed-source commercial repositories. I've probably closed 500 broken dependabot pull requests this week alone)

@tri-adam
Copy link
Member

tri-adam commented Jul 2, 2024

Thanks for that, it helps fill in the gaps.

I gather this is a direct result of the new requirement on that go directive in Go 1.21 (from the Go Toolchains documentation):

A module’s go line must declare a version greater than or equal to the go version declared by each of the modules listed in require statements.

Since SIF bumped to 1.21.0, and 1.21<1.21.0, go mod tidy needs to update 1.21->1.21.0.

I agree that this is probably something Dependabot could/should automate. In reality this will come up fairly often as dependent modules adopt new minimum Go versions.

As an aside, I think we were about to be forced to add the patch version to the go directive since we import github.com/sigstore/sigstore. That Sigstore module was in turn forced down that path by github.com/letsencrypt/boulder. So there's a bit of a network effect here, and as a result in time I would expect an increasing number of projects will include a patch version.

To summarize, I think we'll stick with our current approach of specifying the patch level explicitly.

Hope you can find a scalable solution to ease the Dependabot pain in all of those repos you are managing. Thanks for the discussion... I learned a lot from it. All the best!

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

No branches or pull requests

4 participants