Skip to content

Commit

Permalink
Adjust logic to work with multiple levels of transitivity
Browse files Browse the repository at this point in the history
  • Loading branch information
@jeffkl
jeffkl committed Jun 27, 2022
1 parent 997350c commit 6ceaad5
Show file tree
Hide file tree
Showing 2 changed files with 147 additions and 17 deletions.
23 changes: 17 additions & 6 deletions src/NuGet.Core/NuGet.Commands/RestoreCommand/LockFileBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ private IEnumerable<LibraryDependency> GetLibraryDependenciesForCentralTransitiv
if (centralPackageTransitivePinningEnabled)
{
// Centrally pinned dependencies are not directly declared but the PrivateAssets from the top-level dependency that pulled it in should apply to it also
foreach (GraphNode<RemoteResolveResult> parentNode in FlattenParentNodes(node))
foreach (GraphNode<RemoteResolveResult> parentNode in EnumerateParentNodes(node))
{
LibraryDependency parentDependency = targetFrameworkInformation.Dependencies.FirstOrDefault(i => i.Name.Equals(parentNode.Item.Key.Name, StringComparison.OrdinalIgnoreCase));

Expand Down Expand Up @@ -558,22 +558,33 @@ private IEnumerable<LibraryDependency> GetLibraryDependenciesForCentralTransitiv
}

/// <summary>
/// Returns a flattened list of all parent nodes for the specified node.
/// Enumerates all parent nodes of the specified node.
/// </summary>
/// <typeparam name="T">The type of the node.</typeparam>
/// <param name="graphNode">The <see cref="GraphNode{TItem}" /> to flatten the parent nodes of.</param>
/// <returns>A top down flattened list of parent nodes of the specied node.</returns>
private static IEnumerable<GraphNode<T>> FlattenParentNodes<T>(GraphNode<T> graphNode)
/// <param name="graphNode">The <see cref="GraphNode{TItem}" /> to enumerate the parent nodes of.</param>
/// <returns>An <see cref="IEnumerable{T}" /> containing a top down list of parent nodes of the specied node.</returns>
private static IEnumerable<GraphNode<T>> EnumerateParentNodes<T>(GraphNode<T> graphNode)
{
foreach (GraphNode<T> item in graphNode.ParentNodes)
{
if (item.ParentNodes.Any())
{
foreach (GraphNode<T> parentNode in FlattenParentNodes(item))
// Transitive pinned nodes have ParentNodes set
foreach (GraphNode<T> parentNode in EnumerateParentNodes(item))
{
yield return parentNode;
}
}
else if (item.OuterNode != null)
{
// Normal transitive nodes use OuterNode to track their parent
foreach (GraphNode<T> outerNode in EnumerateParentNodes(item.OuterNode))
{
yield return outerNode;
}

yield return item.OuterNode;
}

yield return item;
}
Expand Down
141 changes: 130 additions & 11 deletions test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2184,13 +2184,11 @@ public async Task RestoreCommand_CentralVersion_AssetsFile_VerifyProjectsReferen
/// </summary>
/// <returns></returns>
[Fact]
public async Task RestoreCommand_CentralVersion_AssetsFile_PrivateAssetsFlowsToPinnedDependencies()
public async Task RestoreCommand_CentralVersion_AssetsFile_PrivateAssetsFlowsToPinnedDependenciesWithTopLevelDependency()
{
// Arrange
var framework = new NuGetFramework("net46");

var privateAssets = LibraryIncludeFlags.Runtime | LibraryIncludeFlags.Compile;

using (var pathContext = new SimpleTestPathContext())
{
var projectInfo = new
Expand All @@ -2202,13 +2200,19 @@ public async Task RestoreCommand_CentralVersion_AssetsFile_PrivateAssetsFlowsToP

var logger = new TestLogger();

// PackageA 1.0.0 -> PackageB 1.0.0 -> PackageC 1.0.0
// PackageA 1.0.0 -> PackageB 1.0.0 -> PackageC 1.0.0 -> PackageD 1.0.0
var packageA = new PackageIdentity("PackageA", new NuGetVersion("1.0.0"));
var packageB = new PackageIdentity("PackageB", new NuGetVersion("1.0.0"));
var packageC = new PackageIdentity("PackageC", new NuGetVersion("1.0.0"));
var packageD = new PackageIdentity("PackageD", new NuGetVersion("1.0.0"));

var packageDContext = new SimpleTestPackageContext(packageD);
packageDContext.AddFile("lib/net46/PackageD.dll");
await SimpleTestPackageUtility.CreateFullPackageAsync(pathContext.PackageSource, packageDContext);

var packageCContext = new SimpleTestPackageContext(packageC);
packageCContext.AddFile("lib/net46/PackageC.dll");
packageCContext.Dependencies.Add(packageDContext);
await SimpleTestPackageUtility.CreateFullPackageAsync(pathContext.PackageSource, packageCContext);

var packageBContext = new SimpleTestPackageContext(packageB);
Expand All @@ -2232,14 +2236,24 @@ public async Task RestoreCommand_CentralVersion_AssetsFile_PrivateAssetsFlowsToP
TypeConstraint = LibraryDependencyTarget.Package,
},
VersionCentrallyManaged = true,
SuppressParent = privateAssets
SuppressParent = LibraryIncludeFlags.Runtime | LibraryIncludeFlags.Compile
},
new LibraryDependency()
{
LibraryRange = new LibraryRange()
{
Name = packageB.Id,
TypeConstraint = LibraryDependencyTarget.Package,
},
VersionCentrallyManaged = true,
SuppressParent = LibraryIncludeFlags.Build
}
},
new List<CentralPackageVersion>
{
new CentralPackageVersion(packageA.Id, new VersionRange(packageA.Version)),
new CentralPackageVersion(packageB.Id, new VersionRange(packageB.Version)),
new CentralPackageVersion(packageC.Id, new VersionRange(packageC.Version))
new CentralPackageVersion(packageD.Id, new VersionRange(packageD.Version))
},
framework);

Expand All @@ -2263,16 +2277,121 @@ public async Task RestoreCommand_CentralVersion_AssetsFile_PrivateAssetsFlowsToP

// Assert
Assert.True(result.Success);
Assert.Equal(lockFile.CentralTransitiveDependencyGroups.Count, 1);
Assert.Equal(1, lockFile.CentralTransitiveDependencyGroups.Count);

List<LibraryDependency> transitiveDependencies = lockFile.CentralTransitiveDependencyGroups.First().TransitiveDependencies.ToList();

Assert.Equal(transitiveDependencies.Count, 2);
// Only PackageD should be in the transitive pinned items
Assert.Equal(1, transitiveDependencies.Count);

LibraryDependency transitiveDependency = transitiveDependencies.First();

Assert.Equal(packageD.Id, transitiveDependency.Name);
Assert.Equal(LibraryIncludeFlags.Build, transitiveDependency.SuppressParent);
}
}

foreach (LibraryDependency transitiveDependency in transitiveDependencies)
/// <summary>
/// Verifies that when a transitive package version is pinned, the PrivateAssets flow from the package that pulled it into the graph to the pinned dependency.
/// </summary>
/// <returns></returns>
[Fact]
public async Task RestoreCommand_CentralVersion_AssetsFile_PrivateAssetsFlowsToPinnedDependencies()
{
// Arrange
var framework = new NuGetFramework("net46");

using (var pathContext = new SimpleTestPathContext())
{
var projectInfo = new
{
Assert.Equal(LibraryIncludeFlags.Runtime | LibraryIncludeFlags.Compile, transitiveDependency.SuppressParent);
}
Name = "ProjectA",
Directory = Directory.CreateDirectory(Path.Combine(pathContext.SolutionRoot, "ProjectA")),
Path = new FileInfo(Path.Combine(pathContext.SolutionRoot, "ProjectA", "Project1.csproj")).FullName
};

var logger = new TestLogger();

// PackageA 1.0.0 -> PackageB 1.0.0 -> PackageC 1.0.0 -> PackageD 1.0.0
var packageA = new PackageIdentity("PackageA", new NuGetVersion("1.0.0"));
var packageB = new PackageIdentity("PackageB", new NuGetVersion("1.0.0"));
var packageC = new PackageIdentity("PackageC", new NuGetVersion("1.0.0"));
var packageD = new PackageIdentity("PackageD", new NuGetVersion("1.0.0"));

var packageDContext = new SimpleTestPackageContext(packageD);
packageDContext.AddFile("lib/net46/PackageD.dll");
await SimpleTestPackageUtility.CreateFullPackageAsync(pathContext.PackageSource, packageDContext);

var packageCContext = new SimpleTestPackageContext(packageC);
packageCContext.AddFile("lib/net46/PackageC.dll");
packageCContext.Dependencies.Add(packageDContext);
await SimpleTestPackageUtility.CreateFullPackageAsync(pathContext.PackageSource, packageCContext);

var packageBContext = new SimpleTestPackageContext(packageB);
packageBContext.AddFile("lib/net46/PackageB.dll");
packageBContext.Dependencies.Add(packageCContext);
await SimpleTestPackageUtility.CreateFullPackageAsync(pathContext.PackageSource, packageBContext);

var packageAContext = new SimpleTestPackageContext(packageA);
packageAContext.AddFile("lib/net46/PackageA.dll");
packageAContext.Dependencies.Add(packageBContext);
await SimpleTestPackageUtility.CreateFullPackageAsync(pathContext.PackageSource, packageAContext);

var tfi = CreateTargetFrameworkInformation(
new List<LibraryDependency>
{
new LibraryDependency()
{
LibraryRange = new LibraryRange()
{
Name = packageA.Id,
TypeConstraint = LibraryDependencyTarget.Package,
},
VersionCentrallyManaged = true,
SuppressParent = LibraryIncludeFlags.Runtime | LibraryIncludeFlags.Compile
},
},
new List<CentralPackageVersion>
{
new CentralPackageVersion(packageA.Id, new VersionRange(packageA.Version)),
new CentralPackageVersion(packageB.Id, new VersionRange(packageB.Version)),
new CentralPackageVersion(packageD.Id, new VersionRange(packageD.Version))
},
framework);

PackageSpec packageSpec = CreatePackageSpec(new List<TargetFrameworkInformation>() { tfi }, framework, projectInfo.Name, projectInfo.Path, centralPackageManagementEnabled: true, centralPackageTransitivePinningEnabled: true);


var dgspec = new DependencyGraphSpec();
dgspec.AddProject(packageSpec);

var request = new TestRestoreRequest(dgspec.GetProjectSpec(projectInfo.Name), new[] { new PackageSource(pathContext.PackageSource) }, pathContext.PackagesV2, logger)
{
LockFilePath = Path.Combine(projectInfo.Directory.FullName, "project.assets.json"),
ProjectStyle = ProjectStyle.PackageReference
};

var restoreCommand = new RestoreCommand(request);
var result = await restoreCommand.ExecuteAsync();
var lockFile = result.LockFile;

var targetLib = lockFile.Targets.First().Libraries.Where(l => l.Name == packageA.Id).FirstOrDefault();

// Assert
Assert.True(result.Success);
Assert.Equal(1, lockFile.CentralTransitiveDependencyGroups.Count);

List<LibraryDependency> transitiveDependencies = lockFile.CentralTransitiveDependencyGroups.First().TransitiveDependencies.ToList();

Assert.Equal(2, transitiveDependencies.Count);

LibraryDependency transitiveDependencyB = transitiveDependencies.Single(i => i.Name.Equals(packageB.Id));

Assert.Equal(LibraryIncludeFlags.Runtime | LibraryIncludeFlags.Compile, transitiveDependencyB.SuppressParent);

LibraryDependency transitiveDependencyD = transitiveDependencies.Single(i => i.Name.Equals(packageD.Id));

Assert.Equal(LibraryIncludeFlags.Runtime | LibraryIncludeFlags.Compile, transitiveDependencyD.SuppressParent);
}
}

Expand Down

0 comments on commit 6ceaad5

Please sign in to comment.