Delete spurious error

[Forgind]

Fixes AB #2357570

Description

The proximal cause of this is that /usr/share/dotnet/sdk-manifests/8.0.100/workloadsets/8.0.402-baseline.24467.1 is a path that exists, but there are no workload sets inside. The PR that I mentioned to Marc in our triage meeting today was actually backported to 8.0.4xx and made it into 8.0.4012, so it wasn’t the issue that I’d initially expected.

The real issue is that that folder is empty. There is no point in which we create that folder without putting a workload set in it. However, I noticed that that folder is for the 8.0.402 SDK, and when the customer ran dotnet –info, it claimed to be 8.0.404. That discrepancy suggests that the 8.0.402 SDK was installed at some point then partially uninstalled—the workload set was deleted, but the folder was not deleted, perhaps because it was in use—and then the 8.0.404 SDK was installed on top.

While looking for corroborating evidence online, I came across this comment from baronfel in which he suggested the issue may be that we are marking the baseline workload set but not the folders during RPM construction. That may be fixable, though he later suggested that it may not be sufficient.

Customer Impact

Customers using the affected (broken) installs can't do anything that involves workloads.

User-reported

Yes

Testing

I made an 8.0.100 folder with a workloadsets folder inside that had a valid (older) feature band folder that was empty, just as in the original report. I tried updating workloads, and it succeeded. I debugged the code and stopped at the point where this had previously failed and verified that it returned null instead of throwing then ignored the null result, not failing and moving on as intended.

Risk

Low—deletes an error and accounts for the possible null response.

Workaround

In the short term, I’d suggest that the customer should just delete the (empty) directory, as that’s the easiest workaround. As a fix from our side, I propose deleting the error, as it isn’t a critical failure anyway. That should have minimal risk, and it would be a very small change.

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs:545

• Ensure that workloadSet.Version is not null before using it. The null-forgiving operator (!) should not be used without a prior null check.

availableWorkloadSets[workloadSet.Version!] = workloadSet;

src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadSet.cs:84

• [nitpick] The method name 'FromWorkloadSetFolder' is misleading now that it can return null. Consider renaming it to 'TryGetWorkloadSetFromFolder' or similar.

public static WorkloadSet? FromWorkloadSetFolder(string path, string workloadSetVersion, SdkFeatureBand defaultFeatureBand)

[dsplaisted]

Is this change necessary? It doesn't seem to be related to an empty workload set folder.

[Forgind]

Previously, FromWorkloadSetFolder guaranteed returning a real workload set (or throwing). That was called in three places: SdkDirectoryWorkloadManifestProvider (which is the only 'important' case, as far as I can tell), FileBasedInstaller, and NetSdkMsiInstaller, both of which pass the workload set to this call.

I don't think this change is actually necessary because both of them should have installed a workload set just prior to this, but if they somehow succeed in finding a workload set, then attempt to install it and even make the folder, then fail to install it, then this could theoretically be null, and we'd get a NRE without this check.

More succinctly, I think it's theoretically possible that this could save us from a NRE but very, very unlikely. But especially since this is servicing, I think safe is better than sorry, and I don't see any way this could hurt.