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

Cleanup GPF while using a PackageSourceMapping when a failed Restore brought in the package #12850

Open
donnie-msft opened this issue Aug 26, 2023 · 3 comments
Assignees
Labels
Area:PackageSourceMapping Issues related to the package source mapping feature Functionality:Restore Functionality:VisualStudioUI Package Manager UI et al Priority:2 Issues for the current backlog. Product:VS.Client Type:DCR Design Change Request

Comments

@donnie-msft
Copy link
Contributor

NuGet Product(s) Affected

Visual Studio Package Management UI

Current Behavior

When installing a package with the PM UI, if a package is brought into the GPF for the first time, but restore ultimately fails, the downloaded package remains in the GPF.

Desired Behavior

Cleanup the GPF in scenarios where restore fails.

Additional Context

No response

@donnie-msft donnie-msft added Product:VS.Client Type:DCR Design Change Request Functionality:VisualStudioUI Package Manager UI et al Functionality:Restore Priority:2 Issues for the current backlog. Area:PackageSourceMapping Issues related to the package source mapping feature labels Aug 26, 2023
@zivkan
Copy link
Member

zivkan commented Aug 26, 2023

I'm not sure this is a good idea. If the next restore needs the same package, NuGet will have to extract the package again if it's deleted after the failed restore. This is a waste of CPU & disk IO. If a future restore of the same package happens more than 30 minutes after the first time, it will have to re-download again.

If the goal is to avoid a huge GPF, I think #4980 is a better solution.

@donnie-msft
Copy link
Contributor Author

This isn't the same problem as GPF growing indefinitely.

I created this issue after a conversation with @nkolev92 .

Here, the consideration is package source mapping. We are moving towards automatically source mapping packages during installs. The GPF's 'source' becomes a "source of truth" when analyzing whether a package can be mapped or not.

We wouldn't want an invalid state of the GPF after a failed restore, because we will be relying on that state in source mapping logic going forward.

I'll take a stab at explaining the why. Nikolche can probably articulate this better:
If a restore grabs the package from sourceA, fails for some other reason, then we now have an ambiguous sourceA in the 'nupkg.metadata' which may not want automatic source mappings to pickup and use.

@nkolev92 nkolev92 changed the title Cleanup GPF when a failed Restore brought in the package Cleanup GPF while using a PackageSourceMapping when a failed Restore brought in the package Sep 2, 2023
@nkolev92
Copy link
Member

nkolev92 commented Sep 2, 2023

If a restore grabs the package from sourceA, fails for some other reason, then we now have an ambiguous sourceA in the 'nupkg.metadata' which may not want automatic source mappings to pickup and use.

Yep, that's perfect.

In a practical example:

Say you "consider" installing package A from source X. You decide not for whatever reason.
At a later point, you install package B from source Y. B has a dependency on A. Because of the first installation, A will be seen and mapped to X. This solves the "ambiguity" problem, so it's not really a violation of the PSM promises.

However, is it what the user would've wanted? A package that they never used?

It's tricky, I'm not convinced it's a must have, but it's a tricky scenario and we should consider it.

@donnie-msft donnie-msft self-assigned this Sep 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:PackageSourceMapping Issues related to the package source mapping feature Functionality:Restore Functionality:VisualStudioUI Package Manager UI et al Priority:2 Issues for the current backlog. Product:VS.Client Type:DCR Design Change Request
Projects
None yet
Development

No branches or pull requests

3 participants