Skip to content

Try to fix the analyzer path when loading in OOP#55520

Merged
genlu merged 2 commits intodotnet:mainfrom
genlu:cherrypick
Aug 10, 2021
Merged

Try to fix the analyzer path when loading in OOP#55520
genlu merged 2 commits intodotnet:mainfrom
genlu:cherrypick

Conversation

@genlu
Copy link
Member

@genlu genlu commented Aug 10, 2021

This is just cherry-picking the change from main-vs-deps to main, to make it easier to resolve potential conflict with #55098. They were originally made in #55425

FYI @tmat @RikkiGibson

genlu added 2 commits August 9, 2021 17:16
This is a hack to temporarily unblock further investigations
@genlu genlu requested a review from a team as a code owner August 10, 2021 00:19
@ghost ghost added the Area-IDE label Aug 10, 2021
@genlu genlu enabled auto-merge August 10, 2021 00:21
// For analyzers shipped in Roslyn, different set of assemblies might be used when running
// in-proc and OOP e.g. in-proc (VS) running on desktop clr and OOP running on ServiceHub .Net6
// host. We need to make sure to use the ones from the same location as the remote.
private class RemoteAnalyzerAssemblyLoader : DefaultAnalyzerAssemblyLoader
Copy link
Member

Choose a reason for hiding this comment

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

Don't this need tests added for it to verify that this fully works?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are absolutely right. However, the best way to test this (and anything related to .net6 servicehub host) is via integration tests, which I will enable once all the required bits are available on our test machines.

@genlu genlu merged commit 4b0b934 into dotnet:main Aug 10, 2021
@ghost ghost added this to the Next milestone Aug 10, 2021
@genlu genlu deleted the cherrypick branch August 10, 2021 19:09
@dibarbet dibarbet modified the milestones: Next, 17.0.P4 Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants