Skip to content
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

Add ExternalAccess.RazorCompiler.dll reference to C# language server #70176

Merged
merged 3 commits into from
Sep 28, 2023

Conversation

allisonchou
Copy link
Contributor

Razor relies on Roslyn to ship this dll with the language server since they use it for source generators.

@allisonchou allisonchou requested a review from a team as a code owner September 28, 2023 19:01
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Sep 28, 2023
@allisonchou allisonchou enabled auto-merge (squash) September 28, 2023 19:04
@genlu
Copy link
Member

genlu commented Sep 28, 2023

Shouldn't this be shipped as part of Razor then?

@allisonchou
Copy link
Contributor Author

Shouldn't this be shipped as part of Razor then?

Roslyn's dotnet.exe process is the one attempting to load the dll in VS Code. I confirmed with David that it'd be ok to include as part of the C# language server

@333fred
Copy link
Member

333fred commented Sep 28, 2023

Shouldn't this be shipped as part of Razor then?

No. We want the version of the external access to match the shipped version of roslyn, not razor, as underlying API changes happen but we keep the EA layer stable. I'm sure @sharwell would also be happy to elaborate on the reasoning.

@allisonchou allisonchou merged commit 4ddba49 into dotnet:main Sep 28, 2023
24 checks passed
@ghost ghost added this to the Next milestone Sep 28, 2023
@allisonchou allisonchou deleted the dev/allichou/AddRazorReference branch September 28, 2023 21:55
@sharwell
Copy link
Member

Shouldn't this be shipped as part of Razor then?

Absolutely not. This package is version-locked to a single exact version of the compiler (no deviation is allowed in either direction), and therefore must ship with the compiler.

@@ -58,6 +58,9 @@

<!-- Dlls we don't directly reference but need to include to build the MEF composition -->
<ProjectReference Include="..\..\CSharp\Portable\Microsoft.CodeAnalysis.CSharp.Features.csproj" />

<!-- Not directly referenced but needed for Razor source generators -->
<ProjectReference Include="..\..\..\Tools\ExternalAccess\RazorCompiler\Microsoft.CodeAnalysis.ExternalAccess.RazorCompiler.csproj" />
Copy link
Member

@sharwell sharwell Sep 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@allisonchou This needs ReferenceOutputAssembly="false" added for compile-time enforcement of the "not directly referenced" condition

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, will add it in a follow up PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants