search

dependabot / dependabot-core

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

Dependabot opens multiple PR's for the same dependencies #10293

Open p-linnane opened 4 months ago

p-linnane commented 4 months ago

Is there an existing issue for this?

Package ecosystem

Bundler

Package manager version

2.5.11

Language version

ruby 3.3.4p94

Manifest location and content before the Dependabot update

https://github.com/Homebrew/brew/blob/master/Library/Homebrew/Gemfile https://github.com/Homebrew/brew/blob/master/Library/Homebrew/Gemfile.lock

dependabot.yml content

https://github.com/Homebrew/brew/blob/master/.github/dependabot.yml

Updated dependency

sorbet, sorbet-runtime, sorbet-static, sorbet-static-and-runtime

What you expected to see, versus what you actually saw

Whenever Sorbet has a version update, Dependabot opens 3 PR's that update the same dependencies. We merge one, and then Dependabot rebases and closes the duplicates.

Native package manager behavior

No response

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

https://github.com/Homebrew/brew/pulls?q=is%3Apr+sort%3Aupdated-desc+is%3Aclosed+dependabot+sorbet

Smallest manifest that reproduces the issue

No response

p-linnane commented 3 months ago

@abdulapopoola Have you had a chance to check this out yet?

Bo98 commented 3 months ago

@deivid-rodriguez This seems to be from https://github.com/rubygems/rubygems/pull/7558. One effect I see from the logs is I think it is confusing requirements_to_unlock: :own to now act the same as requirements_to_unlock: :all. Given this comment:

https://github.com/dependabot/dependabot-core/blob/86ab940b72ad9253610e70af76a4834559b17567/bundler/helpers/v2/lib/functions/version_resolver.rb#L96-L104

I assume Dependabot is not expecting that to happen. The top-level dependencies are being unlocked here via unlocking the shared subdependencies.

(I realise you're not part of the Dependabot team anymore - just was asking your insight if you feel this is a Bundler-side issue given downgrading Bundler in the Dockerfile to 2.5.9 seems to make things work again. Apologies if not.)

deivid-rodriguez commented 3 months ago

Interesting, when I looked into this, Dependabot seemed to have some logic in place to avoid duplicate PRs, I remember some kind of error saying "There's an open PR updating the same dependency" or something like that, making it skip extra PRs. But maybe I did not try this correctly.

I'm happy to allocate some time to try figure out if this is something that should be improved in Bundler or Dependabot. I suspect we need to adapt something in Dependabot to avoid these duplicated PRs.

Regarding the code snippet that you shared, I think Bundler's conservative option actually no longer works like the comment expects 😬. See this issue. In any case, I believe the new logic added in https://github.com/rubygems/rubygems/pull/7558 to exceptionally unlock other top level dependencies if really necessary is independent from the conservative flag.

Bo98 commented 3 months ago

Interesting, when I looked into this, Dependabot seemed to have some logic in place to avoid duplicate PRs, I remember some kind of error saying "There's an open PR updating the same dependency" or something like that, making it skip extra PRs. But maybe I did not try this correctly.

I'm not an expert of the code so I don't know necessarily how it's supposed to work, but how it seems to currently work is linked to the requirements_to_unlock. If it reaches :all then it will deduplicate things correctly (for top-level dependencies anyway - I don't think it handles dependency-type: indirect very well so there's always been some duplication for those), but it will assume other top-level dependencies are unchanged under :own. Here's two logs before and after the Bundler bump:

updater | 2024/06/04 14:51:09 INFO <job_837345299> Checking if sorbet-static-and-runtime 0.5.11406 needs updating
  proxy | 2024/06/04 14:51:09 [365] GET https://rubygems.org:443/api/v1/versions/sorbet-static-and-runtime.json
  proxy | 2024/06/04 14:51:09 [365] 200 https://rubygems.org:443/api/v1/versions/sorbet-static-and-runtime.json
updater | 2024/06/04 14:51:09 INFO <job_837345299> Latest version is 0.5.11409
  proxy | 2024/06/04 14:51:10 [367] GET https://index.rubygems.org:443/versions
2024/06/04 14:51:10 [367] 304 https://index.rubygems.org:443/versions
  proxy | 2024/06/04 14:51:12 [369] GET https://index.rubygems.org:443/versions
2024/06/04 14:51:12 [369] 304 https://index.rubygems.org:443/versions
updater | 2024/06/04 14:51:15 INFO <job_837345299> Requirements to unlock all
2024/06/04 14:51:15 INFO <job_837345299> Requirements update strategy bump_versions
  proxy | 2024/06/04 14:51:16 [371] GET https://rubygems.org:443/api/v1/versions/sorbet-runtime.json
  proxy | 2024/06/04 14:51:16 [371] 200 https://rubygems.org:443/api/v1/versions/sorbet-runtime.json
updater | 2024/06/04 14:51:16 INFO <job_837345299> Filtered out 1 pre-release versions
  proxy | 2024/06/04 14:51:16 [373] GET https://index.rubygems.org:443/versions
  proxy | 2024/06/04 14:51:16 [373] 304 https://index.rubygems.org:443/versions
updater | 2024/06/04 14:51:19 INFO <job_837345299> Updating sorbet-static-and-runtime, sorbet-runtime
  proxy | 2024/06/04 14:51:19 [375] GET https://index.rubygems.org:443/versions
2024/06/04 14:51:19 [375] 304 https://index.rubygems.org:443/versions
updater | 2024/06/04 14:51:22 INFO <job_837345299> Submitting sorbet-static-and-runtime, sorbet-runtime pull request for creation
updater | 2024/08/20 18:28:18 INFO <job_871895671> Checking if sorbet-runtime 0.5.11528 needs updating
  proxy | 2024/08/20 18:28:18 [561] GET https://rubygems.org:443/api/v1/versions/sorbet-runtime.json
  proxy | 2024/08/20 18:28:18 [561] 200 https://rubygems.org:443/api/v1/versions/sorbet-runtime.json
updater | 2024/08/20 18:28:18 INFO <job_871895671> Filtered out 1 pre-release versions
updater | 2024/08/20 18:28:18 INFO <job_871895671> Latest version is 0.5.11531
  proxy | 2024/08/20 18:28:19 [563] GET https://index.rubygems.org:443/versions
2024/08/20 18:28:19 [563] 304 https://index.rubygems.org:443/versions
updater | 2024/08/20 18:28:21 INFO <job_871895671> Requirements to unlock own
2024/08/20 18:28:21 INFO <job_871895671> Requirements update strategy bump_versions
updater | 2024/08/20 18:28:21 INFO <job_871895671> Updating sorbet-runtime from 0.5.11528 to 0.5.11531
  proxy | 2024/08/20 18:28:22 [565] GET https://index.rubygems.org:443/versions
2024/08/20 18:28:22 [565] 304 https://index.rubygems.org:443/versions
updater | 2024/08/20 18:28:24 INFO <job_871895671> Submitting sorbet-runtime pull request for creation

(Note the "requirements to unlock" difference)

:own seems to use VersionResolver and :all seems to use ForceUpdater?

Regarding the code snippet that you shared, I think Bundler's conservative option actually no longer works like the comment expects 😬. See this issue. In any case, I believe the new logic added in rubygems/rubygems#7558 to exceptionally unlock other top level dependencies if really necessary is independent from the conservative flag.

Yeah, I agree with this assessment.