Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect cycle between root project and transitive dependency in new dependency resolver #6247

Merged
merged 2 commits into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,16 @@ public sealed class DependencyGraphItemIndexer
/// </summary>
private LibraryRangeIndex _nextLibraryRangeIndex = LibraryRangeIndex.Project + 1;

/// <summary>
/// Initializes a new instance of the <see cref="DependencyGraphItemIndexer" /> class.
/// </summary>
/// <param name="libraryDependency">A <see cref="LibraryDependency" /> representing the root project.</param>
public DependencyGraphItemIndexer(LibraryDependency libraryDependency)
{
_libraryDependencyTable.TryAdd(libraryDependency.Name, LibraryDependencyIndex.Project);
_libraryRangeTable.TryAdd(libraryDependency.LibraryRange, LibraryRangeIndex.Project);
}

/// <summary>
/// Indexes a <see cref="LibraryDependency" /> and returns a <see cref="LibraryDependencyIndex" /> associated with the its name.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ internal sealed partial class DependencyGraphResolver
/// <summary>
/// A <see cref="DependencyGraphItemIndexer" /> used to index dependency graph items.
/// </summary>
private readonly DependencyGraphItemIndexer _indexingTable = new();
private readonly DependencyGraphItemIndexer _indexingTable;

/// <summary>
/// A <see cref="RestoreCollectorLogger" /> used for logging.
Expand All @@ -67,6 +67,11 @@ internal sealed partial class DependencyGraphResolver
/// </summary>
private readonly TelemetryActivity _telemetryActivity;

/// <summary>
/// Represents the root project as a <see cref="LibraryDependency" />.
/// </summary>
private readonly LibraryDependency _rootProjectLibraryDependency;

/// <summary>
/// Initializes a new instance of the <see cref="DependencyGraphResolver" /> class.
/// </summary>
Expand All @@ -78,6 +83,16 @@ public DependencyGraphResolver(RestoreCollectorLogger logger, RestoreRequest res
_logger = logger;
_request = restoreRequest;
_telemetryActivity = telemetryActivity;

_rootProjectLibraryDependency = new LibraryDependency(
new LibraryRange()
{
Name = _request.Project.Name,
VersionRange = new VersionRange(_request.Project.Version),
TypeConstraint = LibraryDependencyTarget.Project | LibraryDependencyTarget.ExternalProject
});

_indexingTable = new DependencyGraphItemIndexer(_rootProjectLibraryDependency);
}

/// <summary>
Expand Down Expand Up @@ -133,15 +148,6 @@ public async Task<ValueTuple<bool, List<RestoreTargetGraph>, RuntimeGraph>> Reso
// Stores the list of download results which contribute to the overall success of the graph resolution
DownloadDependencyResolutionResult[]? downloadDependencyResolutionResults = default;

// A LibraryDependency representing the root of the graph which is the project itself
LibraryDependency mainProjectDependency = new(
new LibraryRange()
{
Name = _request.Project.Name,
VersionRange = new VersionRange(_request.Project.Version),
TypeConstraint = LibraryDependencyTarget.Project | LibraryDependencyTarget.ExternalProject
});

// Create the list of framework/runtime pairs to resolve graphs for. This method returns the pairs in order with the target framework pairs without runtime identifiers come first
List<FrameworkRuntimePair> projectFrameworkRuntimePairs = RestoreCommand.CreateFrameworkRuntimePairs(_request.Project, runtimeIds: RequestRuntimeUtility.GetRestoreRuntimes(_request));

Expand Down Expand Up @@ -182,12 +188,12 @@ public async Task<ValueTuple<bool, List<RestoreTargetGraph>, RuntimeGraph>> Reso
// Create a DependencyGraphItem representing the root project to add to the queue for processing
DependencyGraphItem rootProjectDependencyGraphItem = new()
{
LibraryDependency = mainProjectDependency,
LibraryDependency = _rootProjectLibraryDependency,
LibraryDependencyIndex = LibraryDependencyIndex.Project,
LibraryRangeIndex = LibraryRangeIndex.Project,
Suppressions = new HashSet<LibraryDependencyIndex>(),
FindLibraryTask = ResolverUtility.FindLibraryCachedAsync(
mainProjectDependency.LibraryRange,
_rootProjectLibraryDependency.LibraryRange,
frameworkRuntimePair.Framework,
runtimeIdentifier: string.IsNullOrWhiteSpace(frameworkRuntimePair.RuntimeIdentifier) ? null : frameworkRuntimePair.RuntimeIdentifier,
context,
Expand Down Expand Up @@ -1289,6 +1295,12 @@ .. chosenResolvedItem.Suppressions
LibraryDependency childDependency = chosenResolvedItem.Item.Data.Dependencies[i];
LibraryDependencyIndex childLibraryDependencyIndex = chosenResolvedItem.GetDependencyIndexForDependencyAt(i);

if (childLibraryDependencyIndex == LibraryDependencyIndex.Project)
{
// Skip any dependency with the same name as the project
jeffkl marked this conversation as resolved.
Show resolved Hide resolved
continue;
}

HashSet<LibraryDependency>? runtimeDependencies = default;

// Evaluate the runtime dependencies if any
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,56 @@ public async Task RestoreCommand_HigherLevelSuppressionsWin_VerifiesEquivalency(
result.LockFile.Targets[0].Libraries[2].Name.Should().Be("X");
}

// LightGbm (project) -> Microsoft.ML.LightGbm -> LightGBM 2.0.0 (package)
// This should result in an error with the cycle
[Fact]
public async Task RestoreCommand_WithCycleContainingProject_FailsAndLogsMessage()
{
// Arrange
using var pathContext = new SimpleTestPathContext();
await SimpleTestPackageUtility.CreateFolderFeedV3Async(
pathContext.PackageSource,
PackageSaveMode.Defaultv3,
new SimpleTestPackageContext("Microsoft.ML.LightGbm", "4.0.1")
{
Dependencies =
[
new SimpleTestPackageContext("LightGBM", "3.3.5"),
]
});

PackageSpec projectSpec = ProjectTestHelpers.GetPackageSpecWithProjectNameAndSpec(
"LightGbm",
pathContext.SolutionRoot,
@"
{
""frameworks"": {
""net472"": {
""dependencies"": {
""Microsoft.ML.LightGbm"": ""4.0.1""
}
}
}
}");

// Act & Assert
jeffkl marked this conversation as resolved.
Show resolved Hide resolved
(var result, _) = await ValidateRestoreAlgorithmEquivalency(pathContext, projectSpec);

// Additional assert
jeffkl marked this conversation as resolved.
Show resolved Hide resolved
result.Success.Should().BeFalse();

result.LogMessages.Should().HaveCount(1);

IAssetsLogMessage logMessage = result.LogMessages.Single();
logMessage.Code.Should().Be(NuGetLogCode.NU1108);
logMessage.Message.Should().Contain("LightGbm -> Microsoft.ML.LightGbm 4.0.1 -> LightGBM (>= 3.3.5)");

result.LockFile.Targets.Should().HaveCount(1);
result.LockFile.Targets[0].Libraries.Should().HaveCount(1);
result.LockFile.Targets[0].Libraries[0].Name.Should().Be("Microsoft.ML.LightGbm");
result.LockFile.Targets[0].Libraries[0].Version.Should().Be(new NuGetVersion("4.0.1"));
}

// Project 1 -> a 1.0.0 -> b 1.0.0
// -> b 2.0.0
// Does not install b 1.0.0 since it is eclipsed by a direct package reference
Expand Down
Loading