From 01eef7c14ec5fcfc4eed3d37af326fdd9f9b346b Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Wed, 19 Jun 2024 18:00:07 -0700 Subject: [PATCH 01/20] Document the plan for fixing torn state compiler This is a _very rough_ draft --- documentation/general/torn-sdk.md | 109 ++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 documentation/general/torn-sdk.md diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md new file mode 100644 index 000000000000..f449c1c6bd22 --- /dev/null +++ b/documentation/general/torn-sdk.md @@ -0,0 +1,109 @@ +# Torn .NET SDK + +## Terminology + +- msbuild: refers the .NET Framework based msbuild included with Visual Studio. +- dotnet msbuild: refers to the .NET Core based msbuild included with the .NET SDK. +- analyzers: this document will use analyzers to refer to both analyzers and generators. + +## Summary + +Visual Studio and .NET SDK are separate products but they are intertwined in command line and design time build scenarios as different components are loaded from each product. + +| Scenario | Loads Roslyn | Loads Analyzers / Generators | +| --- | --- | --- | +| msbuild | From Visual Studio | From .NET SDK | +| dotnet msbuild | From .NET SDK | From .NET SDK | +| Visual Studio Design Time | From Visual Studio | From Both | + +Generally this mixing of components is fine because Visual Studio will install a .NET SDK that is functionally paired with it. That is the compiler and analyzers are the same hence mixing poses no real issue. For example 17.10 installs .NET SDK 8.0.3xx, 17.9 installs .NET SDK 8.0.2xx, etc ... However when the .NET SDK is not paired with the Visual Studio version,then compatibility issues can, and will, arise. Particularly when the .NET SDK is _newer_ than Visual Studio customers will end up with the following style of error: + +> CSC : warning CS9057: The analyzer assembly '..\dotnet\sdk\8.0.200\Sdks\Microsoft.NET.Sdk.Razor\source- +generators\Microsoft.CodeAnalysis.Razor.Compiler.SourceGenerators.dll' references version '4.9.0.0' of the compiler, which is newer +than the currently running version '4.8.0.0'. + +To address this issue we are going to separate Visual Studio and the .NET SDK in build scenarios. That will change our matrix to the following: + +| Scenario | Loads Roslyn | Loads Analyzers / Generators | +| --- | --- | --- | +| msbuild | From .NET SDK | From .NET SDK | +| dotnet msbuild | From .NET SDK | From .NET SDK | +| Visual Studio Design Time | From Visual Studio | From Visual Studio | + +Specifically we will be + +1. Changing msbuild to use a compiler from the .NET SDK +2. Changing Visual Studio to use analyzers from Visual Studio + +In addition to making our builds more reliable this will also massively simplify our analyzer development strategy. Analyzers following this model can always target the latest Roslyn version without the need for complicated multi-targeting. + +## Detailed Design + +### MSBuild using .NET SDK Compiler + +The .NET SDK will start producing a package named Microsoft.NETSdk.Compiler.Roslyn. This will contain a .NET Framework version of the Roslyn compiler that matches .NET Core version. This will be published as a part of the .NET SDK release process meaning it's available for all publicly available .NET SDKs. + +The .NET SDK has the capability to [detect a torn state][pr-detect-torn-state]. When this is detected the matching Microsoft.NETSDK.Compiler.Roslyn package for this version of the .NET SDK will be downloaded via ``. Then the `$(RoslynTargetsPath)` will be reset to point into the package contents. This will cause MSBuild to load the Roslyn compiler from that location vs. the Visual Studio location. + +One downside to this approach is that it is possible to end up with two VBCSCompiler server processes. Consider that when a solution has a mix of .NET SDK style projects and legacy projects then in a torn state the .NET SDK projects will use the .NET SDK compiler will legacy projects will use the Visual Studio compiler. While not a desirable outcome, it is a correct one. Customers who wish to only have one VBCSCompiler should correct the torn state in their build tools. + +### Visual Studio using Visual Studio Analyzers + +#### .NET SDK in box analyzers + +Analyzers which ship in the .NET SDK box will change to having a copy checked into Visual Studio. When .NET SDK based projects are loaded at design time, the Visual Studio copy of the analyzer will be loaded. Roslyn already understands how to prefer Visual Studio copies of analyzers. That work will need to be extended a bit but that is pretty straight forward code. + +This approach enables us to take actions like NGEN or R2R analyzers inside of Visual Studio. This is a long standing request from the Visual Studio perf team but very hard to satisfy in our current + +This approach is already taken by [the Razor generator][code-razor-vs-load]. This work was done for other reliability reasons but turned out working well for this scenario. There is initial upfront work to get the in box copy loading but it has virtually zero maintanence cost. + +This also means these analyzers can vastly simplify their development model by alwasy targeting the latest version of Roslyn. There is no more need to multi-target beacuse the version of the compiler the analyzer will be used with is known at ship time for all core build scenarios. + +This means that our design time experience can differ from our command line experience when customers are in a torn state. Specifically it's possible, even likely in some cases, that the diagonstics produced by design time builds will differ from command line builds. That is a trade off that we are willing to make. Customers who wish to have a consistent experience between design time should not operate in a torn state. + +#### NuGet based analyzers + +Analyzers which ship via NuGet will continue to following the existing [support policies][matrix-of-paine]. This means that they will either need to target a sufficiently old version of Roslyn or implement multi-targeting support. + +## Work Breakdown + +- [ ]: Flow the Micosoft.Net.Compilers.Toolset.Framework package to the .NET SDK +- [ ]: Create a new package Micosoft.NETSdk.Compiler.Roslyn in .NET SDK. + - [ ]: The contents of this package will include the contents of the Microsoft.Net.Compilers.Toolset.Framework package + - [ ]: The contents will include a README.md stating the package is **not** supported for direct user consumption. + - [ ]: The package will follow the versioning scheme of the .NET SDK. +- [ ]: Change the Sdk.targets file to have copies of the following three `` from [Microsoft.Common.tasks][microsoft-common-tasks]. Having a copy in Sdk.targets means that resetting `$(RoslynTargetsPath)` during build will change the chosen compiler. +- [ ]: When the .NET SDK detects a torn state + - [ ]: Use a `` to acquire the Microsoft.NETSdk.Compiler.Roslyn package + - [ ]: Change `$(RoslynTargetsPath)` to point into the package contents + +## Alternative + +### PackageReference + +Instead of `` the toolset package could be installed via ``. This is how the existing Microsoft.Net.Compilers.Toolset package works today. This has a number of disadvantages: + +1. PackageReferences are not side effect free and have been shown to cause issues in complex builds. For example this package did not work by default in the Visual Studio or Bing builds. Deploying this at scale to all our customers will almost ceratinly cause issues. +2. This approach does not solve all scenarios. The mechanics of restore mean that the compiler is not swapped until part way through the build. Build operations that happen earlier, such as legacy WPF, will still use the Visual Studio compiler and hence experience analyzer compatibility issues. +3. Mixing builds that do and do not restore can lead to different build outcomes as they can end up choosing different compilers. Mixing the version of msbuild / dotnet msbuild can also lead to strange outcomes as they can end up choosing different compilers. + +### Install the Framework compiler + +An alternative is simply shipping both Roslyn compilers in the Windows .NET SDK. This means there is no need for a `` step as the compiler is always there. This is a simpler build but it means that all .NET SDK installs on Windows will incease by ~6-7%. + +### Use the .NET Core compiler + +Instead of downloading a .NET Framework Roslyn in a torn state, the SDK could just use the .NET Core Roslyn it comes with. That would have virtually no real compatibility issues. This has a number of disadvantages: + +1. Changes the process name from VBCSCompiler to `dotnet`. That seems insignificant but the process name is important as it's referenced in a lot of build scripts. This would be a subtle breaking change that we'd have to work with customers on. +2. This would make Visual Studio dependent on the servicing lifetime of the .NET Core runtime in the SDK. That has a very different lifetime and could lead to surprising outcomes down the road. For example if policy removed out of support runtimes from machines it would break the Visual Studio build. + +## Related Issues + +- https://github.com/dotnet/roslyn/issues/72672 +- https://github.com/dotnet/installer/pull/19144 + +[microsoft-common-tasks]: https://github.com/dotnet/msbuild/blob/main/src/Tasks/Microsoft.Common.tasks#L106-L109 +[matrix-of-paine]: https://aka.ms/dotnet/matrix-of-paine +[pr-detect-torn-state]: https://github.com/dotnet/installer/pull/19144 +[code-razor-vs-load]: https://github.com/dotnet/roslyn/blob/9aea80927e3d4e5a2846efaa710438c0d8d2bfa2/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProject.cs#L1009 From b1d7a9f24704dbbc07c0bb7995f00b7bd3e8a9ce Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Thu, 20 Jun 2024 20:24:08 -0700 Subject: [PATCH 02/20] Apply suggestions from code review Co-authored-by: Fred Silberberg --- documentation/general/torn-sdk.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index f449c1c6bd22..5c1800e8cac8 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -57,7 +57,7 @@ This approach enables us to take actions like NGEN or R2R analyzers inside of Vi This approach is already taken by [the Razor generator][code-razor-vs-load]. This work was done for other reliability reasons but turned out working well for this scenario. There is initial upfront work to get the in box copy loading but it has virtually zero maintanence cost. -This also means these analyzers can vastly simplify their development model by alwasy targeting the latest version of Roslyn. There is no more need to multi-target beacuse the version of the compiler the analyzer will be used with is known at ship time for all core build scenarios. +This also means these analyzers can vastly simplify their development model by always targeting the latest version of Roslyn. There is no more need to multi-target because the version of the compiler the analyzer will be used with is known at ship time for all core build scenarios. This means that our design time experience can differ from our command line experience when customers are in a torn state. Specifically it's possible, even likely in some cases, that the diagonstics produced by design time builds will differ from command line builds. That is a trade off that we are willing to make. Customers who wish to have a consistent experience between design time should not operate in a torn state. From ff51747f6eb060ace2e1034f0cbfb8e6184e0db8 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Thu, 20 Jun 2024 20:54:04 -0700 Subject: [PATCH 03/20] PR feedback --- documentation/general/torn-sdk.md | 55 +++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 18 deletions(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index 5c1800e8cac8..eba61ae53504 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -37,9 +37,15 @@ Specifically we will be In addition to making our builds more reliable this will also massively simplify our analyzer development strategy. Analyzers following this model can always target the latest Roslyn version without the need for complicated multi-targeting. -## Detailed Design +## Goals -### MSBuild using .NET SDK Compiler +This design has a number of goals: + +1. The `msbuild` and `dotnet build` build experience should be equivalent. +1. The Visual Studio Design time experience is independent of the .NET SDK installed. +1. To make explicit that it is okay, and even expected, that the design time and command line build experiences can differ when the SDK is in a torn state. + +## MSBuild using .NET SDK Compiler The .NET SDK will start producing a package named Microsoft.NETSdk.Compiler.Roslyn. This will contain a .NET Framework version of the Roslyn compiler that matches .NET Core version. This will be published as a part of the .NET SDK release process meaning it's available for all publicly available .NET SDKs. @@ -47,38 +53,36 @@ The .NET SDK has the capability to [detect a torn state][pr-detect-torn-state]. One downside to this approach is that it is possible to end up with two VBCSCompiler server processes. Consider that when a solution has a mix of .NET SDK style projects and legacy projects then in a torn state the .NET SDK projects will use the .NET SDK compiler will legacy projects will use the Visual Studio compiler. While not a desirable outcome, it is a correct one. Customers who wish to only have one VBCSCompiler should correct the torn state in their build tools. -### Visual Studio using Visual Studio Analyzers +## Visual Studio using Visual Studio Analyzers -#### .NET SDK in box analyzers +### .NET SDK in box analyzers dual insert Analyzers which ship in the .NET SDK box will change to having a copy checked into Visual Studio. When .NET SDK based projects are loaded at design time, the Visual Studio copy of the analyzer will be loaded. Roslyn already understands how to prefer Visual Studio copies of analyzers. That work will need to be extended a bit but that is pretty straight forward code. This approach enables us to take actions like NGEN or R2R analyzers inside of Visual Studio. This is a long standing request from the Visual Studio perf team but very hard to satisfy in our current -This approach is already taken by [the Razor generator][code-razor-vs-load]. This work was done for other reliability reasons but turned out working well for this scenario. There is initial upfront work to get the in box copy loading but it has virtually zero maintanence cost. +This approach is already taken by [the Razor generator][code-razor-vs-load]. This work was done for other reliability reasons but turned out working well for this scenario. There is initial upfront work to get the Visual Studio copy loading in design time builds but it has virtually zero maintanence cost. This also means these analyzers can vastly simplify their development model by always targeting the latest version of Roslyn. There is no more need to multi-target because the version of the compiler the analyzer will be used with is known at ship time for all core build scenarios. -This means that our design time experience can differ from our command line experience when customers are in a torn state. Specifically it's possible, even likely in some cases, that the diagonstics produced by design time builds will differ from command line builds. That is a trade off that we are willing to make. Customers who wish to have a consistent experience between design time should not operate in a torn state. +This does mean that our design time experience can differ from our command line experience when customers are in a torn state. Specifically it's possible, even likely in some cases, that the diagonstics produced by design time builds will differ from command line builds. That is a trade off that we are willing to make for reliability. Customers who wish to have a consistent experience between design time should not operate in a torn state. -#### NuGet based analyzers +## .NET SDK in box analyzers target oldest -Analyzers which ship via NuGet will continue to following the existing [support policies][matrix-of-paine]. This means that they will either need to target a sufficiently old version of Roslyn or implement multi-targeting support. +Analyzers which have no need to target the latest Roslyn could instead choose to target the oldest supported version of Roslyn. That ensures they can safely load into any supported scenario without issue. -## Work Breakdown +This strategy should be considered short term. The Roslyn API is under constant development to respond to the performance demands of Visual Studio. New APIs can quickly become virtually mandatory for analyzers and generators to use and will only be available on latest. At that point the analyzer will need to also update to use dual insertions. -- [ ]: Flow the Micosoft.Net.Compilers.Toolset.Framework package to the .NET SDK -- [ ]: Create a new package Micosoft.NETSdk.Compiler.Roslyn in .NET SDK. - - [ ]: The contents of this package will include the contents of the Microsoft.Net.Compilers.Toolset.Framework package - - [ ]: The contents will include a README.md stating the package is **not** supported for direct user consumption. - - [ ]: The package will follow the versioning scheme of the .NET SDK. -- [ ]: Change the Sdk.targets file to have copies of the following three `` from [Microsoft.Common.tasks][microsoft-common-tasks]. Having a copy in Sdk.targets means that resetting `$(RoslynTargetsPath)` during build will change the chosen compiler. -- [ ]: When the .NET SDK detects a torn state - - [ ]: Use a `` to acquire the Microsoft.NETSdk.Compiler.Roslyn package - - [ ]: Change `$(RoslynTargetsPath)` to point into the package contents +### NuGet based analyzers + +Analyzers which ship via NuGet will continue to following the existing [support policies][matrix-of-paine]. This means that they will either need to target a sufficiently old version of Roslyn or implement multi-targeting support. ## Alternative +### .NET SDK in box analyzers multi-target + +Technically in box analyzers can have a multi-targeting strategy just as NuGet based analyzers do. This is actively discouraged because it leads to unnecessary increases in .NET SDK sizes. The time spent implementing multi-targeting is likely better spent moving to a dual insertion to keep .NET SDK sizes down and provide a consistent experience with other analyzers. + ### PackageReference Instead of `` the toolset package could be installed via ``. This is how the existing Microsoft.Net.Compilers.Toolset package works today. This has a number of disadvantages: @@ -107,3 +111,18 @@ Instead of downloading a .NET Framework Roslyn in a torn state, the SDK could ju [matrix-of-paine]: https://aka.ms/dotnet/matrix-of-paine [pr-detect-torn-state]: https://github.com/dotnet/installer/pull/19144 [code-razor-vs-load]: https://github.com/dotnet/roslyn/blob/9aea80927e3d4e5a2846efaa710438c0d8d2bfa2/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProject.cs#L1009 + +## Work Breakdown + +This will be moved to an issue when this PR is opened against dotnet/sdk proper: + +- [ ]: Flow the Micosoft.Net.Compilers.Toolset.Framework package to the .NET SDK +- [ ]: Create a new package Micosoft.NETSdk.Compiler.Roslyn in .NET SDK. + - [ ]: The contents of this package will include the contents of the `tasks\net472` folder in the Microsoft.Net.Compilers.Toolset.Framework package. This subset is all that is needed and makes the package not usable via ``. The latter reduces the incentive for customers to use it directly. + - [ ]: The contents will include a README.md stating the package is **not** supported for direct user consumption. + - [ ]: The package will follow the versioning scheme of the .NET SDK. + - [ ]: The package will be unlisted (ideal but not a hard requirement) +- [ ]: Change the Sdk.targets file to have copies of the following three `` from [Microsoft.Common.tasks][microsoft-common-tasks]. Having a copy in Sdk.targets means that resetting `$(RoslynTargetsPath)` during build will change the chosen compiler. +- [ ]: When the .NET SDK detects a torn state + - [ ]: Use a `` to acquire the Microsoft.NETSdk.Compiler.Roslyn package + - [ ]: Change `$(RoslynTargetsPath)` to point into the package contents From b54fa35a6addde7d8f5a0ec44281a1b2ec81bb99 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Fri, 21 Jun 2024 09:07:13 -0700 Subject: [PATCH 04/20] more --- documentation/general/torn-sdk.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index eba61ae53504..4563f4fc5d35 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -37,6 +37,19 @@ Specifically we will be In addition to making our builds more reliable this will also massively simplify our analyzer development strategy. Analyzers following this model can always target the latest Roslyn version without the need for complicated multi-targeting. +## Motivations + +There are a number of scenarios where customers end up in a torn state. The most common is when customers have a CI setup with the following items: + +1. They use a task like [actions/setup-dotnet][setup-dotnet] to install the .NET SDK and use a flexible version format like `8.x`. +2. They use `msbuild` to actually drive their build. This `msbuild` comes from the Visual Studio installed on the machine image. + +This means that CI systems are updated to the latest .NET SDK virtually as soon as we release them. However the version of Visual Studio is updated much later as CI images usually take several weeks to upgrade to a new Visual Studio version. This is a very common CI setup and means a significant number of our customers end up in a torn state for several weeks. + +Another reason is that teams use older Visual Studio versions due to internal constraints: like an organizational policy. At the same time they install the latest .NET SDK which puts them into a torn state. + +This also hits any customer that uses a preview version of .NET SDK. These inherently represent a torn SDK state because they almost never match the compiler in Visual Studio This results in blockers for big teams like Bing from testing out our previews. + ## Goals This design has a number of goals: @@ -111,6 +124,7 @@ Instead of downloading a .NET Framework Roslyn in a torn state, the SDK could ju [matrix-of-paine]: https://aka.ms/dotnet/matrix-of-paine [pr-detect-torn-state]: https://github.com/dotnet/installer/pull/19144 [code-razor-vs-load]: https://github.com/dotnet/roslyn/blob/9aea80927e3d4e5a2846efaa710438c0d8d2bfa2/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProject.cs#L1009 +[setup-dotnet]: https://github.com/actions/setup-dotnet ## Work Breakdown From 677732c0b199bd17a59ffdb16890c080d227bfbb Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Sun, 23 Jun 2024 20:00:52 -0700 Subject: [PATCH 05/20] Apply suggestions from code review Co-authored-by: Rainer Sigwald --- documentation/general/torn-sdk.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index 4563f4fc5d35..cecef41a7a8a 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -72,13 +72,13 @@ One downside to this approach is that it is possible to end up with two VBCSComp Analyzers which ship in the .NET SDK box will change to having a copy checked into Visual Studio. When .NET SDK based projects are loaded at design time, the Visual Studio copy of the analyzer will be loaded. Roslyn already understands how to prefer Visual Studio copies of analyzers. That work will need to be extended a bit but that is pretty straight forward code. -This approach enables us to take actions like NGEN or R2R analyzers inside of Visual Studio. This is a long standing request from the Visual Studio perf team but very hard to satisfy in our current +This approach enables us to take actions like NGEN or R2R analyzers inside of Visual Studio. This is a long standing request from the Visual Studio perf team but very hard to satisfy with our current distribution strategy. -This approach is already taken by [the Razor generator][code-razor-vs-load]. This work was done for other reliability reasons but turned out working well for this scenario. There is initial upfront work to get the Visual Studio copy loading in design time builds but it has virtually zero maintanence cost. +This approach is already taken by [the Razor generator][code-razor-vs-load]. This work was done for other reliability reasons but turned out to work well for this scenario. There is initial upfront work to get the Visual Studio copy loading in design time builds but it has virtually zero maintenance cost. This also means these analyzers can vastly simplify their development model by always targeting the latest version of Roslyn. There is no more need to multi-target because the version of the compiler the analyzer will be used with is known at ship time for all core build scenarios. -This does mean that our design time experience can differ from our command line experience when customers are in a torn state. Specifically it's possible, even likely in some cases, that the diagonstics produced by design time builds will differ from command line builds. That is a trade off that we are willing to make for reliability. Customers who wish to have a consistent experience between design time should not operate in a torn state. +This does mean that our design time experience can differ from our command line experience when customers are in a torn state. Specifically it's possible, even likely in some cases, that the diagnostics produced by design time builds will differ from command line builds. That is a trade off that we are willing to make for reliability. Customers who wish to have a consistent experience between IDE and build should not operate in a torn state. ## .NET SDK in box analyzers target oldest @@ -106,11 +106,11 @@ Instead of `` the toolset package could be installed via `` step as the compiler is always there. This is a simpler build but it means that all .NET SDK installs on Windows will incease by ~6-7%. +An alternative is simply shipping both Roslyn compilers in the Windows .NET SDK. This means there is no need for a `` step as the compiler is always there. This is a simpler build but it means that all .NET SDK installs on Windows will increase by ~6-7%. ### Use the .NET Core compiler -Instead of downloading a .NET Framework Roslyn in a torn state, the SDK could just use the .NET Core Roslyn it comes with. That would have virtually no real compatibility issues. This has a number of disadvantages: +Instead of downloading a .NET Framework Roslyn in a torn state, the SDK could use the .NET Core Roslyn it comes with. That would have virtually no real compatibility issues. This has a number of disadvantages: 1. Changes the process name from VBCSCompiler to `dotnet`. That seems insignificant but the process name is important as it's referenced in a lot of build scripts. This would be a subtle breaking change that we'd have to work with customers on. 2. This would make Visual Studio dependent on the servicing lifetime of the .NET Core runtime in the SDK. That has a very different lifetime and could lead to surprising outcomes down the road. For example if policy removed out of support runtimes from machines it would break the Visual Studio build. From 9a0e431f2f14ebcbe0715e1c8c927aa5214871b9 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Sun, 23 Jun 2024 20:01:02 -0700 Subject: [PATCH 06/20] PR feedback --- documentation/general/torn-sdk.md | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index cecef41a7a8a..8578389616ae 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -22,6 +22,17 @@ Generally this mixing of components is fine because Visual Studio will install a generators\Microsoft.CodeAnalysis.Razor.Compiler.SourceGenerators.dll' references version '4.9.0.0' of the compiler, which is newer than the currently running version '4.8.0.0'. +This torn state is a common customer setup: + +|Visual Studio Version | Match% | Float % | Torn % | +| --- | --- | --- | --- | +| 16.11 | 81.4% | 10.8% | 7.8% | +| 17.2 | 91% | 8% | 1% | +| 17.4 | 91.5% | 5.7% | 2.7% | +| 17.6 | 91.6% | 5% | 3.5% | +| 17.7 | 94.9% | 2.2% | 3% | +| 17.8 | 96% | 0% | 4% | + To address this issue we are going to separate Visual Studio and the .NET SDK in build scenarios. That will change our matrix to the following: | Scenario | Loads Roslyn | Loads Analyzers / Generators | @@ -35,7 +46,7 @@ Specifically we will be 1. Changing msbuild to use a compiler from the .NET SDK 2. Changing Visual Studio to use analyzers from Visual Studio -In addition to making our builds more reliable this will also massively simplify our analyzer development strategy. Analyzers following this model can always target the latest Roslyn version without the need for complicated multi-targeting. +In addition to making our builds more reliable this will also massively simplify our [analyzer Development strategy][sdk-lifecycle]. Analyzers following this model can always target the latest Roslyn version without the need for complicated multi-targeting. ## Motivations @@ -60,11 +71,11 @@ This design has a number of goals: ## MSBuild using .NET SDK Compiler -The .NET SDK will start producing a package named Microsoft.NETSdk.Compiler.Roslyn. This will contain a .NET Framework version of the Roslyn compiler that matches .NET Core version. This will be published as a part of the .NET SDK release process meaning it's available for all publicly available .NET SDKs. +The .NET SDK will start producing a package named Microsoft.Net.Sdk.Compilers.Toolset. This will contain a .NET Framework version of the Roslyn compiler that matches .NET Core version. This will be published as a part of the .NET SDK release process meaning it's available for all publicly available .NET SDKs. -The .NET SDK has the capability to [detect a torn state][pr-detect-torn-state]. When this is detected the matching Microsoft.NETSDK.Compiler.Roslyn package for this version of the .NET SDK will be downloaded via ``. Then the `$(RoslynTargetsPath)` will be reset to point into the package contents. This will cause MSBuild to load the Roslyn compiler from that location vs. the Visual Studio location. +The .NET SDK has the capability to [detect a torn state][pr-detect-torn-state]. When this is detected the matching Microsoft.Net.Sdk.Compilers.Toolset package for this version of the .NET SDK will be downloaded via ``. Then the `$(RoslynTargetsPath)` will be reset to point into the package contents. This will cause MSBuild to load the Roslyn compiler from that location vs. the Visual Studio location. -One downside to this approach is that it is possible to end up with two VBCSCompiler server processes. Consider that when a solution has a mix of .NET SDK style projects and legacy projects then in a torn state the .NET SDK projects will use the .NET SDK compiler will legacy projects will use the Visual Studio compiler. While not a desirable outcome, it is a correct one. Customers who wish to only have one VBCSCompiler should correct the torn state in their build tools. +One downside to this approach is that it is possible to end up with two VBCSCompiler server processes. Consider a solution that has a mix of .NET SDK style projects and non-SDK .NET projects. In a torn the .NET SDK projects will use the .NET SDK compiler and non-SDK .NET projects will use the Visual Studio compiler. While not a desirable outcome, it is a correct one. Customers who wish to only have one VBCSCompiler should correct the torn state in their build tools. ## Visual Studio using Visual Studio Analyzers @@ -117,11 +128,13 @@ Instead of downloading a .NET Framework Roslyn in a torn state, the SDK could us ## Related Issues -- https://github.com/dotnet/roslyn/issues/72672 -- https://github.com/dotnet/installer/pull/19144 +- [Roslyn tracking issue for torn state](https://github.com/dotnet/roslyn/issues/72672) +- [MSBuild property for torn state detection](https://github.com/dotnet/installer/pull/19144) +- [Long term build-server shutdown issue](https://github.com/dotnet/msbuild/issues/10035) [microsoft-common-tasks]: https://github.com/dotnet/msbuild/blob/main/src/Tasks/Microsoft.Common.tasks#L106-L109 [matrix-of-paine]: https://aka.ms/dotnet/matrix-of-paine +[sdk-lifecycle]: https://learn.microsoft.com/en-us/dotnet/core/porting/versioning-sdk-msbuild-vs#lifecycle [pr-detect-torn-state]: https://github.com/dotnet/installer/pull/19144 [code-razor-vs-load]: https://github.com/dotnet/roslyn/blob/9aea80927e3d4e5a2846efaa710438c0d8d2bfa2/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProject.cs#L1009 [setup-dotnet]: https://github.com/actions/setup-dotnet @@ -131,12 +144,12 @@ Instead of downloading a .NET Framework Roslyn in a torn state, the SDK could us This will be moved to an issue when this PR is opened against dotnet/sdk proper: - [ ]: Flow the Micosoft.Net.Compilers.Toolset.Framework package to the .NET SDK -- [ ]: Create a new package Micosoft.NETSdk.Compiler.Roslyn in .NET SDK. +- [ ]: Create a new package Micosoft.Microsoft.Net.Sdk.Compilers.Toolset in .NET SDK. - [ ]: The contents of this package will include the contents of the `tasks\net472` folder in the Microsoft.Net.Compilers.Toolset.Framework package. This subset is all that is needed and makes the package not usable via ``. The latter reduces the incentive for customers to use it directly. - [ ]: The contents will include a README.md stating the package is **not** supported for direct user consumption. - [ ]: The package will follow the versioning scheme of the .NET SDK. - [ ]: The package will be unlisted (ideal but not a hard requirement) - [ ]: Change the Sdk.targets file to have copies of the following three `` from [Microsoft.Common.tasks][microsoft-common-tasks]. Having a copy in Sdk.targets means that resetting `$(RoslynTargetsPath)` during build will change the chosen compiler. - [ ]: When the .NET SDK detects a torn state - - [ ]: Use a `` to acquire the Microsoft.NETSdk.Compiler.Roslyn package + - [ ]: Use a `` to acquire the Microsoft.Net.Sdk.Compilers.Toolset package - [ ]: Change `$(RoslynTargetsPath)` to point into the package contents From c799879796e1aa9d5b955d8bcbfbd0f58c6c6a06 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Sun, 23 Jun 2024 20:26:29 -0700 Subject: [PATCH 07/20] PR feedback --- documentation/general/torn-sdk.md | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index 8578389616ae..50bed0a50382 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -83,13 +83,13 @@ One downside to this approach is that it is possible to end up with two VBCSComp Analyzers which ship in the .NET SDK box will change to having a copy checked into Visual Studio. When .NET SDK based projects are loaded at design time, the Visual Studio copy of the analyzer will be loaded. Roslyn already understands how to prefer Visual Studio copies of analyzers. That work will need to be extended a bit but that is pretty straight forward code. -This approach enables us to take actions like NGEN or R2R analyzers inside of Visual Studio. This is a long standing request from the Visual Studio perf team but very hard to satisfy with our current distribution strategy. +This approach enables us to take actions like NGEN or R2R analyzers inside of Visual Studio. This is a long standing request from the Visual Studio perf team but very hard to satisfy in our current -This approach is already taken by [the Razor generator][code-razor-vs-load]. This work was done for other reliability reasons but turned out to work well for this scenario. There is initial upfront work to get the Visual Studio copy loading in design time builds but it has virtually zero maintenance cost. +This approach is already taken by [the Razor generator][code-razor-vs-load]. This work was done for other reliability reasons but turned out working well for this scenario. There is initial upfront work to get the Visual Studio copy loading in design time builds but it has virtually zero maintanence cost. This also means these analyzers can vastly simplify their development model by always targeting the latest version of Roslyn. There is no more need to multi-target because the version of the compiler the analyzer will be used with is known at ship time for all core build scenarios. -This does mean that our design time experience can differ from our command line experience when customers are in a torn state. Specifically it's possible, even likely in some cases, that the diagnostics produced by design time builds will differ from command line builds. That is a trade off that we are willing to make for reliability. Customers who wish to have a consistent experience between IDE and build should not operate in a torn state. +This does mean that our design time experience can differ from our command line experience when customers are in a torn state. Specifically it's possible, even likely in some cases, that the diagonstics produced by design time builds will differ from command line builds. That is a trade off that we are willing to make for reliability. Customers who wish to have a consistent experience between design time should not operate in a torn state. ## .NET SDK in box analyzers target oldest @@ -101,6 +101,8 @@ This strategy should be considered short term. The Roslyn API is under constant Analyzers which ship via NuGet will continue to following the existing [support policies][matrix-of-paine]. This means that they will either need to target a sufficiently old version of Roslyn or implement multi-targeting support. +In the case of [multi-targeting][issue-analyzer-mt] this proposal is effectively a no-op. Analyzer multi-targeting is already based off of Roslyn versions, not .NET SDK versions, hence the proper version of the analyzer will still load in a torn state. The version of the Roslyn compiler in a torn state for msbuild is the same used for dotnet build hence it's already a scenario the analyzer needs to support. + ## Alternative ### .NET SDK in box analyzers multi-target @@ -112,16 +114,21 @@ Technically in box analyzers can have a multi-targeting strategy just as NuGet b Instead of `` the toolset package could be installed via ``. This is how the existing Microsoft.Net.Compilers.Toolset package works today. This has a number of disadvantages: 1. PackageReferences are not side effect free and have been shown to cause issues in complex builds. For example this package did not work by default in the Visual Studio or Bing builds. Deploying this at scale to all our customers will almost ceratinly cause issues. -2. This approach does not solve all scenarios. The mechanics of restore mean that the compiler is not swapped until part way through the build. Build operations that happen earlier, such as legacy WPF, will still use the Visual Studio compiler and hence experience analyzer compatibility issues. +2. This approach does not solve all scenarios. The mechanics of restore mean that the compiler is not swapped until part way through the build. Build operations that happen earlier, such as [legacy WPF][issue-legacy-wpf], will still use the Visual Studio compiler and hence experience analyzer compatibility issues. 3. Mixing builds that do and do not restore can lead to different build outcomes as they can end up choosing different compilers. Mixing the version of msbuild / dotnet msbuild can also lead to strange outcomes as they can end up choosing different compilers. ### Install the Framework compiler -An alternative is simply shipping both Roslyn compilers in the Windows .NET SDK. This means there is no need for a `` step as the compiler is always there. This is a simpler build but it means that all .NET SDK installs on Windows will increase by ~6-7%. +An alternative is simply shipping both Roslyn compilers in the Windows .NET SDK. This means there is no need for a `` step as the compiler is always there. This is a simpler build but it means that all .NET SDK installs on Windows will incease by ~6-7%. + +This approach does have downsides: + +1. The Windows .NET SDK increased in size even when it's not needed. For example our main Windows containers do not install Visual Studio hence are never in a torn state yet they'll see increased size cost here. Changing the .NET SDK such that these extra compilers are only installed sometimes would add significant complexity to our installer setup. +2. This will make the Windows and Linux .NET SDK's visibly different in layout. Comparing them will show different contents which could be confusing to customers. At the same time the current proposal is functionally the same, it just changes where the framework compilers are placed on disk and that also could be confusing to customers. ### Use the .NET Core compiler -Instead of downloading a .NET Framework Roslyn in a torn state, the SDK could use the .NET Core Roslyn it comes with. That would have virtually no real compatibility issues. This has a number of disadvantages: +Instead of downloading a .NET Framework Roslyn in a torn state, the SDK could just use the .NET Core Roslyn it comes with. That would have virtually no real compatibility issues. This has a number of disadvantages: 1. Changes the process name from VBCSCompiler to `dotnet`. That seems insignificant but the process name is important as it's referenced in a lot of build scripts. This would be a subtle breaking change that we'd have to work with customers on. 2. This would make Visual Studio dependent on the servicing lifetime of the .NET Core runtime in the SDK. That has a very different lifetime and could lead to surprising outcomes down the road. For example if policy removed out of support runtimes from machines it would break the Visual Studio build. @@ -135,9 +142,11 @@ Instead of downloading a .NET Framework Roslyn in a torn state, the SDK could us [microsoft-common-tasks]: https://github.com/dotnet/msbuild/blob/main/src/Tasks/Microsoft.Common.tasks#L106-L109 [matrix-of-paine]: https://aka.ms/dotnet/matrix-of-paine [sdk-lifecycle]: https://learn.microsoft.com/en-us/dotnet/core/porting/versioning-sdk-msbuild-vs#lifecycle -[pr-detect-torn-state]: https://github.com/dotnet/installer/pull/19144 [code-razor-vs-load]: https://github.com/dotnet/roslyn/blob/9aea80927e3d4e5a2846efaa710438c0d8d2bfa2/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProject.cs#L1009 [setup-dotnet]: https://github.com/actions/setup-dotnet +[pr-detect-torn-state]: https://github.com/dotnet/installer/pull/19144 +[issue-analyzer-mt]: https://github.com/dotnet/sdk/issues/20355 +[issue-legacy-wpf]: https://github.com/dotnet/wpf/issues/9112 ## Work Breakdown From 4ce624dea492c0a50fe95c79a16a5e3df97136fb Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Mon, 24 Jun 2024 07:48:15 -0700 Subject: [PATCH 08/20] Apply suggestions from code review Co-authored-by: Jan Jones --- documentation/general/torn-sdk.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index 50bed0a50382..d0e8ad9cb7e7 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -59,7 +59,7 @@ This means that CI systems are updated to the latest .NET SDK virtually as soon Another reason is that teams use older Visual Studio versions due to internal constraints: like an organizational policy. At the same time they install the latest .NET SDK which puts them into a torn state. -This also hits any customer that uses a preview version of .NET SDK. These inherently represent a torn SDK state because they almost never match the compiler in Visual Studio This results in blockers for big teams like Bing from testing out our previews. +This also hits any customer that uses a preview version of .NET SDK. These inherently represent a torn SDK state because they almost never match the compiler in Visual Studio. This results in blockers for big teams like Bing from testing out our previews. ## Goals @@ -85,11 +85,11 @@ Analyzers which ship in the .NET SDK box will change to having a copy checked in This approach enables us to take actions like NGEN or R2R analyzers inside of Visual Studio. This is a long standing request from the Visual Studio perf team but very hard to satisfy in our current -This approach is already taken by [the Razor generator][code-razor-vs-load]. This work was done for other reliability reasons but turned out working well for this scenario. There is initial upfront work to get the Visual Studio copy loading in design time builds but it has virtually zero maintanence cost. +This approach is already taken by [the Razor generator][code-razor-vs-load]. This work was done for other reliability reasons but turned out working well for this scenario. There is initial upfront work to get the Visual Studio copy loading in design time builds but it has virtually zero maintenance cost. This also means these analyzers can vastly simplify their development model by always targeting the latest version of Roslyn. There is no more need to multi-target because the version of the compiler the analyzer will be used with is known at ship time for all core build scenarios. -This does mean that our design time experience can differ from our command line experience when customers are in a torn state. Specifically it's possible, even likely in some cases, that the diagonstics produced by design time builds will differ from command line builds. That is a trade off that we are willing to make for reliability. Customers who wish to have a consistent experience between design time should not operate in a torn state. +This does mean that our design time experience can differ from our command line experience when customers are in a torn state. Specifically it's possible, even likely in some cases, that the diagnostics produced by design time builds will differ from command line builds. That is a trade off that we are willing to make for reliability. Customers who wish to have a consistent experience between design time should not operate in a torn state. ## .NET SDK in box analyzers target oldest @@ -113,13 +113,13 @@ Technically in box analyzers can have a multi-targeting strategy just as NuGet b Instead of `` the toolset package could be installed via ``. This is how the existing Microsoft.Net.Compilers.Toolset package works today. This has a number of disadvantages: -1. PackageReferences are not side effect free and have been shown to cause issues in complex builds. For example this package did not work by default in the Visual Studio or Bing builds. Deploying this at scale to all our customers will almost ceratinly cause issues. +1. PackageReferences are not side effect free and have been shown to cause issues in complex builds. For example this package did not work by default in the Visual Studio or Bing builds. Deploying this at scale to all our customers will almost certainly cause issues. 2. This approach does not solve all scenarios. The mechanics of restore mean that the compiler is not swapped until part way through the build. Build operations that happen earlier, such as [legacy WPF][issue-legacy-wpf], will still use the Visual Studio compiler and hence experience analyzer compatibility issues. 3. Mixing builds that do and do not restore can lead to different build outcomes as they can end up choosing different compilers. Mixing the version of msbuild / dotnet msbuild can also lead to strange outcomes as they can end up choosing different compilers. ### Install the Framework compiler -An alternative is simply shipping both Roslyn compilers in the Windows .NET SDK. This means there is no need for a `` step as the compiler is always there. This is a simpler build but it means that all .NET SDK installs on Windows will incease by ~6-7%. +An alternative is simply shipping both Roslyn compilers in the Windows .NET SDK. This means there is no need for a `` step as the compiler is always there. This is a simpler build but it means that all .NET SDK installs on Windows will increase by ~6-7%. This approach does have downsides: From aa0d435c5a1e49cf4fb9c9a48f42d4ec9b310dbd Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Tue, 25 Jun 2024 08:48:56 -0700 Subject: [PATCH 09/20] DevKit discussion --- documentation/general/torn-sdk.md | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index d0e8ad9cb7e7..c30dc9eeab1c 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -133,7 +133,24 @@ Instead of downloading a .NET Framework Roslyn in a torn state, the SDK could ju 1. Changes the process name from VBCSCompiler to `dotnet`. That seems insignificant but the process name is important as it's referenced in a lot of build scripts. This would be a subtle breaking change that we'd have to work with customers on. 2. This would make Visual Studio dependent on the servicing lifetime of the .NET Core runtime in the SDK. That has a very different lifetime and could lead to surprising outcomes down the road. For example if policy removed out of support runtimes from machines it would break the Visual Studio build. -## Related Issues +## Misc + +### Visual Studio Code + +This is how Visual Studio Code fits into our matrix: + +| Scenario | Loads Roslyn | Loads Analyzers / Generators | +| --- | --- | --- | +| msbuild | From Visual Studio | From .NET SDK | +| dotnet msbuild | From .NET SDK | From .NET SDK | +| Visual Studio Design Time | From Visual Studio | From Both | +| DevKit | From DevKit | From .NET SDK | + +On the surface it seems like VS Code has the same issues as Visual Studio does today. However this is not the case. Visual Studio is problematic because at any given time there can be ~5 different versions in active support each with a different version of the compiler. Every Visual Studio but the latest is an older compiler that run into issues with analyzers. + +There is only one version of the DevKit extension. It is released using the latest compilers from roslyn at a very high frequency. That frequency is so high that the chance it has an older compiler than the newest .NET SDK is virtually zero. That means at the moment there is no need to solve the problem here. If DevKit changes its release cadance in the future this may need to be revisited. + +### Related Issues - [Roslyn tracking issue for torn state](https://github.com/dotnet/roslyn/issues/72672) - [MSBuild property for torn state detection](https://github.com/dotnet/installer/pull/19144) From b4e011d9b89d97cf912593f44724447a691953da Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Tue, 25 Jun 2024 08:50:02 -0700 Subject: [PATCH 10/20] prepare for PR --- documentation/general/torn-sdk.md | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index c30dc9eeab1c..fdbaa6e19477 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -164,18 +164,3 @@ There is only one version of the DevKit extension. It is released using the late [pr-detect-torn-state]: https://github.com/dotnet/installer/pull/19144 [issue-analyzer-mt]: https://github.com/dotnet/sdk/issues/20355 [issue-legacy-wpf]: https://github.com/dotnet/wpf/issues/9112 - -## Work Breakdown - -This will be moved to an issue when this PR is opened against dotnet/sdk proper: - -- [ ]: Flow the Micosoft.Net.Compilers.Toolset.Framework package to the .NET SDK -- [ ]: Create a new package Micosoft.Microsoft.Net.Sdk.Compilers.Toolset in .NET SDK. - - [ ]: The contents of this package will include the contents of the `tasks\net472` folder in the Microsoft.Net.Compilers.Toolset.Framework package. This subset is all that is needed and makes the package not usable via ``. The latter reduces the incentive for customers to use it directly. - - [ ]: The contents will include a README.md stating the package is **not** supported for direct user consumption. - - [ ]: The package will follow the versioning scheme of the .NET SDK. - - [ ]: The package will be unlisted (ideal but not a hard requirement) -- [ ]: Change the Sdk.targets file to have copies of the following three `` from [Microsoft.Common.tasks][microsoft-common-tasks]. Having a copy in Sdk.targets means that resetting `$(RoslynTargetsPath)` during build will change the chosen compiler. -- [ ]: When the .NET SDK detects a torn state - - [ ]: Use a `` to acquire the Microsoft.Net.Sdk.Compilers.Toolset package - - [ ]: Change `$(RoslynTargetsPath)` to point into the package contents From 61e93f4b3b67fb493a80d3d5f04a37cb57472f52 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Tue, 25 Jun 2024 09:02:15 -0700 Subject: [PATCH 11/20] PR feedback --- documentation/general/torn-sdk.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index fdbaa6e19477..eef355df2aa8 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -83,7 +83,7 @@ One downside to this approach is that it is possible to end up with two VBCSComp Analyzers which ship in the .NET SDK box will change to having a copy checked into Visual Studio. When .NET SDK based projects are loaded at design time, the Visual Studio copy of the analyzer will be loaded. Roslyn already understands how to prefer Visual Studio copies of analyzers. That work will need to be extended a bit but that is pretty straight forward code. -This approach enables us to take actions like NGEN or R2R analyzers inside of Visual Studio. This is a long standing request from the Visual Studio perf team but very hard to satisfy in our current +This approach enables us to take actions like NGEN or R2R analyzers inside of Visual Studio. This is a long standing request from the Visual Studio perf team but very hard to satisfy in our current setup. This approach is already taken by [the Razor generator][code-razor-vs-load]. This work was done for other reliability reasons but turned out working well for this scenario. There is initial upfront work to get the Visual Studio copy loading in design time builds but it has virtually zero maintenance cost. @@ -157,7 +157,7 @@ There is only one version of the DevKit extension. It is released using the late - [Long term build-server shutdown issue](https://github.com/dotnet/msbuild/issues/10035) [microsoft-common-tasks]: https://github.com/dotnet/msbuild/blob/main/src/Tasks/Microsoft.Common.tasks#L106-L109 -[matrix-of-paine]: https://aka.ms/dotnet/matrix-of-paine +[matrix-of-paine]: https://aka.ms/dotnet/matrixofpaine [sdk-lifecycle]: https://learn.microsoft.com/en-us/dotnet/core/porting/versioning-sdk-msbuild-vs#lifecycle [code-razor-vs-load]: https://github.com/dotnet/roslyn/blob/9aea80927e3d4e5a2846efaa710438c0d8d2bfa2/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProject.cs#L1009 [setup-dotnet]: https://github.com/actions/setup-dotnet From 2e32903f1a6e2dd3e7489cd3f1170296701dc829 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Tue, 25 Jun 2024 10:44:57 -0700 Subject: [PATCH 12/20] Apply suggestions from code review Co-authored-by: Chris Sienkiewicz --- documentation/general/torn-sdk.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index eef355df2aa8..f5bbf6f392ff 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -75,7 +75,7 @@ The .NET SDK will start producing a package named Microsoft.Net.Sdk.Compilers.To The .NET SDK has the capability to [detect a torn state][pr-detect-torn-state]. When this is detected the matching Microsoft.Net.Sdk.Compilers.Toolset package for this version of the .NET SDK will be downloaded via ``. Then the `$(RoslynTargetsPath)` will be reset to point into the package contents. This will cause MSBuild to load the Roslyn compiler from that location vs. the Visual Studio location. -One downside to this approach is that it is possible to end up with two VBCSCompiler server processes. Consider a solution that has a mix of .NET SDK style projects and non-SDK .NET projects. In a torn the .NET SDK projects will use the .NET SDK compiler and non-SDK .NET projects will use the Visual Studio compiler. While not a desirable outcome, it is a correct one. Customers who wish to only have one VBCSCompiler should correct the torn state in their build tools. +One downside to this approach is that it is possible to end up with two VBCSCompiler server processes. Consider a solution that has a mix of .NET SDK style projects and non-SDK .NET projects. In a torn state the .NET SDK projects will use the .NET SDK compiler and non-SDK .NET projects will use the Visual Studio compiler. While not a desirable outcome, it is a correct one. Customers who wish to only have one VBCSCompiler should correct the torn state in their build tools. ## Visual Studio using Visual Studio Analyzers From bc7b2340719592a71098e1491491aae2a0948c37 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Tue, 25 Jun 2024 14:34:22 -0700 Subject: [PATCH 13/20] Apply suggestions from code review Co-authored-by: Sam Harwell --- documentation/general/torn-sdk.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index f5bbf6f392ff..923f8320726f 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -2,9 +2,9 @@ ## Terminology -- msbuild: refers the .NET Framework based msbuild included with Visual Studio. -- dotnet msbuild: refers to the .NET Core based msbuild included with the .NET SDK. -- analyzers: this document will use analyzers to refer to both analyzers and generators. +- **msbuild**: refers the .NET Framework based msbuild included with Visual Studio. +- **dotnet msbuild**: refers to the .NET Core based msbuild included with the .NET SDK. +- **analyzers**: this document will use analyzers to refer to both analyzers and generators. ## Summary @@ -19,8 +19,8 @@ Visual Studio and .NET SDK are separate products but they are intertwined in com Generally this mixing of components is fine because Visual Studio will install a .NET SDK that is functionally paired with it. That is the compiler and analyzers are the same hence mixing poses no real issue. For example 17.10 installs .NET SDK 8.0.3xx, 17.9 installs .NET SDK 8.0.2xx, etc ... However when the .NET SDK is not paired with the Visual Studio version,then compatibility issues can, and will, arise. Particularly when the .NET SDK is _newer_ than Visual Studio customers will end up with the following style of error: > CSC : warning CS9057: The analyzer assembly '..\dotnet\sdk\8.0.200\Sdks\Microsoft.NET.Sdk.Razor\source- -generators\Microsoft.CodeAnalysis.Razor.Compiler.SourceGenerators.dll' references version '4.9.0.0' of the compiler, which is newer -than the currently running version '4.8.0.0'. +> generators\Microsoft.CodeAnalysis.Razor.Compiler.SourceGenerators.dll' references version '4.9.0.0' of the compiler, which is newer +> than the currently running version '4.8.0.0'. This torn state is a common customer setup: From 4b777164ae891bb665c48423445dbd23ac027213 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Fri, 28 Jun 2024 10:22:23 -0700 Subject: [PATCH 14/20] Apply suggestions from code review Co-authored-by: Sam Harwell --- documentation/general/torn-sdk.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index 923f8320726f..40c1e9dfce9e 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -46,7 +46,7 @@ Specifically we will be 1. Changing msbuild to use a compiler from the .NET SDK 2. Changing Visual Studio to use analyzers from Visual Studio -In addition to making our builds more reliable this will also massively simplify our [analyzer Development strategy][sdk-lifecycle]. Analyzers following this model can always target the latest Roslyn version without the need for complicated multi-targeting. +In addition to making our builds more reliable this will also massively simplify our [analyzer Development strategy][sdk-lifecycle]. Analyzers in the SDK following this model can always target the latest Roslyn version without the need for complicated multi-targeting. ## Motivations From 0bfab37abb37a28dcd3fd0f2a02384f28c8d27e6 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Fri, 28 Jun 2024 10:25:47 -0700 Subject: [PATCH 15/20] more --- documentation/general/torn-sdk.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index 40c1e9dfce9e..0f59975656ee 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -8,7 +8,7 @@ ## Summary -Visual Studio and .NET SDK are separate products but they are intertwined in command line and design time build scenarios as different components are loaded from each product. +Visual Studio and .NET SDK are separate products but they are intertwined in command line and design time build scenarios as different components are loaded from each product. This table represents how the products currently function: | Scenario | Loads Roslyn | Loads Analyzers / Generators | | --- | --- | --- | @@ -109,6 +109,10 @@ In the case of [multi-targeting][issue-analyzer-mt] this proposal is effectively Technically in box analyzers can have a multi-targeting strategy just as NuGet based analyzers do. This is actively discouraged because it leads to unnecessary increases in .NET SDK sizes. The time spent implementing multi-targeting is likely better spent moving to a dual insertion to keep .NET SDK sizes down and provide a consistent experience with other analyzers. +### .NET SDK in box analyzers use light up + +In box could also employ a light up strategy. Essentially reference older versions of the compiler and use reflection to opportunistically discover + use newer APIs. This is successfully employed in analyzers like StyleCop. + ### PackageReference Instead of `` the toolset package could be installed via ``. This is how the existing Microsoft.Net.Compilers.Toolset package works today. This has a number of disadvantages: From 8d49f434e523b171acee194a1c75767a45118a84 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Fri, 28 Jun 2024 10:28:52 -0700 Subject: [PATCH 16/20] more --- documentation/general/torn-sdk.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index 0f59975656ee..92e8890746a0 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -79,6 +79,8 @@ One downside to this approach is that it is possible to end up with two VBCSComp ## Visual Studio using Visual Studio Analyzers +Analyzers are required to function in all of our currently supported Visual Studio versions. This design proposal does not change this requirement. Instead it seeks to simplify the set of scenarios that analyzers need to consider by making compiler versions more predictable. It also helps establish a simple path for inbox analyzers to simply use the latest Roslyn in their development. + ### .NET SDK in box analyzers dual insert Analyzers which ship in the .NET SDK box will change to having a copy checked into Visual Studio. When .NET SDK based projects are loaded at design time, the Visual Studio copy of the analyzer will be loaded. Roslyn already understands how to prefer Visual Studio copies of analyzers. That work will need to be extended a bit but that is pretty straight forward code. @@ -141,13 +143,13 @@ Instead of downloading a .NET Framework Roslyn in a torn state, the SDK could ju ### Visual Studio Code -This is how Visual Studio Code fits into our matrix: +This is how Visual Studio Code fits into our matrix after this work is complete: | Scenario | Loads Roslyn | Loads Analyzers / Generators | | --- | --- | --- | -| msbuild | From Visual Studio | From .NET SDK | +| msbuild | From .NET SDK | From .NET SDK | | dotnet msbuild | From .NET SDK | From .NET SDK | -| Visual Studio Design Time | From Visual Studio | From Both | +| Visual Studio Design Time | From Visual Studio | From Visuaal Studio | | DevKit | From DevKit | From .NET SDK | On the surface it seems like VS Code has the same issues as Visual Studio does today. However this is not the case. Visual Studio is problematic because at any given time there can be ~5 different versions in active support each with a different version of the compiler. Every Visual Studio but the latest is an older compiler that run into issues with analyzers. From 8a2a7d01c3d3f060d5812424a9de8a00d70b3061 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Mon, 8 Jul 2024 06:35:26 -0700 Subject: [PATCH 17/20] Apply suggestions from code review Co-authored-by: Jan Jones --- documentation/general/torn-sdk.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index 92e8890746a0..440ebbc039f0 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -149,7 +149,7 @@ This is how Visual Studio Code fits into our matrix after this work is complete: | --- | --- | --- | | msbuild | From .NET SDK | From .NET SDK | | dotnet msbuild | From .NET SDK | From .NET SDK | -| Visual Studio Design Time | From Visual Studio | From Visuaal Studio | +| Visual Studio Design Time | From Visual Studio | From Visual Studio | | DevKit | From DevKit | From .NET SDK | On the surface it seems like VS Code has the same issues as Visual Studio does today. However this is not the case. Visual Studio is problematic because at any given time there can be ~5 different versions in active support each with a different version of the compiler. Every Visual Studio but the latest is an older compiler that run into issues with analyzers. From 5a9793500761d541608b34110e9e3390447555e0 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Thu, 8 Aug 2024 05:28:21 -0700 Subject: [PATCH 18/20] More --- documentation/general/torn-sdk.md | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index 440ebbc039f0..dc3ca082bbc8 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -33,7 +33,7 @@ This torn state is a common customer setup: | 17.7 | 94.9% | 2.2% | 3% | | 17.8 | 96% | 0% | 4% | -To address this issue we are going to separate Visual Studio and the .NET SDK in build scenarios. That will change our matrix to the following: +To address this issue we are going to separate Visual Studio and the .NET SDK in build scenarios. That will change our matrix to _logically_ be the following: | Scenario | Loads Roslyn | Loads Analyzers / Generators | | --- | --- | --- | @@ -43,7 +43,7 @@ To address this issue we are going to separate Visual Studio and the .NET SDK in Specifically we will be -1. Changing msbuild to use a compiler from the .NET SDK +1. Changing msbuild to use a compiler from the .NET SDK when msbuild's compiler is older 2. Changing Visual Studio to use analyzers from Visual Studio In addition to making our builds more reliable this will also massively simplify our [analyzer Development strategy][sdk-lifecycle]. Analyzers in the SDK following this model can always target the latest Roslyn version without the need for complicated multi-targeting. @@ -65,7 +65,7 @@ This also hits any customer that uses a preview version of .NET SDK. These inher This design has a number of goals: -1. The `msbuild` and `dotnet build` build experience should be equivalent. +1. The `msbuild` and `dotnet msbuild` build experience should be equivalent. 1. The Visual Studio Design time experience is independent of the .NET SDK installed. 1. To make explicit that it is okay, and even expected, that the design time and command line build experiences can differ when the SDK is in a torn state. @@ -73,7 +73,7 @@ This design has a number of goals: The .NET SDK will start producing a package named Microsoft.Net.Sdk.Compilers.Toolset. This will contain a .NET Framework version of the Roslyn compiler that matches .NET Core version. This will be published as a part of the .NET SDK release process meaning it's available for all publicly available .NET SDKs. -The .NET SDK has the capability to [detect a torn state][pr-detect-torn-state]. When this is detected the matching Microsoft.Net.Sdk.Compilers.Toolset package for this version of the .NET SDK will be downloaded via ``. Then the `$(RoslynTargetsPath)` will be reset to point into the package contents. This will cause MSBuild to load the Roslyn compiler from that location vs. the Visual Studio location. +The .NET SDK has the capability to [detect a torn state][pr-detect-torn-state]. When this is detected and msbuild's compiler is older, the matching Microsoft.Net.Sdk.Compilers.Toolset package for this version of the .NET SDK will be downloaded via ``. Then the `$(RoslynTargetsPath)` will be reset to point into the package contents. This will cause MSBuild to load the Roslyn compiler from that location vs. the Visual Studio location. One downside to this approach is that it is possible to end up with two VBCSCompiler server processes. Consider a solution that has a mix of .NET SDK style projects and non-SDK .NET projects. In a torn state the .NET SDK projects will use the .NET SDK compiler and non-SDK .NET projects will use the Visual Studio compiler. While not a desirable outcome, it is a correct one. Customers who wish to only have one VBCSCompiler should correct the torn state in their build tools. @@ -156,13 +156,22 @@ On the surface it seems like VS Code has the same issues as Visual Studio does t There is only one version of the DevKit extension. It is released using the latest compilers from roslyn at a very high frequency. That frequency is so high that the chance it has an older compiler than the newest .NET SDK is virtually zero. That means at the moment there is no need to solve the problem here. If DevKit changes its release cadance in the future this may need to be revisited. +### Why not always use Micosoft.Net.Sdk.Compilers.Toolset? + +This design puts us in a state where `msbuild` will sometimes use the compiler from the .NET and sometimes use it from the MSBuild install. One may ask it why not simplify this and always load from the .NET SDK? Essentially why have all this complexity around torn state detection instead of just always using Microsoft.Net.Sdk.Compilers.Toolset? Or always use it when the compiler versions are different, not just when the .NET SDK is newer than Visual Studio. + +The main reason is performance. The compiler inside MSBuild is NGEN'd while the compiler inside Microosft.Net.Sdk.Compilers.Toolset is not. That means there is a small initial startup penalty when loading the compiler from the package. The VBCSCompiler server process largely amortizes that penalty though. Our expectation is that this difference will be largely unnoticeable to customers. + +The second reason is that this introduces a new `PackageDownload` step into the build. Doing that always carries some amount of risk. By scoping to only when the `msbuild` compiler is older we reduce the scope of this potential problem. + +We plan to revisit this decision in future releases of the .NET SDK as we get real world experience with this change and the customer feedback that comes from it. + ### Related Issues - [Roslyn tracking issue for torn state](https://github.com/dotnet/roslyn/issues/72672) - [MSBuild property for torn state detection](https://github.com/dotnet/installer/pull/19144) - [Long term build-server shutdown issue](https://github.com/dotnet/msbuild/issues/10035) -[microsoft-common-tasks]: https://github.com/dotnet/msbuild/blob/main/src/Tasks/Microsoft.Common.tasks#L106-L109 [matrix-of-paine]: https://aka.ms/dotnet/matrixofpaine [sdk-lifecycle]: https://learn.microsoft.com/en-us/dotnet/core/porting/versioning-sdk-msbuild-vs#lifecycle [code-razor-vs-load]: https://github.com/dotnet/roslyn/blob/9aea80927e3d4e5a2846efaa710438c0d8d2bfa2/src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProject.cs#L1009 From 6b9e8c87d761ad32aea2112cbc9b6876887966ec Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Thu, 8 Aug 2024 05:29:42 -0700 Subject: [PATCH 19/20] more --- documentation/general/torn-sdk.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index dc3ca082bbc8..9d918cb8d1f9 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -43,7 +43,7 @@ To address this issue we are going to separate Visual Studio and the .NET SDK in Specifically we will be -1. Changing msbuild to use a compiler from the .NET SDK when msbuild's compiler is older +1. Changing msbuild to use a compiler at least as new as the .NET SDKs compiler 2. Changing Visual Studio to use analyzers from Visual Studio In addition to making our builds more reliable this will also massively simplify our [analyzer Development strategy][sdk-lifecycle]. Analyzers in the SDK following this model can always target the latest Roslyn version without the need for complicated multi-targeting. From 5d2049485d34522a1ccbc74039fc69367b60cf69 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Thu, 8 Aug 2024 07:32:12 -0700 Subject: [PATCH 20/20] more --- documentation/general/torn-sdk.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/documentation/general/torn-sdk.md b/documentation/general/torn-sdk.md index 9d918cb8d1f9..e8de8b9f945d 100644 --- a/documentation/general/torn-sdk.md +++ b/documentation/general/torn-sdk.md @@ -119,9 +119,10 @@ In box could also employ a light up strategy. Essentially reference older versio Instead of `` the toolset package could be installed via ``. This is how the existing Microsoft.Net.Compilers.Toolset package works today. This has a number of disadvantages: -1. PackageReferences are not side effect free and have been shown to cause issues in complex builds. For example this package did not work by default in the Visual Studio or Bing builds. Deploying this at scale to all our customers will almost certainly cause issues. -2. This approach does not solve all scenarios. The mechanics of restore mean that the compiler is not swapped until part way through the build. Build operations that happen earlier, such as [legacy WPF][issue-legacy-wpf], will still use the Visual Studio compiler and hence experience analyzer compatibility issues. -3. Mixing builds that do and do not restore can lead to different build outcomes as they can end up choosing different compilers. Mixing the version of msbuild / dotnet msbuild can also lead to strange outcomes as they can end up choosing different compilers. +1. `PackageReference` elements are not side effect free and have been shown to cause issues in complex builds. For example this package did not work by default in the Visual Studio or Bing builds. Deploying this at scale to all our customers will almost certainly cause issues. +1. This approach does not solve all scenarios. The mechanics of restore mean that the compiler is not swapped until part way through the build. Build operations that happen earlier, such as [legacy WPF][issue-legacy-wpf], will still use the Visual Studio compiler and hence experience analyzer compatibility issues. +1. Mixing builds that do and do not restore can lead to different build outcomes as they can end up choosing different compilers. Mixing the version of msbuild / dotnet msbuild can also lead to strange outcomes as they can end up choosing different compilers. +1. `PackageReference` elements, even when implicit, end up showing up in NuGet lock files. That makes it difficult for lock files to be kept in a consistent state as subtly changing the Visual Studio or .NET SDK version would end up impacting the lock file. ### Install the Framework compiler