Skip to content

Revert "Remove Razor IVTs"#43668

Merged
genlu merged 1 commit intomasterfrom
revert-43235-RazorIvtRemoval
Apr 27, 2020
Merged

Revert "Remove Razor IVTs"#43668
genlu merged 1 commit intomasterfrom
revert-43235-RazorIvtRemoval

Conversation

@genlu
Copy link
Member

@genlu genlu commented Apr 24, 2020

@genlu genlu requested review from a team as code owners April 24, 2020 21:55
@NTaylorMullen
Copy link

NTaylorMullen commented Apr 27, 2020

Why did it break ngen?

@tmat
Copy link
Member

tmat commented Apr 27, 2020

IDK

@genlu
Copy link
Member Author

genlu commented Apr 27, 2020

@NTaylorMullen

04/24/2020 17:01:14.945 [4692]: 1>Warning: System.TypeLoadException: Access is denied: 'System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1+ConfiguredTaskAwaiter[Microsoft.VisualStudio.LanguageServices.Razor.RazorLanguageServiceClient]'. while resolving 0x2000047 - <ResolveTagHelpersOutOfProcessAsync>d__6.
04/24/2020 17:01:15.336 [4692]: 3>    Compiling assembly C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\Common7\IDE\CommonExtensions\Microsoft\RazorLanguageServices\Microsoft.VisualStudio.LiveShare.Razor.dll (CLR v4.0.30319) ...
04/24/2020 17:01:15.664 [4692]: 1>Access is denied: 'System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1+ConfiguredTaskAwaiter[Microsoft.VisualStudio.LanguageServices.Razor.RazorLanguageServiceClient]'. while compiling method OOPTagHelperResolver.ResolveTagHelpersOutOfProcessAsync
04/24/2020 17:01:15.711 [4692]: 1>Access is denied: 'System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1+ConfiguredTaskAwaiter[Microsoft.VisualStudio.LanguageServices.Razor.RazorLanguageServiceClient]'. while compiling method token 0x6000154
04/24/2020 17:01:15.711 [4692]: 1>Access is denied: 'System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1+ConfiguredTaskAwaiter[Microsoft.VisualStudio.LanguageServices.Razor.RazorLanguageServiceClient]'. while compiling method token 0x6000155

@NTaylorMullen
Copy link

NTaylorMullen commented Apr 27, 2020

Thanks @genlu! It looks like some older pieces of that binary are still depending on IVTs for the ServiceHub components. I'll take a peak, i've filed https://github.com/dotnet/aspnetcore/issues/21253 to track the work.

@genlu
Copy link
Member Author

genlu commented Apr 27, 2020

@NTaylorMullen OK, I will go ahead and merge this to unblock our insertion. Please let us know once you have updated your bits in VS, we will revert this then. Thanks!

@genlu genlu merged commit 50e1edd into master Apr 27, 2020
@ghost ghost added this to the Next milestone Apr 27, 2020
@genlu genlu deleted the revert-43235-RazorIvtRemoval branch April 27, 2020 21:02
tmat
tmat reviewed Apr 27, 2020
View reviewed changes
Copy link
Member

@tmat tmat left a comment

Choose a reason for hiding this comment

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

LGTM

@sharwell sharwell modified the milestones: Next, temp, 16.7.P1 Apr 28, 2020
NTaylorMullen
NTaylorMullen reviewed Apr 28, 2020
View reviewed changes
src/VisualStudio/Razor/Microsoft.VisualStudio.LanguageServices.Razor.RemoteClient.csproj
<ItemGroup>
<ProjectReference Include="..\..\Workspaces\Core\Portable\Microsoft.CodeAnalysis.Workspaces.csproj" />
</ItemGroup>
<ItemGroup>
Copy link

@NTaylorMullen NTaylorMullen Apr 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@tmat I just noticed. This ItemGroup being removed is probably the crux of the issue actually given this is Microsoft.VisualStudio.LanguageServices.Razor.RemoteClient.csproj

I don't think you can actually remove this one.

Copy link

@NTaylorMullen NTaylorMullen Apr 28, 2020

Choose a reason for hiding this comment

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

That being said of course, I imagine we'd want to transition everything from Microsoft.VisualStudio.LanguageServices.Razor.RemoteClient.csproj into ExternalAccess.Razor right?

Copy link
Member

@tmat tmat Apr 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

You are right. I forgot this hasn't been ExternalAccess-ized. Yes, we can actually remove this assembly and add ExternalAccess APIs to ExternalAccess.Razor. That would be much simpler. I'm gonna add these.

@NTaylorMullen NTaylorMullen mentioned this pull request Oct 9, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tmat tmat tmat left review comments

@agocke agocke agocke approved these changes

+1 more reviewer

@NTaylorMullen NTaylorMullen NTaylorMullen left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

16.7.P1

Development

Successfully merging this pull request may close these issues.

5 participants

@genlu @NTaylorMullen @tmat @agocke @sharwell