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

Grouped dependency updates being assigned to incorrect group #8558

Open
1 task done
edmorley opened this issue Dec 7, 2023 · 10 comments
Open
1 task done

Grouped dependency updates being assigned to incorrect group #8558

edmorley opened this issue Dec 7, 2023 · 10 comments
Labels
F: grouped-updates 🎳 Relates to bumping more than one dependency in a single PR T: bug 🐞 Something isn't working

Comments

@edmorley
Copy link

edmorley commented Dec 7, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Package ecosystem

Cargo

Package manager version

1.74

Language version

1.74

Manifest location and content before the Dependabot update

https://github.com/heroku/languages-github-actions/blob/e280b86033bbe1f1f5a51a87d2dc71e89da09b76/Cargo.toml
https://github.com/heroku/languages-github-actions/blob/e280b86033bbe1f1f5a51a87d2dc71e89da09b76/Cargo.lock

dependabot.yml content

https://github.com/heroku/languages-github-actions/blob/e280b86033bbe1f1f5a51a87d2dc71e89da09b76/.github/dependabot.yml

Updated dependency

No response

What you expected to see, versus what you actually saw

Our Dependabot config has the libcnb* and libherokubuildpack packages assigned to a group named libcnb and all other minor/patch dependencies assigned to a group named rust-dependencies. The libcnb group is defined first, so should "win" if a package could fall into either group.

In this PR, the libcnb* packages are incorrectly grouped into the rust-dependencies group, rather than their own libcnb group:
heroku/languages-github-actions#177

Manually triggering a new Dependabot run (via "Insights" -> Dependency graph -> ... etc) didn't fix the PR.

And in fact the PR was then later recreated again with the same issue:
heroku/languages-github-actions#179

This seems to be a recent regression.

It has also affected other repos, for example this PR too:
heroku/buildpacks-go#193

Native package manager behavior

No response

Images of the diff or a link to the PR, issue, or logs

The initial broken PR was created in this job:
https://github.com/heroku/languages-github-actions/network/updates/757142572

This is a later rebase job, which I was hoping might fix the PR, but didn't:
https://github.com/heroku/languages-github-actions/network/updates/759606224

And then the second broken PR was opened in:
https://github.com/heroku/languages-github-actions/network/updates/759606988

Smallest manifest that reproduces the issue

No response

@edmorley edmorley added the T: bug 🐞 Something isn't working label Dec 7, 2023
@jakecoffman jakecoffman added the F: grouped-updates 🎳 Relates to bumping more than one dependency in a single PR label Dec 12, 2023
@jakecoffman
Copy link
Member

jakecoffman commented Jan 2, 2024

I think this might be an issue where the existing grouped pull requests are affecting new runs. In other words it got into a bad state from the previous code.

If I run a job locally with the Dependabot CLI it seems to produce the correct groups:

updater | +------------------------------------------------------------------------------------------------------------------------------------+
updater | |                                                Changes to Dependabot Pull Requests                                                 |
updater | +---------+--------------------------------------------------------------------------------------------------------------------------+
updater | | created | libcnb-common ( from 0.16.0 to 0.17.0 ), libcnb-data ( from 0.16.0 to 0.17.0 ), libcnb-package ( from 0.16.0 to 0.17.0 ) |
updater | | created | clap ( from 4.4.8 to 4.4.12 ), ignore ( from 0.4.20 to 0.4.21 ), semver ( from 1.0.20 to 1.0.21 ), serde_json ( from ... |
updater | | created | markdown ( from 1.0.0-alpha.14 to 1.0.0-alpha.16 )                                                                       |
updater | +---------+--------------------------------------------------------------------------------------------------------------------------+

The input I used for this is:

dependabot update -f input.yml

job:
  package-manager: cargo
  source:
    provider: github
    repo: heroku/languages-github-actions
    directory: "/."
    api-endpoint: https://api.github.com/
    hostname: github.com
    commit: 1ee2143f5925b1cce189ab663bcfa47bb38d6a18
  allowed-updates:
    - dependency-type: direct
      update-type: all
  dependency-groups:
    - name: libcnb
      rules:
        patterns:
          - libcnb*
          - libherokubuildpack
    - name: rust-dependencies
      rules:
        update-types:
          - minor
          - patch

Since I didn't include existing-group-pull-requests it's behaving correctly.

If we can get Dependabot to start over in a clean state I think it will create the correct PRs. Merging the PRs will work, or you can manually bump some of the dependencies and it should close the existing PRs.

@edmorley
Copy link
Author

edmorley commented Jan 2, 2024

Thank you for taking a look! I'm happy to close this out for now and see if it reoccurs in future update PRs once the current batch are already merged :-)

@edmorley edmorley closed this as completed Jan 2, 2024
@edmorley
Copy link
Author

@jakecoffman Hi again :-) Sadly this issue doesn't appear to be fixed - would you mind reopening it?

