Grouped dependabot updates for NuGet updates more packages than it should #8578
Comments
|
Relevant logs: |
deivid-rodriguez
added
F: grouped-updates 🎳
|
Another case of a package being updated that is explicitly marked as ignored in the Dependabot configuration: martincostello/sqllocaldb#786 |
|
@martincostello ; the crew has been shipping a lot of fixes in this area; can you confirm if this still repros? |
|
When did the possible fix for this get shipped? I was observing issues like this yesterday, e.g. App-vNext/Polly-Samples#90 |
|
We've been shipping multiple daily fixes all week. IIUC, the issue is that grouping does not respect Tagging @jurre , @jakecoffman , @Nishnha as fyi |
|
It's possible I confused two different issues I logged you've commented on today. The above example was of a package that should have been updated as part of a group not being updated. |
|
Yeah; I've been a bit active and you are just as active 😄 It does seem that Polly got updated in a group; is the issue now resolved then? |
|
It looks like xunit was updated, but the native NuGet Updater updated too many xunit-related dependencies? I'd have to defer to @dependabot/azure-dev-ops's expertise on this one. |
|
In the example I gave the problem was that it should have updated three Polly packages, but it only updated two. I had to manually patch the third to get the intended result. |
|
Got it; tagging @sebasgomez238 |
|
These are peer dependencies that are required to be updated to avoid a package downgrade error when updating xunit. If ignoring these packages should trump updating xunit, then we should consider this a bug in the updater. xunit 2.6.3 depends on xunit.core 2.6.3
xunit.core 2.6.3 depends on xunit.extensibility.execution 2.6.3
xunit.core 2.6.3 also depends on xunit.extensibility.core 2.6.3 xunit.extensibility.core 2.6.3 depends on xunit.abstractions 2.0.3
|
|
That's true, but the reference I'm ignoring is for a library that only depends on the one I'm ignoring explicitly. That project's tests don't reference that package explicitly, and use the other two and transiently reference the one I'm ignoring. |
|
Essentially, I have one project that references the extensibility library which I never want dependabot to update because I want to target the lowest common denominator. Then I have the test project for that library that references the other two I always want to be kept up to date as a pair. The issue here is that dependabot is ignoring the ignore, but also not taking into account which projects are using the versions that are in the Directory.Packages.props file. |
|
@martincostello Thanks for pointing that out. I agree there is a bug here. We need to do a better job updating dependencies based on how the project sees them instead of how they exist in the workspace as a whole. |
|
I have a similar (or same) issue with this repository. Dependabot seems to get confused in a multi-targeting scenario and errors: Then in another repo it appears to be getting the dependency chain wrong(?) and failing: |
|
This morning dependabot has started opening PRs which either mention dependencies it states it has updated which are not present in the diff, or it generates package updates that should not have occurred.
Presumably a recent change has made this issue more prevalent than it was previously. |
|
Another interesting weird behaviour that just appeared - dependabot opened two PRs similar to the above updating things it shouldn't, but one of the two PRs then was closed as superseded by the second. |
|
@brettfo I'm triaging bugs with @abdulapopoola and came across this; how's it going? 🙏 |
|
@martincostello I'm trying to understand the original issue, but I'm not quite there: xUnit can't update from 2.6.2 to 2.6.3 because doing so would force xunit.abstractions to go from 2.0.2 to 2.0.3, so what would the desired output be? It can't be that xunit goes 2.6.2 => 2.6.3, but xunit.abstractions remains the same, because that's not a valid package graph. Is the desired behavior that the update from 2.6.2 doesn't happen because that would violate the other constraint? Edit: |
|
It definitely works, otherwise I wouldn't be able to manually update the reference because the code wouldn't compile. My understanding is that as xunit itself depends on a higher version of the package, it can be updated without also updating the reference in the referenced project as the test project will get the version xunit wants. If the graph traversal to derive that is too complicated, I would say consider the configuration of ignores to be a "I know better leave it alone" directive and for dependabot to honour it. |
|
On reflection, in fact yes, it's more that it's not honouring my ignores. "Update everything that matches xunit* except the ones I explicitly told you not to". I could change the group pattern to explicitly bump the two I want updated together, but I think it's counter-intuitive that ignores aren't being ignored. |
I've done this anyway, but updates are broken due to #9495 so I can't tell if it's made any difference. |
|
@martincostello the linked issue has now been fixed, are you still seeing this issue? |
|
Now I have a new problem and xunit isn't being updated at all. Unhandled exception: System.ArgumentException: '1.2.0.556' is not a valid version string. (Parameter 'value')
at NuGet.Versioning.SemanticVersion.Parse(String value) in /opt/nuget/lib/NuGet.Client/src/NuGet.Core/NuGet.Versioning/SemanticVersionFactory.cs:line 22
at NuGetUpdater.Core.Discover.SdkProjectDiscovery.GetTransitiveDependencies(String repoRootPath, String projectPath, ImmutableArray`1 tfms, ImmutableArray`1 directDependencies, Logger logger) in /opt/nuget/lib/NuGetUpdater/NuGetUpdater.Core/Discover/SdkProjectDiscovery.cs:line 114
at NuGetUpdater.Core.Discover.SdkProjectDiscovery.DiscoverAsync(String repoRootPath, String workspacePath, String projectPath, Logger logger) in /opt/nuget/lib/NuGetUpdater/NuGetUpdater.Core/Discover/SdkProjectDiscovery.cs:line 70
at NuGetUpdater.Core.Discover.DiscoveryWorker.RunForProjectPathsAsync(String repoRootPath, String workspacePath, IEnumerable`1 projectPaths) in /opt/nuget/lib/NuGetUpdater/NuGetUpdater.Core/Discover/DiscoveryWorker.cs:line 160
at NuGetUpdater.Core.Discover.DiscoveryWorker.RunForDirectoryAsnyc(String repoRootPath, String workspacePath) in /opt/nuget/lib/NuGetUpdater/NuGetUpdater.Core/Discover/DiscoveryWorker.cs:line 125
at NuGetUpdater.Core.Discover.DiscoveryWorker.RunAsync(String repoRootPath, String workspacePath, String outputPath) in /opt/nuget/lib/NuGetUpdater/NuGetUpdater.Core/Discover/DiscoveryWorker.cs:line 59
at NuGetUpdater.Cli.Commands.DiscoverCommand.<>c.<<GetCommand>b__4_0>d.MoveNext() in /opt/nuget/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/DiscoverCommand.cs:line 30 |
|
@martincostello , this PR should fix the issue. Can you recheck again? |
|
In a different repo I tried to update xunit and spotted this error in the logs after nothing happened: |
Is there an existing issue for this?
Package ecosystem
NuGet
Package manager version
.NET SDK 8.0.100
Language version
C# 12
Manifest location and content before the Dependabot update
Directory.Packages.props
dependabot.yml content
dependabot.yml
Updated dependency
xunit 2.6.2 => 2.6.3
What you expected to see, versus what you actually saw
Dependabot should have updated xunit, and only xunit, to 2.6.3.
Instead it also updates xunit.abstractions and xunit.extensibility.execution which are explicitly marked as ignored in
dependabot.yml.Expected
Actual
Native package manager behavior
No response
Images of the diff or a link to the PR, issue, or logs
martincostello/xunit-logging#533
Smallest manifest that reproduces the issue
No response
The text was updated successfully, but these errors were encountered: