Add TaskExtensions to compiler package#26703
Conversation
|
FYI @jasonmalinowski @dotnet/roslyn-infrastructure @dotnet/roslyn-compiler |
| file source=$(OutputPath)\Vsix\CompilerExtension\System.Security.Cryptography.X509Certificates.dll vs.file.ngenArchitecture=all | ||
| file source=$(OutputPath)\Vsix\CompilerExtension\System.Security.Principal.Windows.dll vs.file.ngenArchitecture=all | ||
| file source=$(OutputPath)\Vsix\CompilerExtension\System.Text.Encoding.CodePages.dll vs.file.ngenArchitecture=all | ||
| file source=$(OutputPath)\Vsix\CompilerExtension\System.Threading.Tasks.Extensions.dll vs.file.ngenArchitecture=all |
There was a problem hiding this comment.
This is suspicious to me. The CompilerExtension is largely a F5 tool for the Roslyn team. It shouldn't be impacting NGEN in any way that I understand.
Yes the set of binaries listed here are also included in both our VS and MSBuild installations. Are we using this extension as a convenient place to list binaries that should be NGEN'd elsewhere in our code.
There was a problem hiding this comment.
This isn't ngen only: this is what is deployed in our actual setup. The ngen is secondary. CompilerExtension is hijacked as the source of dependencies to deploy, by taking advantage of nuget resolution to pick the right runtime assets.
There was a problem hiding this comment.
Where does this get deployed? Up until now I've thought of CompilerExtension as our little F5 toy. If there is product impact then I'd like to understand it better.
There was a problem hiding this comment.
The .swr here being modified is payload being installed during a real VS install on customer machines. Since that needs a source of NuGet-provided dependencies, rather than hard-coding paths into the NuGet packages or trying to reinvent the wheel, the "solution" used is to take the binaries out of CompilerExtension so the same binaries are being shipped in both places.
There was a problem hiding this comment.
If that is the intent then I wonder if we should switch to generating this file as we do for the portable facades. This would save us from making this mistake in the future. It's also quite definitive vs. the facades which is more of a hueristic.
There was a problem hiding this comment.
You could. It might also be possible to actually make CompilerExtension work in both. There are some differences today (the extension has the package that forces it to load during design time), but maybe there's some way to refactor this to avoid the duplication.
There was a problem hiding this comment.
I think we are missing this file in at least one other place: VS.Tools.Roslyn.nuspec file we generate doesn't have this file either. Presumably this will block any toolset insertion.
Add
System.Threading.Tasks.Extensionsto compiler package.We see an RPS regression in this insertion PR, which is caused by jitting MS.CA.CSharp (in VBCSCompiler process). Further investigation shows that binding for native image for that assembly failed because of