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

Disable go version updates by default #16715

Closed
harryzcy opened this issue Jul 22, 2022 · 31 comments · Fixed by #19453
Closed

Disable go version updates by default #16715

harryzcy opened this issue Jul 22, 2022 · 31 comments · Fixed by #19453
Assignees
Labels
manager:gomod Go Modules priority-2-high Bugs impacting wide number of users or very important features status:in-progress Someone is working on implementation type:bug Bug fix of existing functionality

Comments

@harryzcy
Copy link

How are you running Renovate?

Mend Renovate hosted app on github.com

If you're self-hosting Renovate, tell us what version of Renovate you run.

No response

Please select which platform you are using if self-hosting.

No response

If you're self-hosting Renovate, tell us what version of the platform you run.

No response

Was this something which used to work for you, and then stopped?

It used to work, and then stopped

Describe the bug

A recent change #16541 cause renovate to update go version in go.mod by default, which shouldn't be the case.

Syntax
    go minimum-go-version

minimum-go-version
    The minimum version of Go required to compile packages in this module.

Per go.mod file reference, version should be the minimum version supported, not an update-to-date one. So the feature to update go version should be disabled by default, and only enabled via configurations.

Relevant debug logs

No response

Have you created a minimal reproduction repository?

No reproduction repository

@harryzcy harryzcy added priority-5-triage status:requirements Full requirements are not yet known, so implementation should not be started type:bug Bug fix of existing functionality labels Jul 22, 2022
@rarkins
Copy link
Collaborator

rarkins commented Jul 23, 2022

I understand your position but this can also lead to regular upgrades failing whenever any dependency needs a newer version of go too. In other words if you stay on an old minimum version of go forever you'll eventually get stuck behind on most dependencies.

@nemunaire
Copy link

nemunaire commented Jul 23, 2022

I support the request of @harryzcy. The current implementation is really annoying and creates lots of issues inherent to the way go.mod evolve.

There are many side-effects introduced regularly by newer go releases on how the go.mod is treated. For example, I have many repository with go 1.10, go 1.13, go 1.16, ... With the introduction of this feature, all my repositories fails to compile with errors likes these:

go: inconsistent vendoring in /drone/src:
	github.com/nfnt/resize@v0.0.0-20180221191011-83c6a9932646: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt

	To ignore the vendor directory, use -mod=readonly or -mod=mod.
	To sync the vendor directory, run:
		go mod vendor

as the format of go.mod changes regularly and can't be considered stable for now.

So I need to do manually run go mod commands in this case to update the format, but it's difficult to know what to expect in future releases. If the format changes again in 1.19, it will not have helped anyone and will have caused a lot of pull request noise for nothing.

As the primary goal of the go x.xx instruction is to hard block on a given release, it seems acceptable to manually upgrade it only when a dependency requires it and not automatically by default. Having the choice to enable this beta implementation can be interesting though.

On some other repositories, that make use of gin-gonic/gin, I decided on some project to make the go 1.18 upgrade (as gin new releases uses any feature), but on other project, that I want to be compatible with older go releases, I make the choice to continue to use the gin branch that does not use go 1.18 features.

Moreover, it's true the introduction of the generics in the 1.18 release decided a lot of project to switch to this release, but from my point of view it's pretty rare to have so interesting new features and projects are rather used to stay on a given go release forever.

So the go version update introduced does more harm than good to the community, because even projects that don't require a new release of go will fail to compile with previous go releases just because renovatebot choose to make the update, by default, even if it's not required.

Either the functionality should be better implemented (by updating the go x.xx only when a dependency needs it), or it should be considered as beta and disabled by default.

@viceice
Copy link
Member

viceice commented Jul 23, 2022

i don't agree, we need to check which command needs to be run to update the sum file, so the update works like any other go mod update.

you can always disable those updates yourself or require dashboard approval to have visibility.

@nemunaire
Copy link

nemunaire commented Jul 23, 2022

If this can be acceptable for an end user project, as the only implication is that this project requires a recent compiler, what about libraries?

With this option enabled by default, unconscious developers using renovatebot will force others end user projects (using renovatebot or not) to update their go.mod to a more recent version, even if no one use or require a recent go feature.

Of course this feature need to be fixed in renovatebot, but this is not the point here: go x.xx should be increased only if it is required.

There is no security implication to not update this value, it only reduces compatibility artificially.

@viceice
Copy link
Member

viceice commented Jul 23, 2022

it was a community feature request/ implementation to support updating go.
so we currently don't plan to change that.

you can always close / ignore those PR's. or disable those updates via config.

we can maybe provide a helper preset to easily disable those updates.

@nemunaire
Copy link

nemunaire commented Jul 23, 2022

It's not the feature that is criticized, but its activation by default.

As @rarkins mention, people need that to perform usual modules update smoothly. And I'll be glad not to have to manually change my go x.xx anymore in such case. But in its current state, this feature is not finished and ruin the compatibility.

We ask that it be disabled by default, while a better solution is implemented. So people that need this can activate it, but knowing the implications.

I can certainly disabled it on my repos, but I don't want to create thousand issues requesting to revert such changes, on dependencies I don't own, when this is unneeded. Just because the dev merge such PR without calculating the implications regarding the compatibility.

The fact is the vast majority of module update can be performed without updating the go x.xx instruction. Sometime a dependency uses a new feature, so in that case renovatebot should update go x.xx at the same time the dependency update is pulled. But this can't be the default case otherwise. The Go team doesn't advertise to update the go x.xx instruction without the need to do so ("The minimum version of Go required to compile packages in this module"), on purpose.

@rarkins
Copy link
Collaborator

rarkins commented Jul 23, 2022

Please provide links/references for your last sentence so we can assess.

@nemunaire
Copy link

This is what @harryzcy writes in first post: https://go.dev/doc/modules/gomod-ref#go-syntax

Syntax

go minimum-go-version

minimum-go-version
: The minimum version of Go required to compile packages in this module.

minimum version is what we ask for. latest version is what renovatebot performs currently.

In a project using go 1.15, if one deps required go 1.18, then we should update our go x.xx to go 1.18, but if a new go version is released, updating the version is not necessary as long as:

  • our code doesn't use any new feature or
  • any deps doesn't use any new feature.

In a project using go 1.15, with all deps requiring go 1.15 or lower, then nothing should happen. As this really is the minimum version of Go required to compile.

@rarkins
Copy link
Collaborator

rarkins commented Jul 23, 2022

That's purely syntax, not any justification or argument. It does not alone imply that you should avoid updating it

@rarkins
Copy link
Collaborator

rarkins commented Jul 23, 2022

Relevant: https://endoflife.date/go

https://go.dev/doc/devel/release#policy

@nemunaire
Copy link

nemunaire commented Jul 23, 2022

Will you accept a pull request implementing a smarter go x.xx, to be used by default instead of the current feature, based on the max(max deps required version,current module version)?

@nemunaire
Copy link

nemunaire commented Jul 23, 2022

Perhaps you should consider this example: go 1.19 will be out next month. All projects (end user project and modules on which end user projects depends) will move to a go 1.19 instruction in their go.mod due to this feature.

But in august, who will have a go 1.19 compiler? Ubuntu users will be on 1.18 until Ubuntu 22.10 in October (and some people prefer to stay on LTS, so on go 1.18 for the next 2 years), so even if they don't need the new features, you'll impose them to download an install a new compiler without any reason... Same for Debian, Alpine, ...

And your link shows that go 1.17 is still officially supported, but this renovatebot feature forces all modules/libraries to use 1.18. How can someone which is still on 1.17.12 use a github module managed by renovatebot?

Also you should consider RedHat Enterprise users, they have an old go compilers in their repository (no more supported officially), some people develops for those users and fix things. They also need their dependency to be compatible with the minimum version.

@rarkins
Copy link
Collaborator

rarkins commented Jul 24, 2022

Is your concern about your own projects, or because Go module authors you rely on may use Renovate and update their modules to a version you can't support?

@nemunaire
Copy link

nemunaire commented Jul 24, 2022

Yes, this is all about Go module authors I and Go people rely on.

For our own projects, the impact of this feature is negligible as we can choose to not merge it until we are ready, as always with others renovatebot PR. But the compatibility we are talking about is Github authors who merge this without knowing their work is used by people that can't update their compiler for any reason: as this creates artificial incompatibility with older go release.

If this feature is disabled by default, people can be warned that if they write go module used by others, they shouldn't use this feature.

The best world would be go x.xx automatically updated when required by a dependency: we could merge PR smoothly while keeping maximum compatibility in all modules and end user projects over the world.

@rarkins
Copy link
Collaborator

rarkins commented Jul 24, 2022

@bcmills you have provided valuable recommendations here in the past, hope you don't mind me @'ing you here. You can perhaps skip the above content if you wish, as I will summarize here:

  • go.mod files include a go version, which according to definition represents the minimum go version the module/project supports
  • Some users would like for that version to be updated automatically, which Renovate can now do
  • Some users think such a version should not be updated ever, unless necessary (e.g. a sub-dependency requires a newer go version to compile)
  • Another possibility would be to regularly bump this version to the minimum "supported" go version (e.g. 1.17 now)

BTW I think this also raises an interesting difference between how the npm ecosystem and Go ecosystem treat compatibility and breaking changes. Today, many/most npm package authors increment a major version whenever they drop support for an old version of Node.js. However, there is no such guideline - or practice to my awareness - of Go modules bumping a major version whenever they drop support for older versions of Go.

@bcmills
Copy link

bcmills commented Jul 25, 2022

The go version unfortunately isn't quite a minimum (see golang/go#30791): users on older versions may be able to build the package successfully, and one can guarantee that by actually running the tests with those older versions.

Instead, I would think of the go version as “the Go version required for a full-fidelity build”. If you are on an older Go version, the package might build with a degraded API (for example, some features may be notched out by //go:build constraints), or it might be completely broken, or it might show spurious additional dependencies (as occurred at the 1.16→1.17 transition).

It also isn't strictly necessary to update your go version in response to a dependency: a go 1.15 module can easily depend on a go 1.18 module without error.

Personally, I would recommend setting the go version to the oldest toolchain you expect contributors to the project to use to author their changes, and then verify support for newer and/or older versions by testing those versions explicitly in CI.

@rarkins
Copy link
Collaborator

rarkins commented Jul 26, 2022

Thanks @bcmills.

Considering that a go 1.15 can depend on a go 1.18 module then it sounds like one of the concerns mentioned above is somewhat ruled out, i.e. that module authors updating their go directive to latest using Renovate Bot are going to cause undesirable ripple effects downstream.

Setting aside automation, let's say that a module author wants to do the "right thing" in the ecosystem. In that case would it be:

  • If it ain't broke then don't fix it: leaving their module on the same go version indefinitely, even if it's years out of date/support, or
  • Bumping their go directive to the minimum "supported" go version each time (e.g. 1.17 now), or
  • Bumping their go directive to latest (e.g. 1.18 now)?

@maxbrunet
Copy link
Contributor

If that helps anyone: for now, if you would like to opt-out, you can use a package rule:

{
  "packageRules": [
    {
      "description": "Opt-out minimum Go version updates: https://github.com/renovatebot/renovate/issues/16715",
      "matchManagers": ["gomod"],
      "matchDepTypes": ["golang"],
      "enabled": false
    }
  ]
}

@burdiyan
Copy link

burdiyan commented Sep 6, 2022

Just got hit by this.

Renovate just updated Go version in the go.mod files in our manually vendored third-party dependencies and merged everything. We also are using Nix + nix-shell for local development, where we pin the version of all the tools, including Go. And for now it's Go 1.18. Upgrading this requirement automatically now prevents us from interacting with modules using go mod command. We can't upgrade to Go 1.19 yet.

Also, AFAIK, Go 1.19 doesn't bring new features that would require setting 1.19 in the go.mod file, so in reality there's no benefit in doing so. This was relevant for Go 1.18 to activate generics for example. But I'm not aware of anything new in Go 1.19 that would require updating go.mod to use.

I believe, that having this enabled automatically and by default might have some undesired implications for the overall community. E.g. if you maintain a library that other people use, and you do this, then you just blocked all the people from being able to use the library without upgrading their version of Go. And I agree with the statement that there's no need to require latest versions without really needing (like if you want to do a package using generics, you must require at least Go 1.18).

@rarkins
Copy link
Collaborator

rarkins commented Sep 6, 2022

Maybe we need to do this:

  • Create a new "go" versioning, just for the go version in go.mod files
  • The only valid values are x.y
  • A value of x.y implicitly means ^x.y in npm semver speak

In turn this would mean:

  • All future x.?.? releases satisfy the existing range
  • We default to replace versioning, which means there's nothing to upgrade (i.e. "go versions updating will be disabled by default"
  • Those that do want to keep bumping Go version can configure rangeStrategy=bump

at-wat added a commit to pion/renovate-config that referenced this issue Oct 9, 2022
Need to support non-latest Go version as a library.
renovatebot/renovate#16715
at-wat added a commit to pion/renovate-config that referenced this issue Oct 9, 2022
Need to support non-latest Go version as library.
renovatebot/renovate#16715
@cevich
Copy link

cevich commented Oct 18, 2022

I think the proposed workaround (above) is wrong, won't that turn off all go updates?

The way I worked around this was:

  "golang": {
    "packageRules": [
      {
        "matchDatasources": ["golang-version"],
        "enabled": false
      },
      "...other rules..."
    ]
  }

@maxbrunet
Copy link
Contributor

I think the proposed workaround (above) is wrong, won't that turn off all go updates?

No, the type in matchDepTypes is only used for the go property of the go.mod file:

"matchDepTypes": ["golang"],

The type for Go modules in a go.mod file is require.

The rule you shared is wider and would block any updates from the mentioned datasource, even outside the gomod manager (e.g. a regex manager using it to update the CI pipeline would be disabled)

@cevich
Copy link

cevich commented Oct 18, 2022

even outside the gomod manager

Doesn't being under a top-level "golang" object guarantee it only applies to golang stuffs?

@cevich
Copy link

cevich commented Oct 18, 2022

(not trying to argue, trying to understand Renovate configuration better)

@cevich
Copy link

cevich commented Oct 18, 2022

Oh okay, I think I follow now. So this will work for us then (since we have a bunch of other golang rules):

  "golang": {
    "packageRules": [
      {
        "matchDepTypes": ["golang"],
        "enabled": false
      },
      "...other rules..."
    ]
  }

@maxbrunet
Copy link
Contributor

depType is a free-form "tag" that managers can use (which could be non-unique and with different meanings between managers), and a logical AND is applied to match*es. So both "matchManagers": ["gomod"], and "matchDepTypes": ["golang"], are required to match the dependencies of type golang from the gomod manager

@viceice
Copy link
Member

viceice commented Oct 18, 2022

Oh okay, I think I follow now. So this will work for us then (since we have a bunch of other golang rules):

  "golang": {
    "packageRules": [
      {
        "matchDepTypes": ["golang"],
        "enabled": false
      },
      "...other rules..."
    ]
  }

don't nest package rules, it will/ can have unwanted side effects

at-wat added a commit to pion/renovate-config that referenced this issue Oct 19, 2022
Need to support non-latest Go version as library.
renovatebot/renovate#16715
@cevich
Copy link

cevich commented Oct 19, 2022

don't nest package rules, it will/ can have unwanted side effects

Thanks for the tip. And thanks @maxbrunet for the explanation.

@HonkingGoose
Copy link
Collaborator

Is this plan by the maintainer enough to get to status:ready? Or do we need to think more about the problem and possible solutions?

@rarkins rarkins added priority-2-high Bugs impacting wide number of users or very important features manager:gomod Go Modules status:ready and removed priority-5-triage status:requirements Full requirements are not yet known, so implementation should not be started labels Dec 12, 2022
@rarkins rarkins self-assigned this Dec 16, 2022
rarkins added a commit that referenced this issue Dec 16, 2022
@rarkins rarkins added status:in-progress Someone is working on implementation and removed status:ready labels Dec 16, 2022
@renovate-release
Copy link
Collaborator

🎉 This issue has been resolved in version 34.61.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@rarkins
Copy link
Collaborator

rarkins commented Dec 17, 2022

This is now rolled out in the app and is autoclosing Go upgrades as expected, e.g. https://togithub.com/pion/logging/pull/85

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
manager:gomod Go Modules priority-2-high Bugs impacting wide number of users or very important features status:in-progress Someone is working on implementation type:bug Bug fix of existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants