Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Fix /warnaserror false positive overall success - #8953

#8953
Merged
JaynieBai merged 1 commit into
dotnet:maindotnet/msbuild:mainfrom
rokonec:rokonec/fix-warnaserror-flaky-bugrokonec/msbuild:rokonec/fix-warnaserror-flaky-bugCopy head branch name to clipboard
Jun 27, 2023
Merged

Fix /warnaserror false positive overall success#8953
JaynieBai merged 1 commit into
dotnet:maindotnet/msbuild:mainfrom
rokonec:rokonec/fix-warnaserror-flaky-bugrokonec/msbuild:rokonec/fix-warnaserror-flaky-bugCopy head branch name to clipboard

Conversation

@rokonec

@rokonec rokonec commented Jun 26, 2023

Copy link
Copy Markdown
Member

Fixes #8735, possibly #5689, #2845

Context

There were race condition when ProjectFinished log message has been processed after BuildResult event. Even though ProjectFinished log message event is always send before BuildResult event from Node, because ProjectFinished is asyncronously routed by LoggingService reversing order can and do often happen.

Changes Made

Updating _overallBuildSuccess in one place when submission is considered to be done and assumed its final state.

Testing

Can't local repro.

where submission is considered to be done and assumed its final state
@rokonec
rokonec requested a review from AR-May June 26, 2023 09:35

@AR-May AR-May left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@JanKrivanek JanKrivanek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Comment thread src/Build/BackEnd/BuildManager/BuildManager.cs
@JaynieBai
JaynieBai merged commit c8c2aaa into dotnet:main Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: /warnaserror false positive overall success

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.