-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Introduce Workload Clean
: Garbage Collection Component
#30266
Conversation
src/Cli/dotnet/commands/dotnet-workload/install/NetSdkMsiInstallerClient.cs
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-workload/clean/WorkloadCleanCommand.cs
Outdated
Show resolved
Hide resolved
Workload Clean
: Garbage Collection Component
…s? confirm.) So that it doesnt show up in list. Will not impact existing packs deleted before this commit
src/Tests/dotnet-workload-install.Tests/GivenFileBasedWorkloadInstall.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Daniel Plaisted <[email protected]>
…l feature bands instead of <=, and the error message.
…t does every band. Because we have v1 in the folder we can change the architecture based on that, instead of worrying about a future breaking change
…t clean and clean --all do
@dsplaisted @joeloff I have responded to all feedback, including making it work for > feature bands. If we would like to revert that part, it is a simple commit revert. Thank you. |
src/Cli/dotnet/commands/dotnet-workload/install/NetSdkMsiInstallerClient.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-workload/clean/LocalizableStrings.resx
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-workload/clean/WorkloadCleanCommand.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-workload/install/FileBasedInstaller.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-workload/install/FileBasedInstaller.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-workload/install/FileBasedInstaller.cs
Outdated
Show resolved
Hide resolved
…lean. Include dotnetDir with newer info changes.
… with an if check instead of in the interface as it would be more confusing to have an msi api in a file based interface
@dsplaisted I have responded to all of the feedback, thanks for taking a look. I know you still need to look @ the tests. |
src/Tests/dotnet.Tests/dotnet-workload-clean/GivenDotnetWorkloadClean.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
src/Cli/dotnet/commands/dotnet-workload/clean/WorkloadCleanCommand.cs
Outdated
Show resolved
Hide resolved
src/Tests/dotnet.Tests/dotnet-workload-clean/GivenDotnetWorkloadClean.cs
Outdated
Show resolved
Hide resolved
src/Tests/dotnet.Tests/dotnet-workload-clean/GivenDotnetWorkloadClean.cs
Outdated
Show resolved
Hide resolved
src/Tests/dotnet.Tests/dotnet-workload-clean/GivenDotnetWorkloadClean.cs
Outdated
Show resolved
Hide resolved
…hat allowed null ref exception
@marcpopMSFT pls merge on red, arcade outage seems to be blocking this pr |
Do we have a link to a known outage we can ping them on? |
I believe it's fixed now, so no action needed :) |
What's this?
Resolves #19603
Doc: #30480
Adds PART ONE of
dotnet workload clean
:(So not the stuff in
2)
shown below.)Make
workload list
show all VS workloads regardless of feature bands.Workload --info now correctly shows
msi
orfilebased
install type.Fixes Incorrect customer facing logging messages.
Refactor MSI Garbage Collection into separate functions to make it easier to use
Note that workloads will still appear after
workload clean
-->workload list
, but not forworkload clean --all
, because we don't remove the workload installation record itself in GC. The old GC also does not remove workload installation records for workloads of uninstalled featurebands. To do this, we would need the manifest or some sort of history ledger to map N-1 packs to a workload id. Or maybe we could include theworkload id
in the workload pack records, but not sure if we want to do that.Evidence of VS Workload Behavior:

Evidence of MSI Clean up:



Shows that MSI clean up works for older feature bands:
(Note that I added this into my own copy of
main
so it won't go into prod.)Should
2) Restore Manifests to 'fresh' state, to whatever baseline manifests were initially shipped with the current SDK feature band, however, including edits that VS made to said manifest, assuming a ledger is available. (Daniel is working on the ledger.)Add
dotnet workload clean --all
:Should
Note that this is different from running
dotnet workload uninstall
en masse. Workload uninstall needs the manifest to know what has been orphaned.2) Restore manifests to 'fresh' state, like above, but for all lower SDK feature bands if possible. (They may not have a ledger available.)Not in this PR:
Make
workload list
show all workloads regardless of feature band.