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

Switch away from deprecated NuGet API #716

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jimmylewis
Copy link
Contributor

The IVsPackageInstallerServices interface is marked deprecated for getting installed package info, due to threading problems within NuGet. Switched to the new interface instead.


public static Guid GetProjectGuid(Project project)
{
string uniqueName = project.UniqueName;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we do some sort of return result checking in any of these? I'd assume there are HRs returned and object could be checked for null.

@jimmylewis
Copy link
Contributor Author

jimmylewis commented Sep 19, 2023

While doing some further testing with this, it seems really flaky. Now that the work of detecting package installation is async, it seems that the OleMenuCommand is not waiting for it to finish - it will use stale text for the menu command. I'm seeing this in the debugger where the context menu appears between stepping through these lines:

button.Visible = true;
button.Enabled = KnownUIContexts.SolutionExistsAndNotBuildingAndNotDebuggingContext.IsActive;
_isPackageInstalled = await IsPackageInstalledAsync(projectGuid);
// context menu is shown before the following lines are executed:
if (_isPackageInstalled)
{
    button.Text = Resources.Text.DisableRestoreOnBuild;
}
else
{
    button.Text = Resources.Text.EnableRestoreOnBuild;
}

I also tried moving the button.Visible = true to after the await, and the command was never visible again. So I'm guessing that the QueryStatus has a timeout, and this new operation is taking too long; it's getting the first half of the changes (button.Visible) but not the second (button.Text).

I tried caching the text to use, thinking I could make up for it, but that revealed that the NuGet API is actually pretty laggy. After executing an install/uninstall operation, it takes several seconds for it to return an updated result (which different from the old implementation, which seemed to update immediately). I'll need to follow up with someone from NuGet to understand why this API seems much flakier.

@jimmylewis jimmylewis marked this pull request as draft January 16, 2024 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants