dependabot / dependabot-core

🤖 Dependabot's core logic for creating update PRs.
https://docs.github.com/en/code-security/dependabot
MIT License
4.65k stars 1k forks source link

Grouped dependency updates being assigned to incorrect group #8558

Open edmorley opened 10 months ago

edmorley commented 10 months ago

Is there an existing issue for this?

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: https://github.com/heroku/languages-github-actions/pull/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: https://github.com/heroku/languages-github-actions/pull/179

This seems to be a recent regression.

It has also affected other repos, for example this PR too: https://github.com/heroku/buildpacks-go/pull/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

jakecoffman commented 9 months ago

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 commented 9 months ago

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 commented 7 months ago

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

Latest PR showing this issue: https://github.com/heroku/buildpacks-python/pull/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 commented 7 months ago

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 https://github.com/heroku/buildpacks-python/pull/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 commented 7 months ago

@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 :-)

Nishnha commented 6 months ago

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

edmorley commented 5 months ago

@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 commented 5 months ago

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 commented 4 months ago

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.