NuGet: Remove redundant GetPackageGraphForDependencies and use discovery DependencyGraph#15122
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the NuGet updater’s update-operation attribution logic by removing the redundant GetPackageGraphForDependencies (temp project + dotnet restore + project.assets.json parsing) and instead deriving parent/version lookup data from the already-discovered in-memory DependencyGraph, making ComputeUpdateOperations synchronous and I/O-free.
Changes:
- Replace restore-based package graph generation with
BuildReverseGraph(dependencyGraph)inPackageReferenceUpdater. - Update
FileWriterWorkerto passfinalProjectDiscovery.DependencyGraphintoComputeUpdateOperations. - Rewrite/update tests to unit-test
BuildReverseGraphand to validateDependencyGraphdiscovery across compatible TFMs.
Show a summary per file
| File | Description |
|---|---|
| nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackageReferenceUpdater.cs | Removes restore-based graph generation and builds reverse-lookup structures from discovery DependencyGraph. |
| nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/FileWriters/FileWriterWorker.cs | Adjusts call site for new synchronous ComputeUpdateOperations signature and supplies DependencyGraph. |
| nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/PackageReferenceUpdaterTests.cs | Replaces integration-style graph test with a unit test for BuildReverseGraph and updates operation tests to inject a graph. |
| nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTests.cs | Adds coverage ensuring discovery populates DependencyGraph when project/package TFMs differ but remain compatible. |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 4
868a7fd to
65c7894
Compare
VXianOng
approved these changes
May 22, 2026
…endencyGraph Replace the GetPackageGraphForDependencies method (which created a temp project, ran dotnet restore, and parsed project.assets.json) with BuildReverseGraph, a pure in-memory function that derives the same (PackageParents, PackageVersions) data from the discovery's existing DependencyGraph property. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…graph-derived versions The dependency graph can contain multiple versions of the same package name (from multi-TFM merging), making a name-keyed version lookup ambiguous. Instead, use the per-TFM resolvedVersions which are already available and unambiguous. Remove PackageVersions from BuildReverseGraph since it is no longer needed. Also adds a test exercising the multi-version-per-package scenario. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
298b274 to
bf7c50b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Now that project discovery generates a package graph, this PR removes duplicate code and instead uses the existing package graph to determine what update operations were performed. One package parent test was moved to be a discovery test since that's where the bulk of the work now happens.
Replace the
GetPackageGraphForDependenciesmethod (which created a temp project, randotnet restore, and parsedproject.assets.json) withBuildReverseGraph, a pure in-memory function that derives the same(PackageParents, PackageVersions)data from the discovery's existingDependencyGraphproperty.Changes
GetPackageGraphForDependenciesand replaced withBuildReverseGraphwhich converts the forward dependency graph (already available from project discovery) into the reverse parent/version lookup.ComputeUpdateOperationsis now synchronous and no longer performs I/O.finalProjectDiscovery.DependencyGraphdirectly.BuildReverseGraphand updatedComputeUpdateOperationstests to supply an explicit dependency graph.TestDependencyGraphWithDifferentTargetFrameworks) that verifies the dependency graph is correctly populated when a project targets a different-but-compatible TFM (e.g.,net10.0-android) from its packages.