Skip to content

Allow VS extensions to participate in Roslyn OOP MEF: #79219

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chsienki
Copy link
Member

@chsienki chsienki commented Jul 1, 2025

  • Allow extensions to supply a set of RoslynOopMefComponent
  • Load those from extensions and pass them over to the OOP process at init
  • Add the passed assembly paths into the OOP MEF catalog construction

Fixes #71510

- Allow extensions to supply a set of RoslynOopMefComponent
- Load those from extensions and pass them over to the OOP process at init
- Add the passed assembly paths into the OOP MEF catalog construction
@@ -15,5 +16,5 @@ internal interface IRemoteInitializationService
/// Called as soon as the remote process is created.
/// </summary>
/// <returns>Process ID of the remote process and an error message if the server encountered initialization issues.</returns>
ValueTask<(int processId, string? errorMessage)> InitializeAsync(WorkspaceConfigurationOptions options, string localSettingsDirectory, CancellationToken cancellationToken);
ValueTask<(int processId, string? errorMessage)> InitializeAsync(WorkspaceConfigurationOptions options, string localSettingsDirectory, ImmutableArray<string> oopMefComponentPaths, CancellationToken cancellationToken);
Copy link
Member Author

Choose a reason for hiding this comment

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

Should probably rename this something like RemoteMefComponentAssemblyPaths

@CyrusNajmabadi
Copy link
Member

@dibarbet to make sure this aligns with the gladstone experiment to load extensions.

@@ -23,7 +23,7 @@ internal sealed partial class VisualStudioDiagnosticAnalyzerProvider
{
private const string AnalyzerContentTypeName = "Microsoft.VisualStudio.Analyzer";

internal const string RazorContentTypeName = "Microsoft.VisualStudio.RazorAssembly";
internal const string RoslynOopMefContentName = "Microsoft.VisualStudio.RoslynOopMefComponent";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
internal const string RoslynOopMefContentName = "Microsoft.VisualStudio.RoslynOopMefComponent";
internal const string RoslynOopMefContentName = "Microsoft.VisualStudio.RoslynOopMefComponent";

Nit: maybe call it RoslynServiceHubMefComponent. Only because I agree that you correctly applied .NET casing rules to the "OOP" but anybody seeing it might be confused.

Copy link
Member

Choose a reason for hiding this comment

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

Obviously when you undraft this we should rename this type.

Comment on lines +104 to +106
// If that fails, try and load it from the codeBasePath if present
var selfAlc = System.Runtime.Loader.AssemblyLoadContext.GetLoadContext(Assembly.GetExecutingAssembly())!;
return selfAlc.LoadFromAssemblyPath(codeBasePath);
Copy link
Member

Choose a reason for hiding this comment

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

We'd have to think about what this means for dependnecies though of your MEF component. Should we put you generally into an ALC? We do similar things in VS Code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RemoteWorkspaceManager SimpleAssemblyLoader doesn't respect codeBasePath
3 participants