Use Roslyn from SDK always for SDK-based project builds #48028
Conversation
jjonescz
commented
Mar 31, 2025
•
jjonescz
commented
- Related to Homogenize MSBuild across VS and SDK msbuild#11142
- Doc: https://github.com/dotnet/sdk/blob/main/documentation/general/decouple-vs-and-net-sdk.md
- Roslyn counterpart: Execute core compiler from full framework msbuild roslyn#77914
ghost
added
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.Common.targets
Outdated
Show resolved
Hide resolved
test/Microsoft.NET.Build.Tests/Microsoft.NET.Build.Tests.csproj
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets
Outdated
Show resolved
Hide resolved
|
Do we know all of the other tools/products that use Csc to compile things internally? (WPF, Azure Functions, etc) - I'd expect we'd want to make sure that those can conditionally pass the new property if present so make sure that users don't flip between different kinds of compilers in the same logical build. |
|
We know some but don't have a comprehensive list we can consult. Probably should start keeping one as we work through this change during this release |
|
@RikkiGibson, @chsienki PTAL |
|
Note that I first need to merge the Roslyn side but waiting for roslyn infra to work again before I do that. |
eng/Versions.props
Outdated
| <MicrosoftCodeAnalysisWorkspacesCommonPackageVersion>5.0.0-1.25210.7</MicrosoftCodeAnalysisWorkspacesCommonPackageVersion> | ||
| <MicrosoftCodeAnalysisWorkspacesMSBuildPackageVersion>5.0.0-1.25210.7</MicrosoftCodeAnalysisWorkspacesMSBuildPackageVersion> | ||
| <MicrosoftCodeAnalysisCSharpWorkspacesPackageVersion>5.0.0-1.25210.7</MicrosoftCodeAnalysisCSharpWorkspacesPackageVersion> | ||
| <MicrosoftNetCompilersToolsetVersion>4.14.0-3.25210.1</MicrosoftNetCompilersToolsetVersion> |
There was a problem hiding this comment.
Why are we downgrading the compiler here?
There was a problem hiding this comment.
This PR needs a corresponding change in Roslyn to flow in first. I built it manually and used it here just temporarily (ie, this will be reverted before merging).
| Use Roslyn deployed with SDK for builds of SDK-style projects (regardless of whether the initiator is `dotnet` or `msbuild`). | ||
| See https://github.com/dotnet/sdk/blob/main/documentation/general/decouple-vs-and-net-sdk.md. | ||
| --> | ||
| <Choose> |
There was a problem hiding this comment.
Consider just making these conditions on the property directly rather than using choose, so you don't have to duplicate the property groups.
There was a problem hiding this comment.
That would mean needing to duplicate the conditions though and I think that would be less readable.
| // UseSharedCompilation should be the default, so set it only if false. | ||
| if (!useSharedCompilation) | ||
| { | ||
| project.AdditionalProperties["UseSharedCompilation"] = "false"; | ||
| } |
There was a problem hiding this comment.
Might be simpler to just always be explicit here:
| // UseSharedCompilation should be the default, so set it only if false. | |
| if (!useSharedCompilation) | |
| { | |
| project.AdditionalProperties["UseSharedCompilation"] = "false"; | |
| } | |
| project.AdditionalProperties["UseSharedCompilation"] = useSharedCompilation ? "true" : "false"; | |
There was a problem hiding this comment.
Which you can probably fold into the initializer above actually
There was a problem hiding this comment.
My intention is to not set UseSharedCompilation to true, so the tests also verify that true is the default value
