Skip to content

Allow dotnet workload update print-download-link-only to use fallback and work with prior SDK versions#26369

Merged
marcpopMSFT merged 28 commits into
dotnet:release/6.0.4xxfrom
gkulin:downloadLinkChange
Jul 15, 2022
Merged

Allow dotnet workload update print-download-link-only to use fallback and work with prior SDK versions#26369
marcpopMSFT merged 28 commits into
dotnet:release/6.0.4xxfrom
gkulin:downloadLinkChange

Conversation

@gkulin

@gkulin gkulin commented Jun 29, 2022

Copy link
Copy Markdown
Contributor

fixes #24335

Description

The VSMac updater calls the prior band SDK and asks it which workloads to install with the current feature band. The feature was originally designed to work with only the current band and not cross-band and this change fixes it to work that way.

Customer Impact

Without this change, VSMac customers who upgrade will not be told on their initial upgrade that there are workloads that require updates. They will be forced to either drop to the command line and manually update workloads or wait for a future update that may or may not work depending on if it's a patch update or not.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

The change itself is a bit bigger but it's affect is almost entirely in the --print-downlaod-linke-only feature of the workloads CLI. This command is only used by VSMac and it's completely broken today.

Verification

  • Manual (required)
  • Automated
    We have not tested with VSMac yet as we need either an official build with newer workloads or to backport this to 6.0.3xx.

@ghost ghost added the Area-Workloads label Jun 29, 2022
Comment thread src/Cli/dotnet/NugetPackageDownloader/NuGetPackageDownloader.cs Outdated
Comment thread src/Cli/dotnet/commands/dotnet-workload/install/IInstaller.cs Outdated
Comment thread src/Cli/dotnet/commands/dotnet-workload/install/WorkloadManifestUpdater.cs Outdated
Comment thread src/Cli/dotnet/commands/dotnet-workload/install/WorkloadManifestUpdater.cs Outdated
Comment thread src/Cli/dotnet/commands/dotnet-workload/install/WorkloadManifestUpdater.cs Outdated
Comment thread src/Layout/toolset-tasks/OverrideAndCreateBundledNETCoreAppPackageVersion.cs Outdated
Comment thread src/Tests/dotnet-workload-install.Tests/GivenWorkloadManifestUpdater.cs Outdated

@nagilson nagilson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In response to earlier comment, provided more targeted feedback

Comment thread src/Cli/dotnet/NugetPackageDownloader/NuGetPackageDownloader.cs Outdated
Comment thread src/Cli/dotnet/commands/dotnet-sdk/check/SdkOutputWriter.cs Outdated
Comment thread src/Cli/dotnet/commands/dotnet-workload/install/WorkloadManifestUpdater.cs Outdated
{
try
{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should update UpdateAdvertisingManifestAsync so that it calls GetManifestPackageDownloadsAsync instead of having the logic duplicated.

Comment thread src/Cli/dotnet/commands/dotnet-workload/install/WorkloadManifestUpdater.cs Outdated
Comment thread src/Tests/dotnet-workload-update.Tests/GivenDotnetWorkloadUpdate.cs Outdated
Comment thread src/Cli/dotnet/commands/InstallingWorkloadCommand.cs Outdated
@marcpopMSFT

Copy link
Copy Markdown
Member

Approved offline, fingers crossed.

@marcpopMSFT
marcpopMSFT merged commit 52a29bc into dotnet:release/6.0.4xx Jul 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants