-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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 build host when only the .net 6 SDK is installed #73818
Conversation
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.
As a workaround this seems fine, but can we file a bug to investigate why this is necessary? This feels like a bug in MSBuild or NuGet...
it kind of seems expected to me? We didn't directly reference it and we're not shipping self contained, so wouldn't it generally not be added to the output dir (since its part of the SDK)? Might be entirely wrong here. |
I was expecting we would have an implicit reference to it from the SDK. From there, the |
/azp run roslyn-ci |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run roslyn-integration-CI |
Azure Pipelines successfully started running 1 pipeline(s). |
using (var pipeServer = new AnonymousPipeServerStream(PipeDirection.Out, HandleInheritability.Inheritable)) | ||
{ | ||
// Pass the client process a handle to the server. | ||
arguments.Add(pipeServer.GetClientHandleAsString()); | ||
pipeClient.StartInfo.Arguments = string.Join(" ", arguments); | ||
pipeClient.StartInfo.UseShellExecute = false; | ||
|
||
// Errors will be logged to stderr, redirect to us so we can capture it. |
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.
errors were getting swallowed so it was just failing with no info. this ensures error output from the discovery worker is captured
|
||
// Wait for 'sync message' from the server. | ||
do | ||
try |
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.
similar change to ensure exception details are captured if something goes wrong.
Was caught by integration tests
SCI was not getting shipped in the output, so when using a .net6 runtime (.net6 SDK only installed, launch server via .exe instead of .dll) it failed to find the 8.0 dependency.
Adding an explicit reference seems to fix the issue