Warnings-as-errors fail BuildSubmission results #6006
Conversation
Fixes dotnet#5837 by using the approach used in BuildManager to check whether warnings-as-errors were emitted to convert the result to a failure at the submission level, because MSBuild was promoting it to failure too late (in time to log it but not to fail the msbuild invocation.
It's handled at the submission level now.
| @@ -198,6 +198,13 @@ private void CheckForCompletion() | |||
| bool hasCompleted = (Interlocked.Exchange(ref _completionInvoked, 1) == 1); | |||
| if (!hasCompleted) | |||
| { | |||
| // Did this submission have warnings elevated to errors? If so, mark it as | |||
| // failed even though it succeeded (with warnings--but they're errors). | |||
| if (((IBuildComponentHost)BuildManager).LoggingService.HasBuildSubmissionLoggedErrors(BuildResult.SubmissionId)) | |||
There was a problem hiding this comment.
Check for understanding:
The build is "completed" only after _completionEvent is set, so this is called before any build displays whether it has been completed. _completionInvoked is initially set to 0, so if the build has completed, checked via line 196, then it checks if we already knew the build was completed with the Interlocked.Exchange check, and if we hadn't already done completion-related things, we trigger them by setting _completionEvent. This check to see if we had logged errors without failing comes immediately before that, so before any BuildSubmission can be marked complete, it has to check whether it has logged any errors (or warnings converted to errors), hence fixing the bug. Is that right?
There was a problem hiding this comment.
To be a bit pedantic, you say "build" several times when it would be more correct to say "BuildSubmission". You can have multiple submissions in a single Begin/EndBuild pair, and that's done in practice in, e.g., the VS solution build--so that all of the different projects share project state across a VS build operation, but have their own Submissions (and so the project system can know when each project individually finishes and schedule the ones that depend on it).
Co-authored-by: Forgind <Forgind@users.noreply.github.com>
|
Hi, which dotnet cli version this should be taken into effect? |
|
@roaladje .NET SDK 5.0.200 and higher should have it. |
Thanks! works! |
Fix #5837 by moving the promotion of warnings to build failures to the
BuildSubmissionlevel instead of doing it inBuildManager(where the result change happened atEndBuild()time and thus wasn't visible when checking the result after the submission completed:msbuild/src/MSBuild/XMake.cs
Lines 1256 to 1268 in c70d520