-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Fix compiler vsix deployment #72321
Fix compiler vsix deployment #72321
Conversation
@dotnet/roslyn-compiler for two reviews |
@@ -195,6 +195,7 @@ | |||
<PackageVersion Include="System.IO.Pipelines" Version="$(SystemIOPipelinesVersion)" /> | |||
<PackageVersion Include="System.IO.Pipes.AccessControl" Version="5.0.0" /> | |||
<PackageVersion Include="System.Memory" Version="4.5.5" /> | |||
<PackageVersion Include="System.Numerics.Vectors" Version="4.5.0" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this work in VS insertions? Won't it cause builds to fail for some reason? Should we do a validation run?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jjonescz this vsix isn't actually used for insertion - it uses one of Microsoft.CodeAnalysis.Compilers.x64/.arm64/.x64.vsix as the actual insertion VSIX
However weirder things have broken the VS build so I can run a val insertion to make sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, thanks for explanation; I think it's fine then, we can always revert :)
"Fails to build in VS" Is that during F5 launches, full installs, etc .... |
@@ -33,6 +33,7 @@ | |||
<PackageReference Include="System.Collections.Immutable" IncludeInVsix="true" /> | |||
<PackageReference Include="System.Memory" IncludeInVsix="true" /> | |||
<PackageReference Include="System.Reflection.Metadata" IncludeInVsix="true" /> | |||
<PackageReference Include="System.Numerics.Vectors" IncludeInVsix="true" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering whether BuildBoss shouldn't catch cases like this. cc @jaredpar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's my concern as well. Generally BuildBoss should be used to catch inconsistencies like this. That is why I'm curious about what scenarios are not working today.
Moving forward with this as David is currently out, and thsi is blocking ability to validate thse scenarios while in an F5 lauched hive. |
The compiler vsix was not deploying system.numerics.vectors which caused csc to fail to launch on builds in VS.
The vsix that actually gets inserted does include the dll, so this is only a problem for local F5 / integration test scenarios.