-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Remove MicrosoftCodeAnalysisVersion_LatestVS and unify analyzer dependencies #123641
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
b598776
20865cc
25401a4
a5185ae
225826b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,25 +65,25 @@ | |
| <MicrosoftCodeAnalysisVersion_4_4>4.4.0</MicrosoftCodeAnalysisVersion_4_4> | ||
| <!-- Compatibility with VS 17.8/.NET SDK 8.0.1xx --> | ||
| <MicrosoftCodeAnalysisVersion_4_8>4.8.0</MicrosoftCodeAnalysisVersion_4_8> | ||
| <!-- Compatibility with the latest Visual Studio Preview release --> | ||
| <!-- | ||
| The exact version is always a moving target. This version should never go ahead of the version of Roslyn that is included in the most recent | ||
| public Visual Studio preview version. If it were to go ahead, then any components depending on this version would not work in Visual Studio | ||
| and would cause a major regression for any local development that depends on those components contributing to the build. | ||
| This version must also not go ahead of the most recently release .NET SDK version, as that would break the source-build build. | ||
| Source-build builds the product with the most recent previously source-built release. Thankfully, these two requirements line up nicely | ||
| such that any version that satisfies the VS version requirement will also satisfy the .NET SDK version requirement because of how we ship. | ||
| Analyzers and source generators reference the latest Microsoft.CodeAnalysis version (MicrosoftCodeAnalysisVersion), | ||
| which is auto-updated via dependency flow. This version may move ahead of what's in the global.json SDK, | ||
| particularly when analyzers need to consume new Roslyn APIs. When that happens, update the bootstrap global.json | ||
| SDK to pull in the new Roslyn (similar to onboarding new language features). | ||
|
|
||
| Note: This version affects the design-time experience in VS, so moving ahead of the latest VS preview or SDK preview | ||
| can break local development. This is expected to happen occasionally as we move faster than previews to depend on | ||
| new analyzer features. | ||
| --> | ||
| <MicrosoftCodeAnalysisVersion_LatestVS>5.0.0-2.26070.104</MicrosoftCodeAnalysisVersion_LatestVS> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems that previously you could have a situation where roslyn 5 was only available in VS 2026 but you wanted to allow devs to use also VS 2022 - then you could set RoslynLatestVS to 4.14 and that would work. Now you will effectively force devs to always use latest VS if I understand correctly. If you instead want to just avoid binary breaks, you could build against latest roslyn only in CI, and still allow different (lower) version locally, I guess.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm open to it, but personally prefer to keep CI and local dev in sync (based on bad experiences with them diverging). I think it's not quite that bad - for example I tried loading+building LibraryImportGeneratorSample in VS 2022 with this change and it works. I think it only breaks if generators/analyzers adopt new analyzer APIs. This puts analyzer APIs in a similar situation as new language features. Analyzers that need to support specific VS versions can also use pinned MicrosoftCodeAnalysisVersion_X_X versions. @jkoritzinsky @stephentoub any thoughts/concerns?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My concern is less people using VS2022 and more in the following scenario:
We can try with this change and adjust as needed, but I think to avoid serious breaks for the dev experience we'll need to have some way to use the Roslyn version that comes with the LKG sdk. I don't believe we can just do a darc flow like our other dependencies as this dependency technically goes in the opposite direction of the build graph. For source-build, we effectively get the LKG behavior today because the version comes from the previously-source-built assets.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That is surprising to me (assuming your analyzer targets roslyn 5.* and your VS has roslyn 4.*), I think Roslyn forbids loading analyzers that target newer versions - we have a warning for that like this:
That's the same failure mode, but yours is a better example, thanks.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Looks like that's because we set Does that align with your expectations @jkoritzinsky @jjonescz @stephentoub? Is there a more serious break I'm missing?
I think source build always sets Seem reasonable? I don't want to cause unnecessary breaks for local dev, but I think we need to prioritize shipping a consistent version here. Any objections to taking this change, and finding other mitigations for local dev when we hit problems? (For example, the workflow for using VS could have a way to override the version to use older Roslyn).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Okay, that makes sense. (FWIW, since .NET 10, VS builds always use the roslyn from the SDK, so if you are using the toolset compiler for this reason, it isn't necessary anymore.)
I guess that's the better case, but if there are other parts of the code that depend more heavily on the generated code, the IDE might become unusable. |
||
| <!-- Some of the analyzer dependencies used by ILLink project --> | ||
| <MicrosoftCodeAnalysisBannedApiAnalyzersVersion>3.3.5-beta1.23270.2</MicrosoftCodeAnalysisBannedApiAnalyzersVersion> | ||
| </PropertyGroup> | ||
|
|
||
| <!-- | ||
| These packages affect the design-time experience in VS, so we update them at the same cadance as the MicrosoftCodeAnalysisVersion_LatestVS version. | ||
| These packages affect the design-time experience in VS, so we update them at the same cadence as the latest Microsoft.CodeAnalysis version. | ||
| --> | ||
| <PropertyGroup> | ||
| <MicrosoftCodeAnalysisCSharpCodeStyleVersion>$(MicrosoftCodeAnalysisVersion_LatestVS)</MicrosoftCodeAnalysisCSharpCodeStyleVersion> | ||
| <MicrosoftCodeAnalysisCSharpCodeStyleVersion>$(MicrosoftCodeAnalysisVersion)</MicrosoftCodeAnalysisCSharpCodeStyleVersion> | ||
| </PropertyGroup> | ||
|
|
||
| <PropertyGroup> | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.