Skip to content

Commit aea7daa

Browse files
jeffkljeffkl
authored
Fix version conflict detection in new dependency resolver (#6999)
1 parent 64cc827 commit aea7daa

2 files changed

Lines changed: 228 additions & 32 deletions

File tree

‎src/NuGet.Core/NuGet.Commands/RestoreCommand/DependencyGraphResolver.cs‎

Lines changed: 84 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,7 @@ public async Task<ValueTuple<bool, List<RestoreTargetGraph>, RuntimeGraph>> Reso
396396
}
397397

398398
LibraryDependencyIndex childLibraryDependencyIndex = resolvedDependencyGraphItem.GetDependencyIndexForDependencyAt(i);
399+
LibraryRangeIndex currentRangeIndex = resolvedDependencyGraphItem.GetRangeIndexForDependencyAt(i);
399400

400401
if (!resolvedDependencyGraphItems.TryGetValue(childLibraryDependencyIndex, out ResolvedDependencyGraphItem? childResolvedDependencyGraphItem))
401402
{
@@ -409,8 +410,6 @@ public async Task<ValueTuple<bool, List<RestoreTargetGraph>, RuntimeGraph>> Reso
409410
// Determine if this dependency has already been visited
410411
if (!visitedItems.Add(childLibraryDependencyIndex))
411412
{
412-
LibraryRangeIndex currentRangeIndex = resolvedDependencyGraphItem.GetRangeIndexForDependencyAt(i);
413-
414413
if (resolvedDependencyGraphItem.Path.Contains(currentRangeIndex))
415414
{
416415
// If the dependency exists in the its own path, then a cycle exists
@@ -513,10 +512,27 @@ public async Task<ValueTuple<bool, List<RestoreTargetGraph>, RuntimeGraph>> Reso
513512
continue;
514513
}
515514

515+
if (TryDetectVersionConflict(
516+
currentGraphNode,
517+
childResolvedDependencyGraphItem,
518+
versionConflicts,
519+
downgrades,
520+
childLibraryDependency,
521+
currentRangeIndex,
522+
childResolvedLibraryRangeIndex,
523+
out GraphNode<RemoteResolveResult>? nodeWithConflict))
524+
{
525+
currentGraphNode.InnerNodes.Add(nodeWithConflict);
526+
527+
nodesById.Add(currentRangeIndex, nodeWithConflict);
528+
529+
continue;
530+
}
531+
516532
// If it wasn't a downgrade, then it was a version conflict like A -> B [1.0.0] but B 1.0.0 was not in the resolved graph
517533
if (versionConflicts.ContainsKey(childResolvedLibraryRangeIndex) && !nodesById.ContainsKey(currentRangeIndex))
518534
{
519-
GraphNode<RemoteResolveResult> nodeWithConflict = new(childResolvedLibraryDependency.LibraryRange)
535+
nodeWithConflict = new(childResolvedLibraryDependency.LibraryRange)
520536
{
521537
Item = childResolvedDependencyGraphItem.Item,
522538
Disposition = Disposition.Acceptable,
@@ -581,43 +597,26 @@ public async Task<ValueTuple<bool, List<RestoreTargetGraph>, RuntimeGraph>> Reso
581597
));
582598
}
583599

584-
// This is a version conflict if:
585-
// 1. The node is not a project and isn't unresolved
586-
// 2. The conflict has not already been detected
587-
// 3. The dependency is transitive and doesn't have PrivateAssets=All
588-
// 4. The dependency has a version specified
589-
// 5. The version range is not satisfied by the resolved version
590-
// 6. A corresponding downgrade was not detected
591-
if (newGraphNode.Item.Key.Type != LibraryType.Project
592-
&& newGraphNode.Item.Key.Type != LibraryType.ExternalProject
593-
&& newGraphNode.Item.Key.Type != LibraryType.Unresolved
594-
&& !versionConflicts.ContainsKey(childResolvedLibraryRangeIndex)
595-
&& childLibraryDependency.SuppressParent != LibraryIncludeFlags.All
596-
&& childLibraryDependency.LibraryRange.VersionRange != null
597-
&& !childLibraryDependency.LibraryRange.VersionRange!.Satisfies(newGraphNode.Item.Key.Version)
598-
&& !downgrades.ContainsKey(childResolvedLibraryRangeIndex))
600+
if (TryDetectVersionConflict(
601+
currentGraphNode,
602+
childResolvedDependencyGraphItem,
603+
versionConflicts, downgrades,
604+
childLibraryDependency,
605+
currentRangeIndex,
606+
childResolvedLibraryRangeIndex,
607+
out GraphNode<RemoteResolveResult>? conflictingNode))
599608
{
600609
// Remove the existing node so it can be replaced with a node representing the conflict
601610
currentGraphNode.InnerNodes.Remove(newGraphNode);
602611

603-
GraphNode<RemoteResolveResult> conflictingNode = new(childLibraryDependency.LibraryRange)
604-
{
605-
Disposition = Disposition.Acceptable,
606-
Item = new GraphItem<RemoteResolveResult>(
607-
new LibraryIdentity(
608-
childLibraryDependency.Name,
609-
childLibraryDependency.LibraryRange.VersionRange.MinVersion!,
610-
LibraryType.Package)),
611-
OuterNode = currentGraphNode,
612-
};
613-
614612
// Add the conflict node to the parent
615613
currentGraphNode.InnerNodes.Add(conflictingNode);
616614

617-
// Track the version conflict for later
618-
versionConflicts.Add(childResolvedLibraryRangeIndex, conflictingNode);
615+
nodesById.Add(currentRangeIndex, conflictingNode);
616+
617+
// This node conflicts and won't be included in the final graph
618+
visitedItems.Remove(childLibraryDependencyIndex);
619619

620-
// Process the next child
621620
continue;
622621
}
623622

@@ -752,6 +751,59 @@ public async Task<ValueTuple<bool, List<RestoreTargetGraph>, RuntimeGraph>> Reso
752751
resolvedDependencies: resolvedPackages);
753752

754753
return (success, restoreTargetGraph);
754+
755+
static bool TryDetectVersionConflict(
756+
GraphNode<RemoteResolveResult> currentGraphNode,
757+
ResolvedDependencyGraphItem resolvedDependencyGraphItem,
758+
Dictionary<LibraryRangeIndex, GraphNode<RemoteResolveResult>> versionConflicts,
759+
Dictionary<LibraryRangeIndex, (LibraryRangeIndex FromParentLibraryRangeIndex, LibraryDependency FromLibraryDependency, LibraryRangeIndex ToParentLibraryRangeIndex, LibraryDependencyIndex ToLibraryDependencyIndex, bool IsCentralTransitive)> downgrades,
760+
LibraryDependency childLibraryDependency,
761+
LibraryRangeIndex childLibraryDependencyIndex,
762+
LibraryRangeIndex childResolvedLibraryRangeIndex,
763+
[NotNullWhen(true)] out GraphNode<RemoteResolveResult>? conflictingNode)
764+
{
765+
conflictingNode = null;
766+
767+
if (resolvedDependencyGraphItem.IsRootPackageReference)
768+
{
769+
return false;
770+
}
771+
772+
// This is a version conflict if:
773+
// 1. The node is not a project and isn't unresolved
774+
// 2. The conflict has not already been detected
775+
// 3. The dependency is transitive and doesn't have PrivateAssets=All
776+
// 4. The dependency has a version specified
777+
// 5. The version range is not satisfied by the resolved version
778+
// 6. A corresponding downgrade was not detected
779+
if (resolvedDependencyGraphItem.Item.Key.Type != LibraryType.Project
780+
&& resolvedDependencyGraphItem.Item.Key.Type != LibraryType.ExternalProject
781+
&& resolvedDependencyGraphItem.Item.Key.Type != LibraryType.Unresolved
782+
&& !versionConflicts.ContainsKey(childResolvedLibraryRangeIndex)
783+
&& childLibraryDependency.SuppressParent != LibraryIncludeFlags.All
784+
&& childLibraryDependency.LibraryRange.VersionRange != null
785+
&& !childLibraryDependency.LibraryRange.VersionRange!.Satisfies(resolvedDependencyGraphItem.Item.Key.Version)
786+
&& !downgrades.ContainsKey(childResolvedLibraryRangeIndex))
787+
{
788+
conflictingNode = new(childLibraryDependency.LibraryRange)
789+
{
790+
Disposition = Disposition.Rejected,
791+
Item = new GraphItem<RemoteResolveResult>(
792+
new LibraryIdentity(
793+
childLibraryDependency.Name,
794+
childLibraryDependency.LibraryRange.VersionRange.MinVersion!,
795+
LibraryType.Package)),
796+
OuterNode = currentGraphNode,
797+
};
798+
799+
// Track the version conflict for later
800+
versionConflicts.Add(childResolvedLibraryRangeIndex, conflictingNode);
801+
802+
return true;
803+
}
804+
805+
return false;
806+
}
755807
}
756808

757809
private static bool EvaluateRuntimeDependencies(ref LibraryDependency libraryDependency, RuntimeGraph? runtimeGraph, string? runtimeIdentifier, ref HashSet<LibraryDependency>? runtimeDependencies)

‎test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/RestoreCommand_AlgorithmEquivalencyTests.cs‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2745,6 +2745,150 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async(
27452745
result2.LockFile.Targets[0].Libraries[1].Version.Should().Be(new NuGetVersion("1.0.0"));
27462746
}
27472747

2748+
[Theory]
2749+
// Project -> D 2.0.0
2750+
// -> A 2.0.0
2751+
// -> C >= 2.0.0
2752+
// -> D 2.0.0
2753+
// -> B 1.0.0
2754+
// -> C >= 1.0.0 & < 2.0.0
2755+
// -> D [1.0.0, 2.0.0)
2756+
// -> A [1.0.0, 2.0.0)
2757+
[InlineData("\"D\":\"2.0.0\",\r\n\"A\":\"2.0.0\",\r\n\"B\":\"1.0.0\",")]
2758+
// Project -> D 2.0.0
2759+
// -> B 1.0.0
2760+
// -> C >= 1.0.0 & < 2.0.0
2761+
// -> D >= 1.0.0 & < 2.0.0)
2762+
// -> A >= 1.0.0 & < 2.0.0)
2763+
// -> A 2.0.0
2764+
// -> C >= 2.0.0
2765+
// -> D 2.0.0
2766+
[InlineData("\"D\":\"2.0.0\",\r\n\"B\":\"1.0.0\",\r\n\"A\":\"2.0.0\",")]
2767+
public async Task RestoreCommand_WhenGraphHasAnUnresolvableRangeConflict_RaisesNU1107_VerifiesEquivalency(string deps)
2768+
{
2769+
// Arrange
2770+
using var pathContext = new SimpleTestPathContext();
2771+
2772+
var D = new SimpleTestPackageContext("D", "2.0.0");
2773+
2774+
var C = new SimpleTestPackageContext("C", "2.0.0")
2775+
{
2776+
Dependencies =
2777+
[
2778+
D,
2779+
]
2780+
};
2781+
2782+
var B = new SimpleTestPackageContext("B", "1.0.0")
2783+
{
2784+
Dependencies =
2785+
[
2786+
new SimpleTestPackageContext("C", "[1.0.0, 2.0.0)"),
2787+
new SimpleTestPackageContext("D", "[1.0.0, 2.0.0)"),
2788+
new SimpleTestPackageContext("A", "[1.0.0, 2.0.0)"),
2789+
]
2790+
};
2791+
2792+
var A = new SimpleTestPackageContext("A", "2.0.0")
2793+
{
2794+
Dependencies =
2795+
[
2796+
C,
2797+
]
2798+
};
2799+
2800+
await SimpleTestPackageUtility.CreatePackagesWithoutDependenciesAsync(pathContext.PackageSource,
2801+
B,
2802+
A,
2803+
C,
2804+
D);
2805+
2806+
2807+
// Switch the order in which A & B are declared
2808+
var projectSpec = @$"
2809+
{{
2810+
""frameworks"": {{
2811+
""net10.0"": {{
2812+
""dependencies"": {{
2813+
{deps}
2814+
}}
2815+
}}
2816+
}}
2817+
}}";
2818+
2819+
var packageSpec = ProjectTestHelpers.GetPackageSpecWithProjectNameAndSpec("Project", pathContext.SolutionRoot, projectSpec);
2820+
2821+
// Act & Assert
2822+
(var result, _) = await ValidateRestoreAlgorithmEquivalency(pathContext, packageSpec);
2823+
}
2824+
2825+
// Project -> B 1.0.0
2826+
// -> C [1.0.0, 2.0.0)
2827+
// -> D [1.0.0, 2.0.0)
2828+
// -> A [1.0.0, 2.0.0)
2829+
// -> A 2.0.0 -> C 2.0.0 -> D 2.0.0
2830+
// -> C 2.0.0
2831+
// -> D 2.0.0
2832+
[Fact]
2833+
public async Task RestoreCommand_WithConflictsOverridenByADirectDependencyRaisesNU1608_VerifiesEquivalency()
2834+
{
2835+
// Arrange
2836+
using var pathContext = new SimpleTestPathContext();
2837+
2838+
var D = new SimpleTestPackageContext("D", "2.0.0");
2839+
2840+
var C = new SimpleTestPackageContext("C", "2.0.0")
2841+
{
2842+
Dependencies =
2843+
[
2844+
D,
2845+
]
2846+
};
2847+
2848+
var B = new SimpleTestPackageContext("B", "1.0.0")
2849+
{
2850+
Dependencies =
2851+
[
2852+
new SimpleTestPackageContext("C", "[1.0.0, 2.0.0)"),
2853+
new SimpleTestPackageContext("D", "[1.0.0, 2.0.0)"),
2854+
new SimpleTestPackageContext("A", "[1.0.0, 2.0.0)"),
2855+
]
2856+
};
2857+
2858+
var A = new SimpleTestPackageContext("A", "2.0.0")
2859+
{
2860+
Dependencies =
2861+
[
2862+
C,
2863+
]
2864+
};
2865+
2866+
await SimpleTestPackageUtility.CreatePackagesWithoutDependenciesAsync(pathContext.PackageSource,
2867+
B,
2868+
A,
2869+
C,
2870+
D);
2871+
2872+
var projectSpec = @"
2873+
{
2874+
""frameworks"": {
2875+
""net10.0"": {
2876+
""dependencies"": {
2877+
""D"": ""2.0.0"",
2878+
""C"": ""2.0.0"",
2879+
""A"": ""2.0.0"",
2880+
""B"": ""1.0.0"",
2881+
}
2882+
}
2883+
}
2884+
}";
2885+
2886+
var packageSpec = ProjectTestHelpers.GetPackageSpecWithProjectNameAndSpec("Project", pathContext.SolutionRoot, projectSpec);
2887+
2888+
// Act & Assert
2889+
(var result, _) = await ValidateRestoreAlgorithmEquivalency(pathContext, packageSpec);
2890+
}
2891+
27482892
// P -> A 1.0.0
27492893
// -> B (no version)
27502894
// B is pinned to 1.0.0

0 commit comments

Comments
 (0)