Permit loose manifest priority

[Forgind]

Description

If the user opts into using workload sets, installs a workload set, then tries to switch back to using loose manifests, we correctly permit installing or updating to loose manifests, but we do not garbage collect the workload sets, both because garbage collection isn't yet implemented for workload sets and because we would keep the latest workload set regardless. That workload set is still findable, therefore, and when the user tries dotnet workload --version, dotnet workload update --print-rollback, or anything else that uses the resolver to find workloads, it uses the workload set instead of correctly switching back to loose manifests.

Customer Impact

Customers who ever use workload sets can never use loose manifests again unless they follow manual steps to delete their workload set(s).

Regression

No; the resolver always worked this way, but workload sets didn't exist, so it wasn't an issue.

Risk

Low. Relatively few people are using workload sets, and this is changing behavior to align with what the user explicitly requested (or got by using a rollback file), so it changes behavior to align with user expectations. If they'd switched to loose manifests by accident (somehow?), they can always switch back very easily.

Testing

Changed automated tests to account for the change and walked through one in a debugger to make sure it did what I'd expect.

[dsplaisted]

Initially I didn't fully understand your change, so I made a version of the change myself: dsplaisted@3c54769

On looking at your change a second time, I think it generally works (I had missed the early return statement). However it looks like your change may not respect global.json or a workload set version passed in via the constructor, both of which it should do.

Since this is a change to the resolver which goes into full VS, I think we should consider taking this change for 8.0.4xx if we can. @marcpopMSFT

[marcpopMSFT]

So the primary change is adding installState.UseWorkloadSets == true to the check, correct? That seems pretty safe and isolated to add. Any concerns with taking this for 8.0.4xx?

[Forgind]

So the primary change is adding installState.UseWorkloadSets == true to the check, correct? That seems pretty safe and isolated to add. Any concerns with taking this for 8.0.4xx?

Not from me. @dsplaisted?

If it's agreed to, I'll rebase on 8.0.4xx.

[dsplaisted]

It would be good to have some tests for this, be they functional and/or VM-based MSI tests.

[Forgind]

/backport to main

[github-actions]

Started backporting to main: https://github.com/dotnet/sdk/actions/runs/10063639987

[github-actions]

@Forgind backporting to main failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Permit loose manifest priority
Using index info to reconstruct a base tree...
M	src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs
A	src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs
Falling back to patching base and 3-way merge...
Auto-merging test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs
Auto-merging src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs
CONFLICT (content): Merge conflict in src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Permit loose manifest priority
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

[github-actions]

@Forgind an error occurred while backporting to main, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.