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

Group updates on Semantic Versioning level Beta #7795

Closed
jurre opened this issue Aug 11, 2023 · 24 comments
Closed

Group updates on Semantic Versioning level Beta #7795

jurre opened this issue Aug 11, 2023 · 24 comments
Labels
F: grouped-updates 🎳 Relates to bumping more than one dependency in a single PR

Comments

@jurre
Copy link
Member

jurre commented Aug 11, 2023

As part of our effort to release grouped updates for Dependabot, we're rolling out a new feature in closed beta that will allow you to group by semver level.

What this looks like:

version: 2
updates:
  - package-ecosystem: "npm"
    directory: "/"
    schedule:
      interval: "weekly"
    groups:
      non-major-versions: # the name of the group
        update-types:     # the key used to specify the semver level to include in the group
        - "minor"         # an array, possible values being minor, patch and major
        - "patch"

For the above configuration, Dependabot will open a grouped PR for any dependencies where the highest resolvable version is a patch or minor SemVer update, and any dependencies that will be updated to a new major version will be opened as separate, individual PRs.

The update-types key can be combined with other group rules, for these examples I will only specify the groups section, assume the rest of the config file is the same as the one listed above:

groups:
  angular:
    patterns: 
    - "@angular*"
    update-types:
    - "minor"
    - "patch"

For this configuration, any packages matching the pattern @angular* where the highest resolvable version is minor or patch will be grouped together, any package that does not match the pattern or that does not update to a minor or patch version will be opened as a separate PR. For cases where you do not want updates to major versions of @angular* packages, you can specify an ignore condition:

groups:
  angular:
    patterns: 
    - "@angular*"
    update-types:
    - "minor"
    - "patch"
ignore:
  - dependency-name: "@angular*"
    update-types: ["version-update:semver-major"]

If you want grouped PRs for development dependencies that have patch updates, you would specify:

groups:
  patch-dev-dependencies:
    dependency-type: "development"
    update-types:
    - "patch"

If you want to join the private beta, please fill out this form and we'll enable the feature.

Any other feedback or thoughts are much appreciated

@jakecoffman jakecoffman added the F: grouped-updates 🎳 Relates to bumping more than one dependency in a single PR label Aug 11, 2023
@jurre
Copy link
Member Author

jurre commented Aug 14, 2023

I've enabled all the repo's that have requested beta access so far, there were a few cases where I could not make out the repo because an account name was not present or the request was for "all repo's in org x", which is not something we are doing right now, and in that case I'd recommend waiting out until the public beta which should happen shortly if we don't find any showstoppers in this private beta round.

Will enable another round tomorrow

@xxchan
Copy link

xxchan commented Aug 14, 2023

risingwavelabs/risingwave#11680

0.y.z is updated in group. In rust community, 0.y.z is very common, and in this case, y is considered major version, so it should not be updated.

This guide uses the terms “major” and “minor” assuming this relates to a “1.0.0” release or later. Initial development releases starting with “0.y.z” can treat changes in “y” as a major release, and “z” as a minor release. “0.0.z” releases are always major changes. This is because Cargo uses the convention that only changes in the left-most non-zero component are considered incompatible.

https://doc.rust-lang.org/cargo/reference/semver.html

Although it seems this isn't defined in the official SemVer spec. It just mentions

Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable.

But I think respecting the "y" as major convention is beneficial.

https://semver.org/

@jurre
Copy link
Member Author

jurre commented Aug 15, 2023

Hmm yeah that's an interesting one @xxchan. It's a bit tricky because it's not in the spec, and doing that means we'll have to make an exception in our code and documentation to clarify this exception for the Rust ecosystem. I can imagine it could also be confusing to folks in the rust community no matter what we do here, on the one hand we're saying Dependabot is grouping these based on SemVer, but on the other hand Rust folks might be used to Rust not following SemVer.

Let me think about this a bit!

@edmorley
Copy link

It's a bit tricky because it's not in the spec, and doing that means we'll have to make an exception in our code and documentation to clarify this exception for the Rust ecosystem.

I agree that the spec on https://semver.org is less prescriptive (or perhaps even differs) from the Rust semver rules, however, I think the Dependabot implementation is still incorrect for non-Rust usages too?

That is, it seems there are three ways Dependabot could handle pre-zero versions:

  1. Treat them the same as post-zero versions (ie: 0.1.0 -> 0.2.0 is a minor change, 0.1.0 -> 0.1.1 is a patch change)
  2. Treat the middle component changing as a breaking change (ie: 0.1.0 -> 0.2.0 is a major change, 0.1.0 -> 0.1.1 is a patch/feature? change)
  3. Treat any component changing as a breaking change (ie: both 0.1.0 -> 0.2.0 and 0.1.0 -> 0.1.1 are major changes)

The Rust semver rules would be (2), and the https://semver.org rules would seem to suggest (3) - but Dependabot is currently implementing (1), which matches neither (and IMO is the worst of all options, since it risks pulling major changes into the grouped PRs -- false positives would be worse than false negatives etc).

@jurre
Copy link
Member Author

jurre commented Aug 15, 2023

@edmorley yes I think there's some merit to what you're saying, there's a case to be made for us to standardize on (3), since SemVer is essentially saying anything below 1 is a free for all. I can imagine this will end up being frustrating for some customers though, because it means there's no way for them to meaningfully group a dependency that is not > 1.0.

I appreciate your input, asking for input from folks on my team 👍

Edit: I think what I struggle with mostly right now is that SemVer is not explicitly saying (3), it's saying that before 1.0 it's undefined, not necessarily a major update, so it kind of feels like no matter what we do here, we potentially confuse people. Seeing 0.6.1 -> 0.6.2 show up in your major group is going to be unintuitive to at least a portion of our users. That's not to say that (1) is strictly a better option.

@jurre
Copy link
Member Author

jurre commented Aug 15, 2023

Interestingly, I think this (1) is also how our current ignore rules behave, I don't think these have ever tripped anyone up 🤔

@xxchan
Copy link

xxchan commented Aug 15, 2023

it's saying that before 1.0 it's undefined, not necessarily a major update

Indeed. BTW, Here are some discussions about the leading zero thing.
semver/semver#221 (comment)

Excluding 0.y.z from any semver group might be another choice.

it kind of feels like no matter what we do here, we potentially confuse people

I would recommend have some case studies about how people in different communities, besides Rust, treat the 0.y.z versions.

e.g., It seems npm also treats y as major, according to https://github.com/npm/node-semver and https://semver.npmjs.com/.

Although it by default starts at 1.0.0, and 0.y.z is not discussed in https://docs.npmjs.com/about-semantic-versioning.

I just personally feel that people either don't use 0.y.z, or want such behavior 😄.

@ying-jeanne
Copy link

ying-jeanne commented Aug 17, 2023

Does anyone has issue when using the groups ? After merge the config here, i am getting following error logs.

updater | 2023/08/17 12:02:28 ERROR <job_710433377> Error processing alpine (NoMethodError)
updater | 2023/08/17 12:02:28 ERROR <job_710433377> undefined method `segments' for "3.18.3":String
updater | 
updater |           major: version.segments[0] || 0,
updater |                         ^^^^^^^^^
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/group_update_creation.rb:225:in `semver_segments'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/group_update_creation.rb:213:in `semver_rules_allow_grouping?'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/group_update_creation.rb:127:in `compile_updates_for'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/group_update_creation.rb:40:in `block in compile_all_dependency_changes_for'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/group_update_creation.rb:28:in `each'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/group_update_creation.rb:28:in `compile_all_dependency_changes_for'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/operations/create_group_update_pull_request.rb:43:in `perform'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/operations/group_update_all_versions.rb:101:in `run_update_for'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/operations/group_update_all_versions.rb:85:in `block in run_grouped_dependency_updates'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/operations/group_update_all_versions.rb:74:in `each'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/operations/group_update_all_versions.rb:74:in `run_grouped_dependency_updates'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/operations/group_update_all_versions.rb:42:in `perform'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater.rb:63:in `run'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/update_files_command.rb:38:in `perform_job'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/base_command.rb:52:in `run'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> bin/update_files.rb:23:in `<main>'
updater | 2023/08/17 12:02:28 INFO <job_710433377> Nothing to update for Dependency Group: 'docker-dependencies-update'
updater | 2023/08/17 12:02:28 INFO <job_710433377> Starting update job for grafana/mimir
updater | 2023/08/17 12:02:28 INFO <job_710433377> Checking all dependencies for version updates...
updater | 2023/08/17 12:02:28 INFO <job_710433377> Checking if alpine 3.18.3 needs updating
  proxy | 2023/08/17 12:02:28 [176] GET https://registry.hub.docker.com:443/v2/library/alpine/tags/list
  proxy | 2023/08/17 12:02:28 [176] 401 https://registry.hub.docker.com:443/v2/library/alpine/tags/list
  proxy | 2023/08/17 12:02:28 [176] {"errors":[{"code":"UNAUTHORIZED","message":"authentication required","detail":[{"Type":"repository","Class":"","Name":"library/alpine","Action":"pull"}]}]}
  proxy | 2023/08/17 12:02:28 [178] GET https://auth.docker.io:443/token?service=registry.docker.io&scope=repository%3Alibrary%2Falpine%3Apull&account
  proxy | 2023/08/17 12:02:28 [178] 200 https://auth.docker.io:443/token?service=registry.docker.io&scope=repository%3Alibrary%2Falpine%3Apull&account

@psof
Copy link

psof commented Aug 17, 2023

We were reviewing the above and we believe that the root cause for this error is caused in this line:

  version = Dependabot::Utils.version_class_for_package_manager(job.package_manager).new(dependency.version.to_s)
  # Not every version class implements .major, .minor, .patch so we calculate it here from the segments
  latest = semver_segments(checker.latest_version)

Because the call to checker.latest_version will return a String from the Tag.name:

module Dependabot
  module Docker
    class UpdateChecker < Dependabot::UpdateCheckers::Base
      def latest_version
        latest_version_from(dependency.version)
      end

(...)
      def latest_version_from(version)
        latest_tag_from(version).name
      end

And the Tag's name attribute is indeed a String:

module Dependabot
  module Docker
    class Tag

      attr_reader :name

      def initialize(name)
        @name = name
      end

      def to_s
        name
      end

I believe this could be resolved by calling checker.version_of_latest_tag which actually returns a Version object instead of checker.latest_version

@brcarp
Copy link

brcarp commented Aug 25, 2023

Group by update-type appears to be part of the public release, if I'm reading correctly. Does that mean the private beta (and this issue) can be closed, or is there still another piece of this that remains in beta?

@edmorley
Copy link

Please could someone take a look at #7305? It makes onboarding existing repos into grouped updates much more painful, since:

  1. The old PRs need manually closing out
  2. Worse, the manually closing of old PRs then means Dependabot starts ignoring those dependencies, rather than moving them into the grouped PR

@ying-jeanne
Copy link

ying-jeanne commented Aug 31, 2023

Hi again, thank you for working on this but I am still facing problem regarding group update for dockerfile, it just can't run see logs. In the meanwhile i tested the group update on go dependencies, it seems working correctly. Please help it is currently blocking us.

@edmorley
Copy link

We're also running into this issue with the documented behaving not matching the implementation: #7939

@tomgrossman
Copy link

Does anyone has issue when using the groups ? After merge the config here, i am getting following error logs.

updater | 2023/08/17 12:02:28 ERROR <job_710433377> Error processing alpine (NoMethodError)
updater | 2023/08/17 12:02:28 ERROR <job_710433377> undefined method `segments' for "3.18.3":String
updater | 
updater |           major: version.segments[0] || 0,
updater |                         ^^^^^^^^^
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/group_update_creation.rb:225:in `semver_segments'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/group_update_creation.rb:213:in `semver_rules_allow_grouping?'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/group_update_creation.rb:127:in `compile_updates_for'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/group_update_creation.rb:40:in `block in compile_all_dependency_changes_for'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/group_update_creation.rb:28:in `each'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/group_update_creation.rb:28:in `compile_all_dependency_changes_for'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/operations/create_group_update_pull_request.rb:43:in `perform'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/operations/group_update_all_versions.rb:101:in `run_update_for'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/operations/group_update_all_versions.rb:85:in `block in run_grouped_dependency_updates'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/operations/group_update_all_versions.rb:74:in `each'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/operations/group_update_all_versions.rb:74:in `run_grouped_dependency_updates'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater/operations/group_update_all_versions.rb:42:in `perform'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/updater.rb:63:in `run'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/update_files_command.rb:38:in `perform_job'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> /home/dependabot/dependabot-updater/lib/dependabot/base_command.rb:52:in `run'
updater | 2023/08/17 12:02:28 ERROR <job_710433377> bin/update_files.rb:23:in `<main>'
updater | 2023/08/17 12:02:28 INFO <job_710433377> Nothing to update for Dependency Group: 'docker-dependencies-update'
updater | 2023/08/17 12:02:28 INFO <job_710433377> Starting update job for grafana/mimir
updater | 2023/08/17 12:02:28 INFO <job_710433377> Checking all dependencies for version updates...
updater | 2023/08/17 12:02:28 INFO <job_710433377> Checking if alpine 3.18.3 needs updating
  proxy | 2023/08/17 12:02:28 [176] GET https://registry.hub.docker.com:443/v2/library/alpine/tags/list
  proxy | 2023/08/17 12:02:28 [176] 401 https://registry.hub.docker.com:443/v2/library/alpine/tags/list
  proxy | 2023/08/17 12:02:28 [176] {"errors":[{"code":"UNAUTHORIZED","message":"authentication required","detail":[{"Type":"repository","Class":"","Name":"library/alpine","Action":"pull"}]}]}
  proxy | 2023/08/17 12:02:28 [178] GET https://auth.docker.io:443/token?service=registry.docker.io&scope=repository%3Alibrary%2Falpine%3Apull&account
  proxy | 2023/08/17 12:02:28 [178] 200 https://auth.docker.io:443/token?service=registry.docker.io&scope=repository%3Alibrary%2Falpine%3Apull&account

hey @ying-jeanne did you solve this? I'm getting lots of these all across my npm directories.

@lorengordon
Copy link
Contributor

If I create a group with update-types for patch and minor versions, do I still get individual PRs for major version updates?

        update-types:
          - patch
          - minor

@lorengordon
Copy link
Contributor

Oh, that's not working anyway. I'm seeing the same error as others:

updater | 2023/09/05 15:29:04 ERROR <job_718292262> undefined method `segments' for "0.24.11":String
updater | 
updater |           major: version.segments[0] || 0,
updater |                         ^^^^^^^^^
updater | 2023/09/05 15:29:04 ERROR <job_718292262> /home/dependabot/dependabot-updater/lib/dependabot/updater/group_update_creation.rb:233:in `semver_segments'
updater | 2023/09/05 15:29:04 ERROR <job_718292262> /home/dependabot/dependabot-updater/lib/dependabot/updater/group_update_creation.rb:221:in `semver_rules_allow_grouping?'

@williamfigtree
Copy link

Hi, providing a bit of feedback on this feature: My team want to group our PRs into "major" and "non-major" updates. The "update-types" parameter seems to solve for this however because it tracks the "highest resolvable version" when a dependency has a major and a non-major version increment the non-major group will not offer an update. This is a shame since I may be unable to update to a new major version immediately (or ever), but I would still like to update my dependency to consume any non-breaking improvements.

As an example: If my solution depends on a version 1.0.0 and a 1.0.1 and a 2.0.0 exists then the non-major dependabot group will not offer an update to 1.0.1.

I'd like to have an option to consume any minor or patch increments via dependabot regardless of whether a major increment exists. Perhaps this could be offered in a future update? Or perhaps I'm missing something!

@mfn
Copy link

mfn commented Sep 6, 2023

My team want to group our PRs into "major" and "non-major" updates

I also wanted this feature but realized, grouping major version updates does not make sense for my team (usually), because one major update of a dependency could also be a project on its own (unless it requires a major update of another dep, who knows).

Therefore I came to peace with not grouping major updates for now and I'm using this stanca here; I have not noticed it having a similar issue but maybe I didn't check properly:

  - package-ecosystem: npm
    directory: /
    schedule:
      time: "04:00"
      interval: weekly
    ignore:
      - dependency-name: "*"
        update-types:
          - version-update:semver-minor
          - version-update:semver-patch

  - package-ecosystem: npm
    directory: /
    schedule:
      time: "04:00"
      interval: weekly
    groups:
      deps:
        patterns:
          - '*'
        update-types:
          - minor
          - patch
  • major versions get separate PRs
  • minor/patch are grouped

@jurre
Copy link
Member Author

jurre commented Sep 6, 2023

@williamfigtree using ignore conditions is the recommended setup for this scenario. It does mean that you won't be aware of any major updates, but we ended up making this trade-off after a lot of deliberation as we figured it would be the least confusing behavior for most of our customers, keeping in line with how Dependabot has historically performed updates.

@mfn it doesn't seem like that configuration is valid, as we only allow one entry per ecosystem/directory? 🤔

@jurre
Copy link
Member Author

jurre commented Sep 6, 2023

If I create a group with update-types for patch and minor versions, do I still get individual PRs for major version updates?

        update-types:
          - patch
          - minor

@lorengordon yes.

Can you create a separate issue for the error you encountered with an example on how to reproduce, please? Thanks!

@mfn
Copy link

mfn commented Sep 6, 2023

@mfn it doesn't seem like that configuration is valid, as we only allow one entry per ecosystem/directory? 🤔

wat 😅

Maybe I didn't notice because there may have been no major versions since I added this, it's a smaller project; minor updates definitely work, I process them weekly.

@lorengordon
Copy link
Contributor

Can you create a separate issue for the error you encountered with an example on how to reproduce, please? Thanks!

@jurre Thanks, see #7978

@MuriloDalRi
Copy link

I have selected only minor and patch updates in my grouping and while it will open a separate PR for major updates it will also get included in the grouped PR, without any mention of it in the description.

Have I configured this incorrectly?

This is the grouped PR that includes a major Redis update.

@jakecoffman
Copy link
Member

Semver grouping is released, if you have any problems with it please file a new issue, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: grouped-updates 🎳 Relates to bumping more than one dependency in a single PR
Projects
None yet
Development

No branches or pull requests