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

don't use transitive dependency version as top level version #11151

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

brettfo
Copy link
Contributor

@brettfo brettfo commented Dec 18, 2024

When trying to update a top level dependency instead of pinning a transitive dependency, don't report the transitive version as a possible solution for the top level package.

This required an update to a test where the minimum version of a dependency doesn't exist. To get around this, the test was updated to use faked local packages with the correct versions. Issue #11150 was filed for the future work.

// If the current version of the parent is less than the current version of the dependency
else if (CurrentVersion < currentVersionDependency)
{
return currentVersionDependency;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the real fix.

@@ -3075,14 +3075,14 @@ await TestUpdateForProject("Some.Package", "12.0.1", "13.0.1",
public async Task UpdatingTransitiveDependencyWithNewSolverCanUpdateJustTheTopLevelPackage()
{
// we've been asked to explicitly update a transitive dependency, but we can solve it by updating the top-level package instead
await TestUpdateForProject("Transitive.Package", "1.0.0", "2.0.0",
await TestUpdateForProject("Transitive.Package", "7.0.0", "8.0.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicitly updated this test to cover the broken scenario.

[InlineData(false)]
public async Task DependencyConflictsCanBeResolvedNewSharingDependency(bool useExistingSdks)
[Fact]
public async Task DependencyConflictsCanBeResolvedNewSharingDependency()
Copy link
Contributor Author

@brettfo brettfo Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the fix in this PR, this test started failing for an unrelated reason. Issue #11150 was filed and this test was made to follow a more common pattern to unblock the fix.

@brettfo brettfo marked this pull request as ready for review December 19, 2024 00:00
@brettfo brettfo requested a review from a team as a code owner December 19, 2024 00:00
@brettfo brettfo force-pushed the dev/brettfo/nuget-solve-transitive branch from 22e29b1 to 8d82ef8 Compare December 19, 2024 17:35
@randhircs randhircs merged commit 3eb3066 into main Dec 19, 2024
70 checks passed
@randhircs randhircs deleted the dev/brettfo/nuget-solve-transitive branch December 19, 2024 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: dotnet:nuget NuGet packages via nuget or dotnet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants