Pin the compiler version in RSG to an exact version #24130
Conversation
ghost
added
the
| @@ -11,8 +11,14 @@ | |||
| <Nullable>enable</Nullable> | |||
| </PropertyGroup> | |||
|
|
|||
| <PropertyGroup> | |||
There was a problem hiding this comment.
@eerhardt \ @MichaelSimons wanted to get your thoughts on this, as an enhancement to https://github.com/dotnet/sdk/pull/22072/files. Users are often affected by this when running a fairly new SDK using MSBuild where the compiler hasn't been updated as yet.
There was a problem hiding this comment.
I think this is a bigger issue / question for the SDK and Roslyn teams. cc @jaredpar @chsienki @dsplaisted
So the repro steps here are:
- Install VS 2022, but not the latest version
- Install .NET Core SDK 6.0.200
and now we don't have a coherent SDK + VS? There are components in the 6.0.200 SDK that reference Roslyn 4.1, but the VS 2022 installed only has Roslyn 4.0?
Is that all true?
IMO - we shouldn't be using different version when building from source vs. building for Microsoft distribution. That's a recipe for disaster.
There was a problem hiding this comment.
Interestingly, this appears to be the path the ApiCompatibility tool has taken - https://github.com/dotnet/sdk/pull/23321/files
There was a problem hiding this comment.
Agreed - I wasn't excited about that change either 😄. The difference with the ApiCompatibility tool is that it also ships OOB from the SDK into it's own NuGet package:
https://www.nuget.org/packages/Microsoft.DotNet.Compatibility
So someone could be using an older SDK, and PackageReference the newer version of that NuGet package.
Correct me if I'm wrong, but I believe that the Razor Source Generator only ships as part of the SDK, right? Shouldn't we have a guaranteed version of Roslyn that the SDK supports?
There was a problem hiding this comment.
Correct me if I'm wrong, but I believe that the Razor Source Generator only ships as part of the SDK, right?
That is right.
Shouldn't we have a guaranteed version of Roslyn that the SDK supports?
👍🏽
There was a problem hiding this comment.
Pinning the compiler means that you're not getting bug fixes, features, etc ...
It's less important for a tool like API Compat which is using us for a limited use case. Mostly to grab a semantic model to inspect the code. Much more important for components that are actually participating in mainline compilation.
In this case though this is not pinning the compiler, it's pinning the Compiler API version Razor is compiling against. At runtime we're going to use the MS.CA that ships with the compiler. Still though this feels very unnecessary.
There was a problem hiding this comment.
@jaredpar do you have suggestions on how we should proceed here? Users can unblock themselves by updating their VS / pinning the SDK, but both of these seem like crummy option.
Would we be open to taking this PR for 6.0.x and consider something more lasting for 7.0?
There was a problem hiding this comment.
Is this the only analyzer / source generator that actually ships in box with the SDK? I know that the roslyn-analyzer pin their version way back to something like 3.3.
We could follow that path here, and then for source-build, we need to put the ref assemblies for the version you pin in the "SBRP".
There was a problem hiding this comment.
Is this the only analyzer / source generator that actually ships in box with the SDK?
ASP.NET Core builds some analyzers that ship as part of the WebSDK. Those are currently pinned to build against 3.3:
https://github.com/dotnet/aspnetcore/blob/main/eng/Versions.props#L198-L199
Edit: This particular source generator relies on IIncrementalGenerator which is only available in 4.0 or newer versions of the compiler.
There was a problem hiding this comment.
Opened dotnet/source-build-reference-packages#357 to add the required bits to SBRP so we can target the same version in source build.
|
@RikkiGibson, @jcouv FYI |
|
I didn't think this was a supported scenario--using a VS with an older version of the compiler than what's in the SDK. Did I misunderstand something? |
|
@RikkiGibson We're still defining the exact support policy, but it is expected to work in some scenarios. In this case the generator isn't actually using any of the new features, it's just picking up a ref to latest MS.CA by default, so we want to change it so it will continue to load and work in downstream compilers. |
Addresses #23972