dependabot / dependabot-core

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

Multiple PRs being opened for the same project and dependency for wrong path #3287

Open mairaw opened 3 years ago

mairaw commented 3 years ago

Package manager/ecosystem NuGet What you expected to see, versus what you actually saw Here's my yml file

version: 2
updates:
- package-ecosystem: npm
  directory: "/src/netlandingpage"
  schedule:
    interval: weekly
  open-pull-requests-limit: 10
- package-ecosystem: nuget
  directory: "/src/netlandingpage"
  schedule:
    interval: weekly
  open-pull-requests-limit: 10
- package-ecosystem: nuget
  directory: "/src/netlive/Client"
  schedule:
    interval: weekly
  open-pull-requests-limit: 10
- package-ecosystem: nuget
  directory: "/src/netlive/Shared"
  schedule:
    interval: weekly
  open-pull-requests-limit: 10

Dependabot opened a PR for each path I have enabled on my yml file, even though that dependency was only present in src/Common image

The contents of each PR was exactly the same. image

feelepxyz commented 3 years ago

@mairaw thanks for reporting, we'll look into it, definitely looks like a bug.

feelepxyz commented 3 years ago

@mairaw hm haven't been able to reproduce with a similar looking setup so wondering if your csproj files are importing any files? Would it be possible for you to put together a public test repo that has the same manifest files? You could strip them down to just including this one dependency. Tricky to debug further without more context on which files are referenced from each csproj file.

mairaw commented 3 years ago

Sure, let me try @feelepxyz

mairaw commented 3 years ago

The repro is located at https://github.com/mairaw/dependabot-repro

You can see the same behavior there.

feelepxyz commented 3 years ago

@mairaw thanks for putting that together 🙇

It looks like each project is referencing src/Common/Common.csproj which is why dependabot thinks each one needs to be updated and references the pull request title/branch to the project that was configured in dependabot.yml.

Will have a look at fixing this but not sure there's an easy fix with the way dependabot is currently configured. Each update currently doesn't know about the others so each one will think it's the only one updating the referenced project from <ProjectReference Include="..\Common\Common.csproj" />.

A hacky workaround would be to create a single .csproj file that requires all others using a <ProjectReference tag and then only configure that folder with dependabot until we have a way to deal with these kinds of cross-referenced project setups.

I tested this out on this fork: https://github.com/feelepxyz/dependabot-repro/pull/2

mairaw commented 3 years ago

I'll continue to deal with the extra PRs but I don't understand why dependabot is also checking the version on the referenced projects too. Shouldn't it just review the versions for projects under the path submitted and nothing else?

feelepxyz commented 3 years ago

@mairaw Dependabot will pull referenced files and update these if the same dependency exists there, the thinking is that you don't end up with lots of different versions of the same dependency. We went down the route of updating referenced projects from previous feedback and I think there are a users who rely on this feature.

It's not trivial to fix this unfortunately.

mairaw commented 3 years ago

But is there a way for me to say that I want the whole repo scanned? I think I tried before but it wouldn't scan all the projects if I didn't add them individually like I show here.

Another option could be to flag whether I want to scan reference projects too. Default value could be true since this is the default behavior, but then I could say don't scan and this issue would be solved for me.

github-actions[bot] commented 6 months ago

👋 This issue has been marked as stale because it has been open for 2 years with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the Keep label to hold stale off permanently, or do nothing. If you do nothing, this issue will be closed eventually by the stale bot. Please see CONTRIBUTING.md for more policy details.

mairaw commented 6 months ago

This is still an issue.

brettfo commented 6 months ago

Thanks to @mairaw for providing a sample repro repo (pun intended).

This is an interesting one that will likely need to be fixed in some internal code that creates the PR. For example, PR mairaw/dependabot-repro#457 was started in directory /src/Updater.Resources then PR mairaw/dependabot-repro#457 was started in directory /src/BlobAssets. Both update processes eventually identified the <PackageReference> node that needed to be updated in /src/Common/Common.csproj and the updater correctly made that change, however, the updater doesn't have the knowledge (and as far as I know, it can't get the knowledge) that another PR exists that makes the exact same change, hence the duplicated work.

I would hope/expect the PR generation code to be able to look for identical changes and instead of creating a separate PR, update the existing one with additional information.

I'm going to pull in @jakecoffman on this to check my understanding.

mairaw commented 6 months ago

Yeah, what I usually do is to look at the multiple PRs that I get and pick the one that actually matches the path being described in the title.

For example, https://github.com/mairaw/dependabot-repro/pull/458 says in the title that updated in /src/BlobAssets, but the actual change didn't happen in that path. The one that matches the path in the title is https://github.com/mairaw/dependabot-repro/pull/451.

jakecoffman commented 6 months ago

Since directory: /src/Common isn't specified in the dependabot.yml I wouldn't expect it to be updated at all, unless it was a security update which explicitly would try to update the manifests in that directory.

mairaw commented 1 month ago

Actually it is on this line @jakecoffman https://github.com/mairaw/dependabot-repro/blob/master/.github/dependabot.yml#L9