Latest PR showing this issue:
heroku/buildpacks-python#169

Note how the PR is for the rust-dependencies group, but includes the libcnb and libherokubuildpack dependencies, even though those are supposed to be in the libcnb group instead, and the libcnb group is listed first in the config:
https://github.com/heroku/buildpacks-python/blob/7aad98005608f274f6e89283ef05ab0bb8bbd49e/.github/dependabot.yml#L11-L20

The log of the run that opened this PR is:
https://github.com/heroku/buildpacks-python/network/updates/785965293

I tried triggering another run to see if Dependabot fixed the PR when rebasing instead, but it didn't:
https://github.com/heroku/buildpacks-python/network/updates/785966510

@jakecoffman jakecoffman reopened this Feb 12, 2024
@jakecoffman
Copy link
Member

Recording a few things I noticed now:

Job 785965293 was a rebase job and says:

updater | 2024/02/12 14:17:58 INFO <job_785965293> Dependencies have changed, closing existing Pull Request
updater | 2024/02/12 14:17:58 INFO <job_785965293> Telling backend to close pull request for the rust-dependencies group (serde, ureq) - dependencies changed

So it ran a rebase of heroku/buildpacks-python#165 and it looks like it considered all dependencies of that group instead of focusing on rebasing the two that were already in it (serde, ureq). That means it pulled in dependencies that the other group would normally cover, but I'd guess there wasn't a PR for it yet at the time.

Rebases seem to be pretty tricky because other groups can affect the group you're rebasing, so you kind of have to run the whole thing again. I'll try to get some better testing around this scenario and figure out how to fix it.

@edmorley
Copy link
Author

@jakecoffman Thank you for taking a look! We have most of our repos set to monthly schedule, so if there was any way for this to be fixed before 1st March, it would save us a lot of incorrect PRs :-)

edmorley added a commit to heroku/buildpacks-php that referenced this issue Mar 1, 2024
Since it's not supposed to be updated in this group, but Dependabot
grouping has some bugs:
dependabot/dependabot-core#8558
edmorley added a commit to heroku/buildpacks-php that referenced this issue Mar 1, 2024
* Bump the rust-dependencies group with 8 updates

Bumps the rust-dependencies group with 8 updates:

| Package | From | To |
| --- | --- | --- |
| [chrono](https://github.com/chronotope/chrono) | `0.4.31` | `0.4.34` |
| [regex](https://github.com/rust-lang/regex) | `1.10.2` | `1.10.3` |
| [serde](https://github.com/serde-rs/serde) | `1.0.195` | `1.0.197` |
| [serde_json](https://github.com/serde-rs/json) | `1.0.111` | `1.0.114` |
| [ureq](https://github.com/algesten/ureq) | `2.9.1` | `2.9.6` |
| [serde_with](https://github.com/jonasbb/serde_with) | `3.4.0` | `3.6.1` |


Updates `chrono` from 0.4.31 to 0.4.34
- [Release notes](https://github.com/chronotope/chrono/releases)
- [Changelog](https://github.com/chronotope/chrono/blob/main/CHANGELOG.md)
- [Commits](chronotope/chrono@v0.4.31...v0.4.34)

Updates `regex` from 1.10.2 to 1.10.3
- [Release notes](https://github.com/rust-lang/regex/releases)
- [Changelog](https://github.com/rust-lang/regex/blob/master/CHANGELOG.md)
- [Commits](rust-lang/regex@1.10.2...1.10.3)

Updates `serde` from 1.0.195 to 1.0.197
- [Release notes](https://github.com/serde-rs/serde/releases)
- [Commits](serde-rs/serde@v1.0.195...v1.0.197)

Updates `serde_json` from 1.0.111 to 1.0.114
- [Release notes](https://github.com/serde-rs/json/releases)
- [Commits](serde-rs/json@v1.0.111...v1.0.114)

Updates `ureq` from 2.9.1 to 2.9.6
- [Changelog](https://github.com/algesten/ureq/blob/main/CHANGELOG.md)
- [Commits](algesten/ureq@2.9.1...2.9.6)

Updates `serde_with` from 3.4.0 to 3.6.1
- [Release notes](https://github.com/jonasbb/serde_with/releases)
- [Commits](jonasbb/serde_with@v3.4.0...v3.6.1)

---
updated-dependencies:
- dependency-name: chrono
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: rust-dependencies
- dependency-name: libcnb
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: rust-dependencies
- dependency-name: libherokubuildpack
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: rust-dependencies
- dependency-name: regex
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: rust-dependencies
- dependency-name: serde
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: rust-dependencies
- dependency-name: serde_json
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: rust-dependencies
- dependency-name: ureq
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: rust-dependencies
- dependency-name: serde_with
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: rust-dependencies
...

Signed-off-by: dependabot[bot] <[email protected]>

* Downgrade `libherokubuildpack`

Since it's not supposed to be updated in this group, but Dependabot
grouping has some bugs:
dependabot/dependabot-core#8558

* Disable unused Cargo features

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ed Morley <[email protected]>
edmorley added a commit to heroku/buildpacks-nodejs that referenced this issue Mar 1, 2024
Since it's not supposed to be updated in this group, but Dependabot
grouping has some bugs:
dependabot/dependabot-core#8558
edmorley added a commit to heroku/buildpacks-nodejs that referenced this issue Mar 1, 2024
* Bump the rust-dependencies group with 4 updates

Bumps the rust-dependencies group with 4 updates: [libherokubuildpack](https://github.com/heroku/libcnb.rs), [opentelemetry](https://github.com/open-telemetry/opentelemetry-rust), [opentelemetry_sdk](https://github.com/open-telemetry/opentelemetry-rust) and [opentelemetry-stdout](https://github.com/open-telemetry/opentelemetry-rust).

Updates `opentelemetry` from 0.21.0 to 0.22.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-rust/releases)
- [Commits](open-telemetry/opentelemetry-rust@v0.21.0...v0.22.0)

Updates `opentelemetry_sdk` from 0.21.2 to 0.22.1
- [Release notes](https://github.com/open-telemetry/opentelemetry-rust/releases)
- [Commits](open-telemetry/opentelemetry-rust@v0.21.2...v0.22.1)

Updates `opentelemetry-stdout` from 0.2.0 to 0.3.0
- [Release notes](https://github.com/open-telemetry/opentelemetry-rust/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-rust/blob/v0.3.0/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-rust@v0.2.0...v0.3.0)

---
updated-dependencies:
- dependency-name: libherokubuildpack
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: rust-dependencies
- dependency-name: opentelemetry
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: rust-dependencies
- dependency-name: opentelemetry_sdk
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: rust-dependencies
- dependency-name: opentelemetry-stdout
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: rust-dependencies
...

Signed-off-by: dependabot[bot] <[email protected]>

* Downgrade `libherokubuildpack`

Since it's not supposed to be updated in this group, but Dependabot
grouping has some bugs:
dependabot/dependabot-core#8558

* Disable unused default Cargo features

* Bump minimum Rust version to 1.76

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ed Morley <[email protected]>
@Nishnha
Copy link
Member

Nishnha commented Apr 4, 2024

@edmorley Hi I was wondering if your monthly PR for April still showed dependencies being assigned to the wrong group?

@edmorley
Copy link
Author

edmorley commented Apr 8, 2024

@Nishnha Hi!

I manually fixed up the PRs we had, and there hasn't been a libcnb release since then (which is what triggers the issue), so I haven't been able to confirm whether this is resolved either way.

Is there a specific fix that's landed that you believe may have resolved this?

I'm hesitant to close the issue without knowing a change has been made, given how many times problems in this area have re-surfaced (both in this issue thread, and in prior issues I've filed: https://github.com/dependabot/dependabot-core/issues?q=is%3Aissue+author%3Aedmorley+label%3A%22F%3A+grouped-updates+%F0%9F%8E%B3%22 ).

@jurre
Copy link
Member

jurre commented Apr 10, 2024

I think there's still the possibility of this happening, I'd like to keep the issue open for now. There are some changes we're investigating in how we perform PR creation and rebases for grouped PRs that I think will resolve this, but they're rather large changes

@pmk1c
Copy link

pmk1c commented May 27, 2024

I can confirm this still happens, when changing grouping rules and having Dependabot PRs open with the "old" group rules. Changing group names i.e. will lead to correct group PRs.

@aniruddh622003
Copy link

Yeah we have been trying to experiment with the PR group rule. And by going through logs, it definetly shows that it found all relevant groups (8 in my case), and then goes to process the groups that were present in earlier dependabot.yml versions. Ignoring the order of appearance (I also tried to move these groups to the very bottom).

But I am not very sure if this behaviour is due to the "old rules" or because these groups have a univeral wildcard("*") in them.. Please inform me if you want the logs, or the dependabot.yml file to get more information.

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 T: bug 🐞 Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants