Add spec for workload resolution in MSBuild #104
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
akoeplinger
reviewed
Mar 27, 2020
This was referenced Apr 1, 2020
Add features to MSBuild SDK Resolver interface to support optional SDK workloads
dotnet/msbuild#5239
Closed
Co-Authored-By: Alexander Köplinger <[email protected]>
Co-Authored-By: Alexander Köplinger <[email protected]>
richlander
reviewed
Apr 8, 2020
richlander
reviewed
Apr 8, 2020
richlander
reviewed
Apr 8, 2020
richlander
reviewed
Apr 8, 2020
richlander
reviewed
Apr 8, 2020
richlander
reviewed
Apr 8, 2020
richlander
reviewed
Apr 8, 2020
richlander
reviewed
Apr 8, 2020
|
|
||
| MSBuild uses multiple phases for evaluation, and properties and imports are evaluated in a phase before items. This means it is possible to have conditions on item or `ItemGroup` elements in a .props file that depend on properties that are defined in the body of the project or in .targets files. | ||
|
|
||
| So workloads may include MSBuild items in the `AutoImport.props` file if they are appropriately conditioned to only activate when the workload is in use. They should not set MSBuild properties, as there is no way to set those properties only if the workload is in use. If workload wants to set a default value for a property, then it should do so in its .targets file with a condition to set the property if it is not already set. |
Member
There was a problem hiding this comment.
Suggested change
| So workloads may include MSBuild items in the `AutoImport.props` file if they are appropriately conditioned to only activate when the workload is in use. They should not set MSBuild properties, as there is no way to set those properties only if the workload is in use. If workload wants to set a default value for a property, then it should do so in its .targets file with a condition to set the property if it is not already set. | |
| Workloads may include MSBuild items in the `AutoImport.props` file if they are appropriately conditioned to only activate when the workload is in use. They should not set MSBuild properties, as there is no way to set those properties only if the workload is in use. If workload wants to set a default value for a property, then it should do so in its .targets file with a condition to set the property if it is not already set. |
richlander
reviewed
Apr 8, 2020
|
|
||
| So workloads may include MSBuild items in the `AutoImport.props` file if they are appropriately conditioned to only activate when the workload is in use. They should not set MSBuild properties, as there is no way to set those properties only if the workload is in use. If workload wants to set a default value for a property, then it should do so in its .targets file with a condition to set the property if it is not already set. | ||
|
|
||
| Note that in contrast to the `WorkloadManifest.targets` file, which is part of the workload manifest package, the `AutoImport.props` files are part of the workload packs (under the Sdk folder). This is so that they can be imported only if the workload is installed, rather than imported for all workloads whether they are installed or not. This reduces the impact of a bug in an `AutoImport.props` file, and means that if there is an issue in the .props file, it can be worked around by uninstalling the workload. |
Member
There was a problem hiding this comment.
This paragraph and the one before it seems like they duplicate some of the same content and could be streamlined a bit.
richlander
reviewed
Apr 8, 2020
richlander
reviewed
Apr 8, 2020
richlander
reviewed
Apr 8, 2020
richlander
reviewed
Apr 8, 2020
|
|
||
| ## NuGet SDK Resolver | ||
|
|
||
| MSBuild includes a [NuGet-based MSBuild SDK resolver](https://docs.microsoft.com/en-us/visualstudio/msbuild/how-to-use-project-sdk?view=vs-2019#how-project-sdks-are-resolved). It will be used if a version number is specified for the SDK, either in the SDK attribute or in a global.json file. |
Member
There was a problem hiding this comment.
It will be used if a version number is specified for the SDK, either in the SDK attribute or in a global.json file.
I don't quite get this (I'm probably just missing context). I was expecting this to say "If you had an sdk reference other than MS.Net.Sdk".
Member
Author
There was a problem hiding this comment.
You have to specify a version number in order for the NuGet SDK resolver to kick in. You can specify that either directly in the Sdk attribute: <Project Sdk="MSBuild.Sdk.Extras/2.0.54">, or in global.json:
{
"msbuild-sdks": {
"MSBuild.Sdk.Extras": "2.0.54"
}
}
Member
There was a problem hiding this comment.
I see. That seems like overly precise wording.
Suggested change
| MSBuild includes a [NuGet-based MSBuild SDK resolver](https://docs.microsoft.com/en-us/visualstudio/msbuild/how-to-use-project-sdk?view=vs-2019#how-project-sdks-are-resolved). It will be used if a version number is specified for the SDK, either in the SDK attribute or in a global.json file. | |
| MSBuild includes a [NuGet-based MSBuild SDK resolver](https://docs.microsoft.com/en-us/visualstudio/msbuild/how-to-use-project-sdk?view=vs-2019#how-project-sdks-are-resolved). It will be used if a custom SDK is specified, as an SDK name and version number, either in the SDK attribute in the project file or in a global.json file. |
richlander
approved these changes
Apr 8, 2020
Co-Authored-By: Rich Lander <[email protected]>
This was referenced Apr 10, 2020
mhutch
approved these changes
Apr 10, 2020
Co-Authored-By: Mikayla Hutchinson <[email protected]>
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This spec covers some of the implementation for optional SDK workloads in .NET 5. The overall spec is in #100.