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

Duplicate pull requests for .Net projects chained via project reference #1408

Closed
krlxgrn opened this issue Sep 27, 2019 · 15 comments
Closed
Labels
F: grouped-updates 🎳 Relates to bumping more than one dependency in a single PR F: monorepo 📦 Issues related to bumping a dep in manifests from multiple apps L: dotnet:nuget NuGet packages via nuget or dotnet T: feature-request Requests for new features

Comments

@krlxgrn
Copy link

krlxgrn commented Sep 27, 2019

When using the dependabot config file with multiple projects in a repository, I'm seeing multiple pull requests for the same package update.

To reproduce create a new .NET solution with two projects, A and B where B depends on A via a project reference.

Add an outdated package to project A.

Setup the dependabot config file so that each project is listed separately and 2 pull requests will be opened.

One will say Bump Package X in /A and the second will say Bump Package X in /B even though they are both updating the same package in the same project.

I would expect this to just open one pull request and additionally it's confusing seeing the PR title saying Bump Package X in /B when the code change is not changing project B directly at all.

I made an example repo that shows the issue, hopefully that helps https://github.com/k-arl/DependabotDuplication

@feelepxyz feelepxyz added the T: feature-request Requests for new features label Oct 23, 2019
@kilbergr

This comment was marked as off-topic.

@feelepxyz

This comment was marked as off-topic.

@kilbergr

This comment was marked as off-topic.

@feelepxyz

This comment was marked as off-topic.

@kilbergr

This comment was marked as off-topic.

@mwaddell

This comment was marked as off-topic.

@Jericho
Copy link

Jericho commented Mar 18, 2022

I just setup dependabot for the first time and it created several duplicate PRs, exactly as described by @krlxgrn. In my case it's even worse because my solution contains 4 projects, 3 of which reference the first one. Dependabot correctly detected 5 outdated NuGet package references in the first project and created the expected 5 PRs, but it also created 5 duplicate PRs for the second project, 5 duplicate PRs for the third project and 5 duplicate PRs for the fourth project.

In other words, instead of the 5 expected PRs, I ended up with 20 PRs, 15 of which are duplicates.

@jeffwidman jeffwidman added the L: dotnet:nuget NuGet packages via nuget or dotnet label Nov 24, 2022
@jeffwidman jeffwidman changed the title Duplicate pull requests Duplicate pull requests for .Net projects chained via project reference Nov 24, 2022
@jeffwidman jeffwidman added F: grouped-updates 🎳 Relates to bumping more than one dependency in a single PR F: monorepo 📦 Issues related to bumping a dep in manifests from multiple apps labels Feb 4, 2023
@jeffwidman
Copy link
Member

jeffwidman commented Feb 9, 2023

This only happens in a monorepo, right?

I'm not super familiar with .Net so unclear if the "projects" terminology refers to disparate apps/libraries, or to submodules for a single app. But it sounds like you've got a couple of libraries sitting alongside an actual app that pulls in those libraries...

@jeffwidman
Copy link
Member

@Jericho
Copy link

Jericho commented Feb 10, 2023

This only happens in a monorepo, right?

Yes, it's a single GitHub repository.

I'm not super familiar with .Net so unclear if the "projects" terminology refers to disparate apps/libraries, or to submodules for a single app. But it sounds like you've got a couple of libraries sitting alongside an actual app that pulls in those libraries...

