Skip to content

Commit

Permalink
Fix new resolver when using transitive pinning to resolve subgraphs c…
Browse files Browse the repository at this point in the history 
…orrectly (#6149)
  • Loading branch information
@nkolev92
nkolev92 authored Dec 16, 2024
1 parent f6f635e commit 7e6aa2e
Show file tree
Hide file tree
Showing 3 changed files with 1,571 additions and 1,010 deletions.
114 changes: 47 additions & 67 deletions src/NuGet.Core/NuGet.Commands/RestoreCommand/DependencyGraphResolver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ public async Task<ValueTuple<bool, List<RestoreTargetGraph>, RuntimeGraph>> Reso
IsDirectPackageReferenceFromRootProject = false,
};

LibraryRangeIndex[] rootedDependencyPath = new[] { rootProjectRefItem.LibraryRangeIndex };

_ = findLibraryEntryCache.GetOrAddAsync(
rootProjectRefItem.LibraryRangeIndex,
async static (state) =>
Expand Down Expand Up @@ -309,75 +311,47 @@ async static (state) =>
{
if (chosenRef.LibraryRange.TypeConstraintAllows(LibraryDependencyTarget.Package) && currentRef.LibraryRange.TypeConstraintAllows(LibraryDependencyTarget.Package))
{
bool isParentCentrallyPinned = false;

if (isCentralPackageTransitivePinningEnabled && importRefItem.Path.Length > 1)
if (chosenResolvedItem.Parents != null)
{
for (int pathIndex = importRefItem.Path.Length - 1; pathIndex > 0; pathIndex--)
{
LibraryRangeIndex parentLibraryRangeIndex = importRefItem.Path[pathIndex];
bool atLeastOneCommonAncestor = false;

if (findLibraryEntryCache.TryGetValue(parentLibraryRangeIndex, out Task<FindLibraryEntryResult>? parentCacheEntryTask))
foreach (LibraryRangeIndex parentRangeIndex in chosenResolvedItem.Parents.NoAllocEnumerate())
{
if (importRefItem.Path.Length > 2 && importRefItem.Path[importRefItem.Path.Length - 2] == parentRangeIndex)
{
FindLibraryEntryResult result = await parentCacheEntryTask;

if (chosenResolvedItems.TryGetValue(result.DependencyIndex, out var parentChosenResolvedItem))
{
isParentCentrallyPinned = parentChosenResolvedItem.IsCentrallyPinnedTransitivePackage;

if (isParentCentrallyPinned)
{
break;
}
}
atLeastOneCommonAncestor = true;
break;
}
}

if (atLeastOneCommonAncestor)
{
continue;
}
}

if (!isParentCentrallyPinned)
if (HasCommonAncestor(chosenResolvedItem.Path, importRefItem.Path))
{
if (chosenResolvedItem.Parents != null)
{
bool atLeastOneCommonAncestor = false;
continue;
}

foreach (LibraryRangeIndex parentRangeIndex in chosenResolvedItem.Parents.NoAllocEnumerate())
{
if (importRefItem.Path.Length > 2 && importRefItem.Path[importRefItem.Path.Length - 2] == parentRangeIndex)
{
atLeastOneCommonAncestor = true;
break;
}
}
if (chosenResolvedItem.ParentPathsThatHaveBeenEclipsed != null)
{
bool hasAlreadyBeenEclipsed = false;

if (atLeastOneCommonAncestor)
foreach (LibraryRangeIndex parentRangeIndex in chosenResolvedItem.ParentPathsThatHaveBeenEclipsed)
{
if (importRefItem.Path.Contains(parentRangeIndex))
{
continue;
hasAlreadyBeenEclipsed = true;
break;
}
}

if (HasCommonAncestor(chosenResolvedItem.Path, importRefItem.Path))
if (hasAlreadyBeenEclipsed)
{
continue;
}

if (chosenResolvedItem.ParentPathsThatHaveBeenEclipsed != null)
{
bool hasAlreadyBeenEclipsed = false;

foreach (LibraryRangeIndex parentRangeIndex in chosenResolvedItem.ParentPathsThatHaveBeenEclipsed)
{
if (importRefItem.Path.Contains(parentRangeIndex))
{
hasAlreadyBeenEclipsed = true;
break;
}
}

if (hasAlreadyBeenEclipsed)
{
continue;
}
}
}
}

Expand Down Expand Up @@ -462,7 +436,7 @@ async static (state) =>
{
LibraryDependency = currentRef,
LibraryRangeIndex = currentRefRangeIndex,
Parents = isCentrallyPinnedTransitivePackage ? new HashSet<LibraryRangeIndex>() { pathToCurrentRef[pathToCurrentRef.Length - 1] } : null,
Parents = isCentrallyPinnedTransitivePackage && !directPackageReferenceFromRootProject ? new HashSet<LibraryRangeIndex>() { importRefItem.Parent } : null,
Path = pathToCurrentRef,
IsCentrallyPinnedTransitivePackage = isCentrallyPinnedTransitivePackage,
IsDirectPackageReferenceFromRootProject = directPackageReferenceFromRootProject,
Expand Down Expand Up @@ -509,10 +483,13 @@ async static (state) =>
chosenResolvedItem.Parents = new HashSet<LibraryRangeIndex>();
}

chosenResolvedItem.Parents?.Add(pathToCurrentRef[pathToCurrentRef.Length - 1]);
if (!chosenResolvedItem.IsDirectPackageReferenceFromRootProject)
{
chosenResolvedItem.Parents?.Add(importRefItem.Parent);
}

//If the one we already have chosen is pure, then we can skip this one. Processing it wont bring any new info
if (chosenSuppressions.Count == 1 && chosenSuppressions[0].Count == 0)
if (chosenSuppressions.Count == 1 && chosenSuppressions[0].Count == 0 && HasCommonAncestor(chosenResolvedItem.Path, pathToCurrentRef))
{
continue;
}
Expand Down Expand Up @@ -597,7 +574,7 @@ async static (state) =>
{
LibraryDependency = currentRef,
LibraryRangeIndex = currentRefRangeIndex,
Parents = isCentrallyPinnedTransitivePackage ? new HashSet<LibraryRangeIndex>() { pathToCurrentRef[pathToCurrentRef.Length - 1] } : null,
Parents = isCentrallyPinnedTransitivePackage && !directPackageReferenceFromRootProject ? new HashSet<LibraryRangeIndex>() { importRefItem.Parent } : null,
Path = pathToCurrentRef,
IsCentrallyPinnedTransitivePackage = isCentrallyPinnedTransitivePackage,
IsDirectPackageReferenceFromRootProject = directPackageReferenceFromRootProject,
Expand Down Expand Up @@ -658,15 +635,14 @@ async static (state) =>
bool isDirectPackageReferenceFromRootProject = (currentRefRangeIndex == rootProjectRefItem.LibraryRangeIndex) && isPackage;

bool isCentrallyPinnedTransitiveDependency = isCentralPackageTransitivePinningEnabled
&& !isDirectPackageReferenceFromRootProject
&& isPackage
&& pinnedPackageVersions?.TryGetValue(depIndex, out pinnedVersionRange) == true;

LibraryRangeIndex rangeIndex = LibraryRangeIndex.Invalid;

LibraryDependency actualLibraryDependency = dep;

if (isCentrallyPinnedTransitiveDependency)
if (isCentrallyPinnedTransitiveDependency && !isDirectPackageReferenceFromRootProject)
{
actualLibraryDependency = new LibraryDependency(dep)
{
Expand All @@ -687,7 +663,8 @@ async static (state) =>
LibraryDependency = actualLibraryDependency,
LibraryDependencyIndex = depIndex,
LibraryRangeIndex = rangeIndex,
Path = LibraryRangeInterningTable.CreatePathToRef(pathToCurrentRef, currentRefRangeIndex),
Path = isCentrallyPinnedTransitiveDependency || isDirectPackageReferenceFromRootProject ? rootedDependencyPath : LibraryRangeInterningTable.CreatePathToRef(pathToCurrentRef, currentRefRangeIndex),
Parent = currentRefRangeIndex,
Suppressions = suppressions,
IsDirectPackageReferenceFromRootProject = isDirectPackageReferenceFromRootProject,
IsCentrallyPinnedTransitivePackage = isCentrallyPinnedTransitiveDependency
Expand Down Expand Up @@ -759,6 +736,7 @@ async static (state) =>
LibraryDependencyIndex = findLibraryCachedAsyncResult.GetDependencyIndexForDependency(runtimeDependencyIndex),
LibraryRangeIndex = findLibraryCachedAsyncResult.GetRangeIndexForDependency(runtimeDependencyIndex),
Path = LibraryRangeInterningTable.CreatePathToRef(pathToCurrentRef, currentRefRangeIndex),
Parent = currentRefRangeIndex,
Suppressions = suppressions,
IsDirectPackageReferenceFromRootProject = false,
};
Expand Down Expand Up @@ -935,19 +913,19 @@ async static (state) =>
{
foreach (var parent in chosenItem.Parents)
{
if (foundItem.Path.Contains(parent))
if (foundItem.Path.Contains(parent) && !foundItem.IsDirectPackageReferenceFromRootProject)
{
downgrades.Add(chosenItemRangeIndex, (foundItem.LibraryRangeIndex, dep, parent, chosenItem.LibraryDependency, false));
downgrades.Add(chosenItemRangeIndex, (foundItem.LibraryRangeIndex, dep, parent, chosenItem.LibraryDependency, isCentralPackageTransitivePinningEnabled ? chosenItem.IsCentrallyPinnedTransitivePackage : false));

foundParentDowngrade = true;
break;
}
}
}

if (!foundParentDowngrade)
if (!foundParentDowngrade && (!isCentralPackageTransitivePinningEnabled || !chosenItem.IsDirectPackageReferenceFromRootProject))
{
downgrades.Add(chosenItemRangeIndex, (foundItem.LibraryRangeIndex, dep, chosenItem.Path[chosenItem.Path.Length - 1], chosenItem.LibraryDependency, false));
downgrades.Add(chosenItemRangeIndex, (foundItem.LibraryRangeIndex, dep, chosenItem.Path[chosenItem.Path.Length - 1], chosenItem.LibraryDependency, isCentralPackageTransitivePinningEnabled ? chosenItem.IsCentrallyPinnedTransitivePackage : false));
}
}

Expand Down Expand Up @@ -980,7 +958,7 @@ async static (state) =>
var newGraphNode = new GraphNode<RemoteResolveResult>(actualDep.LibraryRange);
newGraphNode.Item = findLibraryEntryResult.Item;

if (chosenItem.IsCentrallyPinnedTransitivePackage)
if (chosenItem.IsCentrallyPinnedTransitivePackage && !chosenItem.IsDirectPackageReferenceFromRootProject)
{
newGraphNode.Disposition = Disposition.Accepted;
newGraphNode.Item.IsCentralTransitive = true;
Expand All @@ -993,7 +971,7 @@ async static (state) =>
currentGraphNode.InnerNodes.Add(newGraphNode);
}

if (dep.SuppressParent != LibraryIncludeFlags.All && isCentralPackageTransitivePinningEnabled && !downgrades.ContainsKey(chosenItemRangeIndex) && !RemoteDependencyWalker.IsGreaterThanOrEqualTo(chosenItem.LibraryDependency.LibraryRange.VersionRange, dep.LibraryRange.VersionRange))
if (dep.SuppressParent != LibraryIncludeFlags.All && isCentralPackageTransitivePinningEnabled && !chosenItem.IsDirectPackageReferenceFromRootProject && !downgrades.ContainsKey(chosenItemRangeIndex) && !RemoteDependencyWalker.IsGreaterThanOrEqualTo(chosenItem.LibraryDependency.LibraryRange.VersionRange, dep.LibraryRange.VersionRange))
{
downgrades.Add(chosenItem.LibraryRangeIndex, (currentLibraryRangeIndex, dep, rootProjectRefItem.LibraryRangeIndex, chosenItem.LibraryDependency, true));
}
Expand Down Expand Up @@ -1081,7 +1059,7 @@ async static (state) =>
{
IsCentralTransitive = downgrade.Value.IsCentralTransitive
},
OuterNode = toNode,
OuterNode = downgrade.Value.IsCentralTransitive ? rootGraphNode : toNode,
}
});
}
Expand All @@ -1093,7 +1071,7 @@ async static (state) =>
{
ResolvedDependencyGraphItem chosenResolvedItem = item.Value;

if (!chosenResolvedItem.IsCentrallyPinnedTransitivePackage || chosenResolvedItem.Parents == null || chosenResolvedItem.Parents.Count == 0)
if (!chosenResolvedItem.IsCentrallyPinnedTransitivePackage || chosenResolvedItem.IsDirectPackageReferenceFromRootProject || chosenResolvedItem.Parents == null || chosenResolvedItem.Parents.Count == 0)
{
continue;
}
Expand Down Expand Up @@ -1386,6 +1364,8 @@ private class DependencyGraphItem

public LibraryRangeIndex[] Path { get; set; } = Array.Empty<LibraryRangeIndex>();

public LibraryRangeIndex Parent { get; set; }

public HashSet<LibraryDependencyIndex>? Suppressions { get; set; }
}

Expand Down
2 changes: 1 addition & 1 deletion test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreNETCoreTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9948,7 +9948,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async(
var assetFileReader = new LockFileFormat();
var assetsFile = assetFileReader.Read(projectA.AssetsFileOutputPath);

var expectedLibraries = new List<string>() { "B.1.0.0", "C.2.0.0", "F.1.0.0", "G.1.0.0", "H.3.0.0", "O.3.0.0", "P.3.0.0", "S.3.0.0", "SS.3.0.0", "U.1.0.0", "V.1.0.0", "X.1.0.0", "Y.3.0.0" };
var expectedLibraries = new List<string>() { "B.1.0.0", "C.2.0.0", "F.1.0.0", "G.1.0.0", "H.3.0.0", "O.3.0.0", "P.3.0.0", "S.3.0.0", "SS.3.0.0", "U.1.0.0", "V.3.0.0", "X.1.0.0", "Y.3.0.0" };
var libraries = assetsFile.Libraries.Select(l => $"{l.Name}.{l.Version}").OrderBy(n => n).ToList();
Assert.Equal(expectedLibraries, libraries);

Expand Down
Loading

0 comments on commit 7e6aa2e

Please sign in to comment.