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

Add design for simplifying workload versioning #294

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

dsplaisted
Copy link
Member

This proposal simplifies how workload versions are expressed and understood.

Currently, the design mainly covers the proposed changes to the user experience for workloads. That is the primary thing we are looking for feedback on now: Would this be a better user experience?

There will also be significant changes needed to implement this. Those still need to be written up and I plan to put those in a separate document, probably as part of this same pull request.


## Specifying workload versions with global.json

We will add support for specifying the workloads version in global.json. For example:
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't this break global.json? It would look for an SDK with that version, which would never exist, or is the plan to look for an SDK version of 8.0.201 and then a workload patch version of 8.0.201.0?

How would someone specify a patch version when the SDK version is prerelease? We cannot have a 4-part version number as that breaks semver 2.0

Should we consider adding a separate entry instead, e.g. `"workload-patch": "0"? That still won't solve the problem with preview SDKs I think

Copy link
Member Author

Choose a reason for hiding this comment

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

The idea was that only the first three parts of the version would be used for SDK selection. You're right though, it's probably better to have a separate value for the workloads version though.


This would force the SDK to use workloads version `8.0.201.0`, and would error if that version was not installed.

We will support side-by-side workload version installations. If 8.0.201.2 is installed, we would support running `dotnet workload install --version 8.0.201.0` to install that version of the workloads. After installing the earlier version, the .NET SDK would still by default use the latest installed workloads version. An earlier workloads version would only be used if it was specified in a global.json file.
Copy link
Member

Choose a reason for hiding this comment

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

This would require having SxS manifests instead of requiring them to upgrade within a band across SDK patches? It would help with the downgrade functionality as well I think.

@marek-safar marek-safar requested a review from lewing April 3, 2023 16:11
A workload update (version 8.0.201.2) is available. Run `dotnet workload update` to install the latest workloads.
```

We will add a new `--version` option to `dotnet workload update` to allow installing (or downgrading to) a specific workloads version:
Copy link
Member

Choose a reason for hiding this comment

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

Would downgrading remove the previous workload version? What happens if a user installs VS and they get 8.0.203.6 of the workloads, but then drop to the CLI and try to downgrade. Would we block this scenario?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it would remove the previous version if possible, but if it was installed by VS it would leave it and print a message saying you'd need to use global.json to select the earlier version.

@steveisok steveisok self-requested a review April 6, 2023 15:16

This would force the SDK to use workloads version `8.0.201.0`, and would error if that version was not installed.

We will support side-by-side workload version installations. If 8.0.201.2 is installed, we would support running `dotnet workload install --version 8.0.201.0` to install that version of the workloads. After installing the earlier version, the .NET SDK would still by default use the latest installed workloads version. An earlier workloads version would only be used if it was specified in a global.json file.
Copy link
Member

Choose a reason for hiding this comment

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

How do we test workloads before they're released? Is there any way to install a locally built workload?

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

The most common error message I see when workloads are broken for customers is something like: TargetPlatformIdentifier 'android' not found.

Usually what happened, is the WorkloadManifest.targets is missing for the android workload, and so the build did not understand the value for $(TargetPlatformIdentifier).

Will these changes be able to give a bit more information in this case? We might have an error message like "Workload Version 8.0.202 is not installed"?

Comment on lines 142 to 144
"sdk": {
"workloadsVersion": "8.0.201"
}
Copy link
Member

Choose a reason for hiding this comment

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

Right now, AutoImport.props is imported into every project for .NET 6, 7, 8, etc. If you have 8.0.201 and 8.0.202 workloads installed, is AutoImport.props only imported for the pinned version?

If so, we might want to add some more robust logging to understand why the workload resolver imported one file over another.

@dsplaisted
Copy link
Member Author

The most common error message I see when workloads are broken for customers is something like: TargetPlatformIdentifier 'android' not found.

Usually what happened, is the WorkloadManifest.targets is missing for the android workload, and so the build did not understand the value for $(TargetPlatformIdentifier).

Will these changes be able to give a bit more information in this case? We might have an error message like "Workload Version 8.0.202 is not installed"?

Do you know why the workload manifest targets aren't installed in these cases? They should always be there.

The new design might be able to help with this but without understanding how people get into this state, it's hard to tell.

@jonathanpeppers
Copy link
Member

I don't think anyone knows exactly how people are getting into this state. I think it is the combination of installing from VS and command-line, then some number of attempted upgrades/downgrades.

Usually the registry query + msiexec I mention here is what solves the problem:

https://gist.github.com/jonathanpeppers/63beb491a9185ac06710261536cc35c9#windows

Copy link
Member

@MiYanni MiYanni left a comment

Choose a reason for hiding this comment

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

I only looked at the first document so far. Will look at the second one tomorrow.

@joeloff
Copy link
Member

joeloff commented May 31, 2023

What are the rules around the versioning scheme inside the manifest? 4-part Windows version or a semantic version?


For workload sets released between releases of the .NET SDK, we will use 4-part version numbers. For example, workload set 8.0.203 would be released together with .NET SDK 8.0.203. If we want to release another workload set before the 8.0.204 SDK release, then we can designate that workload set as 8.0.203.1 (and 8.0.203.2 if there's another interim workload set, etc.). This is a departure from Semantic Versioning, but appears to be the best way to have an understandable version number for interim workload set releases.

Note that the (possibly 4-part) workload set version will be what is displayed by the .NET CLI and can be specified in places such as the command line and global.json files, but it won't be used as a literal package verison for any NuGet packages. NuGet package versions will remain semantically versions. For the NuGet packages that represent the workload set, there will be a mapping from the workload set version to both the package ID and version, where the feature band is part of the package ID, freeing up space in the version for the additional interim release number.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Note that the (possibly 4-part) workload set version will be what is displayed by the .NET CLI and can be specified in places such as the command line and global.json files, but it won't be used as a literal package verison for any NuGet packages. NuGet package versions will remain semantically versions. For the NuGet packages that represent the workload set, there will be a mapping from the workload set version to both the package ID and version, where the feature band is part of the package ID, freeing up space in the version for the additional interim release number.
Note that the (possibly 4-part) workload set version will be what is displayed by the .NET CLI and can be specified in places such as the command line and global.json files, but it won't be used as a literal package version for any NuGet packages. NuGet package versions will remain semantically versions. For the NuGet packages that represent the workload set, there will be a mapping from the workload set version to both the package ID and version, where the feature band is part of the package ID, freeing up space in the version for the additional interim release number.

Comment on lines +100 to +103
- `dotnet workload restore` - global.json aware, same as update
- `dotnet workload install` - global.json aware
- `dotnet workload clean` - should look through all records of global.json, see if they are still there and up-to-date, delete as necessary before GC
- `dotnet workload roots` - (not sure about command name) List all registered global.json files and versions of workload sets they pin
Copy link
Member

Choose a reason for hiding this comment

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

Does this mean in some future, it's possible to have old workload packs around on your system. And then you'd have to delete a global.json to successfully remove those packs?

That seems OK: as the behavior now is you'd lose old packs that some project might be using.


### Release schedule updates

Workload sets will be created in a separate repo (possibly dotnet/workloadversions). This repo will have the versions for all the workload manifests, either via Arcade dependency flow (for workloads that build as part of .NET) or via manual updates. The build will create a json file in the rollback file format from these versions, and create NuGet packages, MSIs, and (in the future) other installers to deliver this json file as a workload set.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Workload sets will be created in a separate repo (possibly dotnet/workloadversions). This repo will have the versions for all the workload manifests, either via Arcade dependency flow (for workloads that build as part of .NET) or via manual updates. The build will create a json file in the rollback file format from these versions, and create NuGet packages, MSIs, and (in the future) other installers to deliver this json file as a workload set.
Workload sets will be created in a separate repo (possibly dotnet/workloadversions). This repo will have the versions for all the workload manifests, either via Arcade dependency flow (for workloads that build as part of .NET) or via manual updates. The build will create a json file in the rollback file format from these versions, and create NuGet packages, MSIs, and (in the future) other installers to deliver this json file as a workload set.


## Side by side workload sets

The workload `restore`, `update`, and `install` commands will be updated to check if there is a global.json file specifying a workload set version applying to the current folder. If so, these commands will install that workload set version side-by-side with any other installed workload set. They will also record the path to the global.json file and the workload set version it specified. This record will be used as a root during garbage collection to prevent the side-by-side workload set (and its packs) from being removed.
Copy link
Member

Choose a reason for hiding this comment

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

Does this mean some file in /usr/local/share/dotnet will include a path to /Users/aklig/work/fooProject/global.json ?

What if I rename the fooProject folder to barProject?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would be better to have an explicit sudo dotnet workload pin <workload version set> and sudo dotnet workload unpin <workload version set> command that could protect a workload set from being removed by dotnet workload clean ? And then dotnet workload --info would display if a certain workload set is pinned or not.

This is what Homebrew does when you need to prevent some out of date version from being removed.


The workload resolver should first look for a workload set version from a global.json file. If not specified via global.json, then it should look in the workload install state folder for a workload set or workload manifest versions to use. If not specified via the install state folder, then the resolver should use the latest workload set version installed for the feature band. Finally, if no workload set is installed, the resolver should use the latest version of each manifest for the feature band.

### Installing workload set from global.json
Copy link
Member

Choose a reason for hiding this comment

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

We talked about this in a recent meeting, but one pain point for users is figuring out what version numbers are valid to go into the global.json - we should have some command that could tell them this information.

@dsplaisted dsplaisted force-pushed the simplify-workload-versioning branch from 59f2d4d to f8990c8 Compare March 1, 2024 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants