Error condition fix Fixes #40297 #41741
Conversation
dotnet-issue-labeler
bot
added
Area-Workloads
untriaged
| @@ -123,6 +123,13 @@ protected bool TryHandleWorkloadUpdateFromVersion(ITransactionContext context, D | |||
| _workloadInstaller.UpdateInstallMode(_sdkFeatureBand, true); | |||
| } | |||
|
|
|||
| // Delete the current advertising manifest because if we fail to find the right workload version, we want to fail. | |||
| var advertisingPackagePath = Path.Combine(_userProfileDir, "sdk-advertising", _sdkFeatureBand.ToString(), "microsoft.net.workloads"); | |||
There was a problem hiding this comment.
nit: Can you pull microsoft.net.workloads out into a variable with a clear name so we can tell this is the workload set folder?
There was a problem hiding this comment.
dsplaisted creates that as a constant in #39991, so I'd rather wait for that to come in then move it somewhere more central and use it.
|
CC @dsplaisted as this may conflict or not be needed with his refactor so maybe wait until after that PR is in. |
| @@ -160,6 +160,6 @@ | |||
| <value>No workload update found.</value> | |||
| </data> | |||
| <data name="WorkloadVersionRequestedNotFound" xml:space="preserve"> | |||
| <value>Workload version {0} not found.</value> | |||
| <value>Workload version {0} not found. Adding a feed that contains it to your NuGet.config may help.</value> | |||
There was a problem hiding this comment.
The message now uses WorkloadVersionFromGlobalJsonNotFound, and the text is:
Workload update failed: Workload version 8.0.300-preview.0.24217.2, which was specified in C:\SdkTesting\global.json, was not found. Run "dotnet workload restore" to install this workload version.
I don't think we need a message here about adding a feed, as in most cases the workload should be available on NuGet.org and they shouldn't need to mess with their feeds. If the dotnet workload restore command fails, that's when we could mention the feeds. It looks like that message could stand to be cleaned up, currently it looks something like this:
Unhandled exception: System.IO.FileNotFoundException: Workload version 8.0.300-preview.0.24217.2, which was specified in C:\SdkTesting\global.json, was not found. Run "dotnet workload restore" to install this workload version.
at Microsoft.NET.Sdk.WorkloadManifestReader.SdkDirectoryWorkloadManifestProvider.ThrowExceptionIfManifestsNotAvailable() in C:\git\dotnet-sdk\src\Resolvers\Microsoft.NET.Sdk.WorkloadManifestReader\SdkDirectoryWorkloadManifestProvider.cs:line 231
at Microsoft.NET.Sdk.WorkloadManifestReader.SdkDirectoryWorkloadManifestProvider.GetManifests() in C:\git\dotnet-sdk\src\Resolvers\Microsoft.NET.Sdk.WorkloadManifestReader\SdkDirectoryWorkloadManifestProvider.cs:line 269
at Microsoft.NET.Sdk.WorkloadManifestReader.WorkloadResolver.LoadManifestsFromProvider(IWorkloadManifestProvider manifestProvider) in C:\git\dotnet-sdk\src\Resolvers\Microsoft.NET.Sdk.WorkloadManifestReader\WorkloadResolver.cs:line 124
at Microsoft.NET.Sdk.WorkloadManifestReader.WorkloadResolver.InitializeManifests() in C:\git\dotnet-sdk\src\Resolvers\Microsoft.NET.Sdk.WorkloadManifestReader\WorkloadResolver.cs:line 91
at Microsoft.NET.Sdk.WorkloadManifestReader.WorkloadResolver.GetInstalledManifests() in C:\git\dotnet-sdk\src\Resolvers\Microsoft.NET.Sdk.WorkloadManifestReader\WorkloadResolver.cs:line 771
at Microsoft.DotNet.Workloads.Workload.Update.WorkloadUpdateCommand.Execute() in C:\git\dotnet-sdk\src\Cli\dotnet\commands\dotnet-workload\update\WorkloadUpdateCommand.cs:line 86
at Microsoft.DotNet.Cli.WorkloadUpdateCommandParser.<>c.b__6_0(ParseResult parseResult) in C:\git\dotnet-sdk\src\Cli\dotnet\commands\dotnet-workload\update\WorkloadUpdateCommandParser.cs:line 52
at System.CommandLine.Invocation.AnonymousCliAction.Invoke(ParseResult parseResult)
at System.CommandLine.Invocation.InvocationPipeline.Invoke(ParseResult parseResult)
at System.CommandLine.ParseResult.Invoke()
at Microsoft.DotNet.Cli.Program.ProcessArgs(String[] args, TimeSpan startupTime, ITelemetry telemetryClient) in C:\git\dotnet-sdk\src\Cli\dotnet\Program.cs:line 232
| // Delete the current advertising manifest because if we fail to find the right workload version, we want to fail. | ||
| var advertisingPackagePath = Path.Combine(_userProfileDir, "sdk-advertising", _sdkFeatureBand.ToString(), "microsoft.net.workloads"); | ||
| if (Directory.Exists(advertisingPackagePath)) | ||
| { | ||
| Directory.Delete(advertisingPackagePath, recursive: true); | ||
| } |
There was a problem hiding this comment.
I think after my refactoring, this is probably not necessary anymore. Can you verify or explain what the scenario was where this was broken?
|
@Forgind I think we should retarget to main along with the merge conflict resolution |
Fixes #40297
This makes an error message a little more clear and ensures that the error actually fires when appropriate. Previously, if a user had successfully installed a workload set, then immediately tried to run, for example,
dotnet workload update --version 8.0.422where 8.0.422 doesn't exist, it would fail to find 8.0422 and instead find the previous version found and 'install' that instead, claiming success. Then it would butcher the install state file and fail. This ensures that it fails early enough to not do that and in a clearer way.