Repository navigation
Decouple ProjectJsonToPackageReferenceMigrator and VSSolutionManager during solution load, to fix the migrator infinite loop project fails migrating or the project is only project in the solution - #6794
Merged
Conversation
nkolev92
marked this pull request as ready for review
September 23, 2025 00:59
nkolev92
changed the title
Member
Author
|
fwiw, NuGet/Home#12044 is probably something we can hit when we have project.json nuget projects even during successful migration. Easy to check, put a breakpoint in VSSolutionManager.EnsureInitializeAsync and see us subscribe to the same events twice. |
donnie-msft
previously approved these changes
Sep 23, 2025
zivkan
reviewed
Sep 23, 2025
nkolev92
enabled auto-merge (squash)
September 23, 2025 17:28
donnie-msft
approved these changes
Sep 23, 2025
This was referenced Nov 6, 2025
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
3 participants
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.
Bug
Fixes: NuGet/Home#14553
Description
VSSolutionManager requires that it's initialized before calls on it can be made.
EnsureNuGetAndVsProjectAdapterCacheAsync for example adds all projects there.
That same method calls the migrator whenever needed.
There are 2 places, but 2 method calls:
GetVsProjectAdapterAsync
GetNuGetProjectAsync
ReloadProjectAsync which calls GetVsProjectAdapterAsync.
GetVsProjectAdapterAsync and GetNuGetProjectAsync are VSSolutionManager methods and as you might've guessed, they call EnsureInitializeAsync which calls EnsureNuGetAndVsProjectAdapterCacheAsync which is where the loop occurs.
You quickly get how this becomes a problem.
This is because the migrator was written for a fully loaded solution in mind.
Fortunately, when we call the migrator at solution load, we actually have project adapter and NuGetProject already, so we can reuse though, so I simply added a method that'll be called during initialization instead of the generic one that'd rely on ISolutionManager.
This is a quick fix that gets rid of that dependency.
The last part is ReloadProjectAsync. I needed to do some history investigation there, the idea being it's something that we'd call whenever a project was partially migrated, like say we write 1 PackageReference and not another.
Unfortunately that doesn't work. Reloading would require doing some other clean-up that frankly should probably be done within the migrator logic itself.
This method has been there for a while though, and failures to migrate projects like this aren't really happening. It's something that requires project-system APIs to fail and these are APIs commonly used for legacy PR projects, so it's very unlikely. Either way, this current implementaiton wasn't helping and given that the migration rarely fails, I've just deleted that method to remove the loop.
This isn't the ideal solution.
Ideally we'd avoid this dance but that's probably a bigger refactoring not worth attempting with a few days left.
PR Checklist
Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.Details