Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use install state in workload manifest provider #33643

Merged
merged 13 commits into from
Jul 13, 2023

Conversation

dsplaisted
Copy link
Member

@dsplaisted dsplaisted commented Jun 28, 2023

This PR contains a bug fix and a feature.

Bug Fix

Description

Fixes issue where when reading side-by-side workload manifests, the Manifest ID would be reported incorrectly.

Customer Impact

If this change doesn't go into 7.0.4xx / VS 17.7, then VS 17.7 and its version of MSBuild will not work properly with future builds of the .NET 8 SDK (once we switch to side-by-side workload manifests). The behavior will either be that the resolver will crash, preventing builds from succeeding, or that it won't successfully load any workloads.

Regression?

Risk

  • High
  • Medium
  • Low

Verification

  • Manual
  • Automated

Feature

Description

Implements last part of resolver logic for dotnet/designs#294, which is to honor workload install state. This will support commands which install a different workload set version, for example dotnet workload update --version 8.0.101.

Customer Impact

If this change doesn't go into 7.0.4xx / VS 17.7, then in some cases where a command is run that selects a specific workload set version, that would not be honored by Visual Studio 17.7 and its version of MSBuild.

A workaround would be to specify the desired workload set version in a global.json file.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

Verification

  • Manual
  • Automated

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Workloads untriaged Request triage from a team member labels Jun 28, 2023
@dsplaisted dsplaisted requested a review from a team June 28, 2023 21:26
@dsplaisted dsplaisted marked this pull request as ready for review June 28, 2023 21:52
@Forgind
Copy link
Member

Forgind commented Jun 29, 2023

Any chance you could isolate the changes that were just moving things from one class to another from the changes that had substantive code impact? The line count is high, but a lot of the early parts especially were just moving code 🙂

@dsplaisted
Copy link
Member Author

Any chance you could isolate the changes that were just moving things from one class to another from the changes that had substantive code impact? The line count is high, but a lot of the early parts especially were just moving code 🙂

@Forgind I've rebased the code moves into separate commits.

@dsplaisted
Copy link
Member Author

@jaredpar This commit seems to have caused the Full Framework CI leg to time out. Is returning a ref struct from a method not supported or handled differently on .NET Framework?

@dsplaisted dsplaisted added Servicing-consider and removed untriaged Request triage from a team member labels Jul 6, 2023
@dsplaisted dsplaisted mentioned this pull request Jul 7, 2023
16 tasks
@dsplaisted dsplaisted merged commit 651aa1d into dotnet:release/7.0.4xx Jul 13, 2023
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.

4 participants