Skip to content

Commit 8719c86

Browse files
jeffkljeffkl
authored
Revert "Fix version conflict detection in new dependency resolver" (#7018)
1 parent a63ce8f commit 8719c86

2 files changed

Lines changed: 32 additions & 228 deletions

File tree

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

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

398398
LibraryDependencyIndex childLibraryDependencyIndex = resolvedDependencyGraphItem.GetDependencyIndexForDependencyAt(i);
399-
LibraryRangeIndex currentRangeIndex = resolvedDependencyGraphItem.GetRangeIndexForDependencyAt(i);
400399

401400
if (!resolvedDependencyGraphItems.TryGetValue(childLibraryDependencyIndex, out ResolvedDependencyGraphItem? childResolvedDependencyGraphItem))
402401
{
@@ -410,6 +409,8 @@ public async Task<ValueTuple<bool, List<RestoreTargetGraph>, RuntimeGraph>> Reso
410409
// Determine if this dependency has already been visited
411410
if (!visitedItems.Add(childLibraryDependencyIndex))
412411
{
412+
LibraryRangeIndex currentRangeIndex = resolvedDependencyGraphItem.GetRangeIndexForDependencyAt(i);
413+
413414
if (resolvedDependencyGraphItem.Path.Contains(currentRangeIndex))
414415
{
415416
// If the dependency exists in the its own path, then a cycle exists
@@ -512,27 +513,10 @@ public async Task<ValueTuple<bool, List<RestoreTargetGraph>, RuntimeGraph>> Reso
512513
continue;
513514
}
514515

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-
532516
// 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
533517
if (versionConflicts.ContainsKey(childResolvedLibraryRangeIndex) && !nodesById.ContainsKey(currentRangeIndex))
534518
{
535-
nodeWithConflict = new(childResolvedLibraryDependency.LibraryRange)
519+
GraphNode<RemoteResolveResult> nodeWithConflict = new(childResolvedLibraryDependency.LibraryRange)
536520
{
537521
Item = childResolvedDependencyGraphItem.Item,
538522
Disposition = Disposition.Acceptable,
@@ -597,26 +581,43 @@ public async Task<ValueTuple<bool, List<RestoreTargetGraph>, RuntimeGraph>> Reso
597581
));
598582
}
599583

600-
if (TryDetectVersionConflict(
601-
currentGraphNode,
602-
childResolvedDependencyGraphItem,
603-
versionConflicts, downgrades,
604-
childLibraryDependency,
605-
currentRangeIndex,
606-
childResolvedLibraryRangeIndex,
607-
out GraphNode<RemoteResolveResult>? conflictingNode))
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))
608599
{
609600
// Remove the existing node so it can be replaced with a node representing the conflict
610601
currentGraphNode.InnerNodes.Remove(newGraphNode);
611602

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+
612614
// Add the conflict node to the parent
613615
currentGraphNode.InnerNodes.Add(conflictingNode);
614616

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

620+
// Process the next child
620621
continue;
621622
}
622623

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

753754
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-
}
807755
}
808756

809757
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: 0 additions & 144 deletions
Original file line numberDiff line numberDiff line change
@@ -2745,150 +2745,6 @@ 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-
28922748
// P -> A 1.0.0
28932749
// -> B (no version)
28942750
// B is pinned to 1.0.0

0 commit comments

Comments
 (0)