In .Net you have a solution (a file with .sln extension) which contains links to one or multiple projects (if you use C#, your project files will have .csproj extension, if you use VB.NET the extension is .vbproj and if you use F# the extension is .fsproj). These project can reference nuget packages (these are the references we expect dependabot to keep up-to-date) but they can also reference each other. For example, your solution could include a library project and a unit tests project. In this example, the tests project would reference the library. There are a multitude of other project types (console app, web app, web API, winform, blazor, etc.), the two that I mentioned are some of the ones I personally use most often.

In fact, feel free to look at the GitHub repo where I experienced this problem, you'll see exactly what I'm talking about. While you're there, look at all the PRs that debendabot created on March 19 2022, you'll see the numerous duplicates.

When you look at the dupes, please pay attention to one detail in particular that may be confusing (it confused me at first before I realized what was going on):

  • Dependabot noticed an outdated nuget package reference in my library project and correctly raised this PR: Bump System.Text.Encoding.CodePages from 5.0.0 to 6.0.0 in /Source/StrongGrid
  • Dependabot looked at my unit testing project (which references my library project) and noticed that my library has an outdated nuget package reference and raised a duplicate PR: Bump System.Text.Encoding.CodePages from 5.0.0 to 6.0.0 in /Source/StrongGrid.UnitTests. The confusing part is the title of this duplicate PR makes it sound like the outdated package reference is in the unit testing project.
  • Dependabot looked at my third project which I use for benchmarking purposes (this project also references my library) and raised another duplicate PR: Bump System.Text.Encoding.CodePages from 5.0.0 to 6.0.0 in /Source/StrongGrid.Benchmark. Again, the title of the PR implies that the nuget reference is in the bechmarking project when in fact it's not.

Is this a duplicate??

It does indeed sound very similar.

@Jericho
Copy link

Jericho commented Feb 10, 2023

Is this a duplicate??

It does indeed sound very similar.

Actually, I take that back. I re-read the issue you linked and I think they are describing a completely different situation. What they are talking about is a case where project A and project B both reference nuget package Z. In this situation dependabot raises two PRs to upgrade the package Z references which they find "noisy". In their scenario the multiple PRs are valid, it's just that it would be more convenient if the various commits were grouped in a single PR. If you look at the screenshot in the July 2020 comment, you'll see 8 PRs to update a package to the latest version. My understanding is that there are indeed 8 references to the same package that need to be upgraded and these PRs are not duplicates.

In my scenario, dependabot raises duplicate PRs. So, it's not the same issue at all.

@abdulapopoola
Copy link
Member

Update: We've started doing some grouped updates work! This particular issue might not be part of the first ship but if you want to track our updates, do follow #1190.

@Jericho
Copy link

Jericho commented Apr 4, 2023

Grouping several commits into a single PR is a nice improvement and might resolve #1595 (I don't want to presume that the person who raised 1595 will be satisfied with grouped updates but I suspect they will be) but it will not help with the problem presented in this issue. @krlxgrn originally brought up the issue and @jeffwidman and I had a subsequent discussion where I attempted to clearly present the problem leading dependabot to create duplicates.

At the risk of repeating what's already been said many times in this discussion: the problem is that dependabot creates multiple PRs for the same update. The solution is not to group the multiple duplicates in a single PR. The solution is to avoid creating duplicates in the first place. I urge you to review the PRs created by dependabot in my repo on March 19 2022 in order to witness the several duplicates created by dependabot.

@jeffwidman
Copy link
Member

jeffwidman commented Apr 5, 2023

Thanks @Jericho and I do understand what you're asking for.

While this issue clearly affects you and a couple of other folks, the overall impact across our userbase appears to be pretty low (for example, currently no 👍 on the original comment), so it's unlikely to be prioritized by the core team anytime soon.

However, the great thing is this is all open source, so we're happy to review a PR and answer any questions along the way. But I just want to be transparent that this is unlikely to move forward until someone from the community steps up to fix it.

rdipardo added a commit to rdipardo/Fornax.Seo that referenced this issue Jun 6, 2023
@abdulapopoola
Copy link
Member

Closing out as this has not seen any activity in recent times; please feel free to reopen if otherwise.

@abdulapopoola abdulapopoola closed this as not planned Won't fix, can't repro, duplicate, stale Oct 9, 2023
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 F: monorepo 📦 Issues related to bumping a dep in manifests from multiple apps L: dotnet:nuget NuGet packages via nuget or dotnet T: feature-request Requests for new features
Projects
Archived in project
Development

No branches or pull requests

7 participants