Skip to content
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

[BuildCheck] Implement double writes analyzer #10184

Merged
merged 9 commits into from
Jun 10, 2024

Conversation

ladipro
Copy link
Member

@ladipro ladipro commented May 28, 2024

Fixes #9881

Context

We believe there is value in flagging cases where the same file is written by more than one task during a build. dotnet/source-build#4390 is an example of a recent hard-to-diagnose build issue that could have been prevented by this analyzer.

Changes Made

This PR adds a new built-in analyzer and makes a few supporting changes.

Testing

  • Existing and new unit tests.
  • Built a few larger repos with the analyzer enabled.

Notes

The changes contain a fix/workaround for #10176.

@ladipro ladipro marked this pull request as ready for review May 29, 2024 07:40
@ladipro ladipro requested a review from JanKrivanek May 29, 2024 07:40
Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Thank you!

I did just a first quick pass - but overall looks good.
I'll do more in depth review early next week

src/Build/BuildCheck/Analyzers/DoubleWritesAnalyzer.cs Outdated Show resolved Hide resolved
src/Build/BuildCheck/Analyzers/DoubleWritesAnalyzer.cs Outdated Show resolved Hide resolved
Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Thank you! Looks very good!

The only reason I'm requesting changes is the rooting of the path in the AnalyzeWrite.

Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Thank you! Awesome work!!!

@ladipro ladipro merged commit 74e23a9 into dotnet:main Jun 10, 2024
10 checks passed
@ladipro ladipro deleted the 9881-double-writes branch June 10, 2024 07:13
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.

[Built-in analyzer] The same file is written by multiple tasks during the build
3 participants