Load razor assembly directly: #76808
Conversation
chsienki
commented
Jan 18, 2025
chsienki
commented
- Pass the root directory to assembly resolvers
- When looking for razor or its deps load them into the same ALC as the compiler
- Pass the root directory to assembly resolvers - When looking for razor or its deps load them into the same ALC as the compiler
dotnet-issue-labeler
bot
added
the
untriaged
| { | ||
| return loadedAssembly; | ||
| } | ||
| return compilerContext.LoadFromAssemblyPath(assembly); |
There was a problem hiding this comment.
Note that here we're loading razor into the 'main' compiler ALC. We could create our own ALC to load into, but it will need to be custom code that defers to the main ALC for things like MS.CA.dll. That seemed overkill, so I just used the main one (which is already conceptually the 'shared' one anyway).
src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs
Outdated
Show resolved
Hide resolved
| s_assemblyResolver = resolver; | ||
| return true; | ||
| var compilerContext = AssemblyLoadContext.GetLoadContext(typeof(Compilation).Assembly)!; | ||
| if (compilerContext.Assemblies.SingleOrDefault(a => a.GetName().Name == assemblyName.Name) is Assembly loadedAssembly) |
There was a problem hiding this comment.
Consider marking the lambda static if possible (by passing assemblyName.Name as an additional argument).
src/Tools/ExternalAccess/Razor/RazorAnalyzerAssemblyResolver.cs
Outdated
Show resolved
Hide resolved
| @@ -26,7 +26,7 @@ internal class RemoteWorkspaceManager | |||
| MefHostServices.DefaultAssemblies | |||
| .Add(typeof(AspNetCoreEmbeddedLanguageClassifier).Assembly) | |||
| .Add(typeof(BrokeredServiceBase).Assembly) | |||
| .Add(typeof(RazorAnalyzerAssemblyResolver).Assembly) | |||
| .Add(typeof(IRazorLanguageServerTarget).Assembly) | |||
There was a problem hiding this comment.
Because it targets both .NET and netstandard. In the netstandard version RazorAnalyzerAssemblyResolver no longer exists. It just seemed neater to change the type we're referencing instead of #if-defing the assembly add.
| { | ||
| if (s_assemblyResolver is null && !s_assembliesRequested.Contains(canaryAssembly)) | ||
| lock (s_loaderLock) |
| return loadedAssembly; | ||
| } | ||
|
|
||
| var assembly = Path.Combine(rootDirectory, $"{assemblyName.Name}.dll"); |
There was a problem hiding this comment.
Think that this code only works when the IDE has normalized the path to the razor assemblies. Essentially this logic only works if every path to the razor compiler passed via /analyzer: is the same path once we get to this layer. If there is a different path then we will be in a "first one wins" situation (the first winner will be cached in the compiler ALC).
If that is correct think we should add some explanation here.
| private static Func<AssemblyName, Assembly?>? s_assemblyResolver; | ||
| private const string RazorCompilerAssemblyName = "Microsoft.CodeAnalysis.Razor.Compiler"; | ||
| private const string RazorUtilsAssemblyName = "Microsoft.AspNetCore.Razor.Utilities.Shared"; | ||
| private const string ObjectPoolAssemblyName = "Microsoft.Extensions.ObjectPool"; |
There was a problem hiding this comment.
Where in the code do we keep the list of generators that the IDE remaps from SDK to the VS copy? Wanted to make sure there is agreement on all the assemblies in that map and what we redirect here.
