workload repair can recover from corrupt workload sets (#52434)#52700
workload repair can recover from corrupt workload sets (#52434)#52700nagilson wants to merge 5 commits intodotnet:release/10.0.2xxfrom
workload repair can recover from corrupt workload sets (#52434)#52700Conversation
Co-authored-by: Marc Paine <marcpop@microsoft.com>
There was a problem hiding this comment.
Pull request overview
This PR is a backport of #52434 that fixes issue #52483, enabling workload repair to recover from corrupt workload sets. The issue occurs when package managers delete workload manifests from older SDK versions during updates, breaking the SDK's workload functionality.
Changes:
- Introduces a corruption detection and repair mechanism that checks for missing manifests before workload operations
- Adds a new
IWorkloadManifestCorruptionRepairerinterface and implementation that reinstalls missing manifests - Updates
SdkDirectoryWorkloadManifestProviderto check manifest health and throw helpful error messages - Ensures history recording ignores corruption to avoid failures when logging state
Reviewed changes
Copilot reviewed 42 out of 43 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| IWorkloadManifestCorruptionRepairer.cs | New interface for manifest corruption repair hook |
| WorkloadManifestCorruptionRepairer.cs | Implementation that detects and repairs missing manifests by reinstalling them |
| SdkDirectoryWorkloadManifestProvider.cs | Adds corruption detection, failure modes, and calls to repairer |
| IWorkloadManifestProvider.cs | New enum defining how to handle manifest corruption (Repair/Throw/Ignore) |
| WorkloadInstallerFactory.cs | Attaches corruption repairer when installer is created |
| WorkloadHistoryRecorder.cs | Sets failure mode to Ignore when recording history |
| MockWorkloadResolver.cs | Adds manifestProvider parameter support for tests |
| MockPackWorkloadInstaller.cs | Implements RepairWorkloads and writes manifest files to disk |
| CorruptWorkloadSetTestHelper.cs | New test helper that sets up corrupt workload scenarios |
| GivenDotnetWorkloadRepair.cs | New test validating repair of corrupt manifests |
| GivenDotnetWorkloadUpdate.cs | Updated tests with manifestProvider and new corruption repair test |
| SdkDirectoryWorkloadManifestProviderTests.cs | Updated exception types from FileNotFoundException to InvalidOperationException |
| Strings.resx and xlf files | New localized error message for missing manifests |
src/Cli/dotnet/Commands/Workload/WorkloadManifestCorruptionRepairer.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/Commands/Workload/WorkloadManifestCorruptionRepairer.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
MiYanni
left a comment
There was a problem hiding this comment.
Looks good, but this file doesn't exist in main. How do these changes flow forward?
| } | ||
|
|
||
| if (!provider?.HasMissingManifests(workloadSet) ?? true) | ||
| if (provider == null || !provider.HasMissingManifests(workloadSet)) |
There was a problem hiding this comment.
Is this actually changing functionality? Is it because the actual predicate should be: if (!(provider?.HasMissingManifests(workloadSet) ?? true))
There was a problem hiding this comment.
Good question, No, it's just making it more clear. The code from 10.0.1xx flowed into 10.0.2xx before this got merged, so this was supposed to have the whole change, this is just copilot's suggestion to make it more clear
There was a problem hiding this comment.
I'll just push this in main so we don't have to service a copilot edit
|
this isn't needed as it flowed, ill have copilot port this |
This ports the fixes from PR #52700 (backport of #52434) to main branch, including: - Workload repair can now recover from corrupt workload sets - Added WorkloadManifestCorruptionRepairer to detect and repair missing manifests - Updated WorkloadInstallerFactory to use the repairer - Added proper error messages for corrupt workload sets - Copilot improvements: clarified nullability check, removed extra blank lines, removed unused import Resolves workload corruption issue where manifests are removed by package managers. Co-authored-by: nagilson <23152278+nagilson@users.noreply.github.com>
A backport of #52434
resolve #52483