Delpoy ExternalAccess.RazorCompiler with compiler #70203
Merged
+94
−12
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This moves the MS.CA.EA.RazorCompiler to deploy with the roslyn compiler. This DLL has IVT with the compiler which means it's relationship is not tracked through the public API tracker. This problem came to light by digging into the fallout from a subtle [binary breaking](dotnet#70082) change in the compiler that invalidated earlier versions of the EA.RazorCompiler DLL. That made it apparent our previous deployment strategy was incorrect. Deploying with the compiler should both fix our present issue (inability to insert Roslyn) as well as tearing issues that could come once the change is released. The compiler will prefer dependencies in its directory over ones deployed with analyzers. That means existing versions of razor that deploy the EA DLL (compat or incompat version) will be overruled by the version deployed next to the compiler. I locally verified that this copy is preferred over the the EA deployed with razor today (dotnet and msbuild). There are tests that verify this behavior in the compiler already but I wanted manual verification as well.
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
333fred
approved these changes
Oct 1, 2023
RikkiGibson
reviewed
Oct 1, 2023
| @@ -81,4 +81,4 @@ | |||
| "src\\Workspaces\\VisualBasic\\Portable\\Microsoft.CodeAnalysis.VisualBasic.Workspaces.vbproj" | |||
| ] | |||
| } | |||
| } | |||
| } | |||
There was a problem hiding this comment.
was this change just a sort?
There was a problem hiding this comment.
Appears to have been by solution explorer.
RikkiGibson
approved these changes
Oct 1, 2023
|
Not sure if the determinism failure was actually related to the change. Let's try and merge and create an insertion today if we can, just so we can start Monday on the right foot. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This moves the MS.CA.EA.RazorCompiler to deploy with the roslyn compiler. This DLL has IVT with the compiler which means it's relationship is not tracked through the public API tracker.
This problem came to light by digging into the fallout from a subtle binary breaking change in the compiler that invalidated earlier versions of the EA.RazorCompiler DLL. That made it apparent our previous deployment strategy was incorrect.
Deploying with the compiler should both fix our present issue (inability to insert Roslyn) as well as tearing issues that could come once the change is released. The compiler will prefer dependencies in its directory over ones deployed with analyzers. That means existing versions of razor that deploy the EA DLL (compat or incompat version) will be overruled by the version deployed next to the compiler.
I locally verified that this copy is preferred over the the EA deployed with razor today (dotnet and msbuild). There are tests that verify this behavior in the compiler already but I wanted manual verification as well.