Skip to content

Comments

build: Fix parallel build race on Analyzers.Common deps file#54

Merged
DrBarnabus merged 1 commit intomainfrom
build/fix-analyzers-common-parallel-build-race
Feb 22, 2026
Merged

build: Fix parallel build race on Analyzers.Common deps file#54
DrBarnabus merged 1 commit intomainfrom
build/fix-analyzers-common-parallel-build-race

Conversation

@DrBarnabus
Copy link
Owner

@DrBarnabus DrBarnabus commented Feb 22, 2026

Summary

  • Fix inconsistent ProjectReference strategy on FunctionalTestsAnalyzers.Common: changed from SetTargetFramework to GlobalPropertiesToRemove to match all other consuming projects. The mismatch created two distinct MSBuild build configurations ({TargetFramework=netstandard2.0} vs {}) for the same single-targeted project, both writing to the same output directory — the root cause of the race.
  • Add missing SetTargetFramework on Analyzers.UnitTestsAnalyzers reference, preventing the 3-TFM test project from spawning 3 parallel builds of the multi-targeted Analyzers project.
  • Add --graph to CI build as defense-in-depth, ensuring each project+config is built exactly once in topological order.

Why the strategy differs by project type

Referenced project Targeting Strategy Why
Analyzers.Common Single (netstandard2.0) GlobalPropertiesToRemove="TargetFramework" Strips TF from global props → matches the solution's natural build config {} → MSBuild deduplicates
Analyzers, SourceGeneration Multi (net8.0;netstandard2.0) SetTargetFramework="TargetFramework=netstandard2.0" Pins to one inner build → matches the solution's dispatched inner build → avoids building unused TFMs

Test plan

  • Verified clean parallel builds pass 5/5 locally (previously failed 100% of the time)
  • Verified --graph builds pass consistently
  • Verified Analyzers.Common now builds exactly once (was twice due to config mismatch)
  • All 1,378 tests pass across all TFMs
  • dotnet pack produces package successfully

Summary by CodeRabbit

Release Notes

  • Chores
    • Optimised build graph configuration during Release builds for improved compilation efficiency.
    • Updated project reference metadata and target framework property settings across test projects to enhance build consistency and standardise project structure.

The previous fix (#37) added GlobalPropertiesToRemove="TargetFramework"
to all direct references to Analyzers.Common, but transient CI failures
persisted. Investigation revealed two root causes:

1. Missing fix on Analyzers.UnitTests -> Analyzers reference: The
   3-TFM test project (net10.0/net9.0/net8.0) passed its own
   TargetFramework to the multi-targeted Analyzers project without
   SetTargetFramework, creating 3 parallel build configurations that
   all converged on Analyzers.Common simultaneously.

2. Inconsistent ProjectReference strategies across consuming projects:
   FunctionalTests used SetTargetFramework="TargetFramework=netstandard2.0"
   on its Analyzers.Common reference while all other projects used
   GlobalPropertiesToRemove="TargetFramework". These produce different
   MSBuild global property sets ({TargetFramework=netstandard2.0} vs {})
   for the same single-targeted project, creating two distinct build
   configurations both writing to the same output directory — the actual
   source of the race condition.

The correct strategy depends on whether the referenced project is
single-targeted or multi-targeted:

- Single-targeted (Analyzers.Common): Use GlobalPropertiesToRemove to
  strip TargetFramework. This produces a config matching the solution's
  natural build, so MSBuild deduplicates correctly.

- Multi-targeted (Analyzers, SourceGeneration): Use SetTargetFramework
  to pin to netstandard2.0. This matches one of the inner builds
  dispatched by the solution's outer multi-target build.

Additionally, --graph is added to the CI build command as
defense-in-depth. Graph builds evaluate the full project dependency
graph upfront and ensure each project+config is built exactly once in
topological order.
@coderabbitai
Copy link

coderabbitai bot commented Feb 22, 2026

Walkthrough

Three targeted configuration adjustments: CI workflow adds --graph flag to Release builds, unit test project adds target framework metadata to analyzer reference, and functional test project modifies target framework property handling for analyzer dependencies.

Changes

Cohort / File(s) Summary
CI Build Configuration
.github/workflows/ci.yml
Added --graph flag to dotnet build command during Release build step.
Project Reference Metadata
src/CompositeKey.Analyzers.UnitTests/CompositeKey.Analyzers.UnitTests.csproj, src/CompositeKey.SourceGeneration.FunctionalTests/CompositeKey.SourceGeneration.FunctionalTests.csproj
Updated ProjectReference attributes for analyzer dependencies: unit tests add SetTargetFramework="TargetFramework=netstandard2.0", while functional tests replace it with GlobalPropertiesToRemove="TargetFramework".

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Poem

🐰 A rabbit hops through build graphs bright,
With flags and references set just right,
Target frameworks dance and sway,
Build configuration finds its way!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title clearly describes the main change: fixing a parallel build race issue related to Analyzers.Common dependencies file. It directly matches the core problem addressed in the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch build/fix-analyzers-common-parallel-build-race

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Feb 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.66%. Comparing base (2162306) to head (6b04630).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #54   +/-   ##
=======================================
  Coverage   87.66%   87.66%           
=======================================
  Files          35       35           
  Lines        2099     2099           
  Branches      344      344           
=======================================
  Hits         1840     1840           
  Misses        157      157           
  Partials      102      102           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
src/CompositeKey.Analyzers.UnitTests/CompositeKey.Analyzers.UnitTests.csproj (1)

38-38: Line 38 exceeds the 120-character limit — consider splitting the attributes.

The changed ProjectReference element runs to ~146 characters. As per coding guidelines, **/*.{cs,csproj,props} files enforce a 120-character maximum line length.

♻️ Proposed split
-        <ProjectReference Include="..\CompositeKey.Analyzers\CompositeKey.Analyzers.csproj" SetTargetFramework="TargetFramework=netstandard2.0" />
+        <ProjectReference
+            Include="..\CompositeKey.Analyzers\CompositeKey.Analyzers.csproj"
+            SetTargetFramework="TargetFramework=netstandard2.0" />
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/CompositeKey.Analyzers.UnitTests/CompositeKey.Analyzers.UnitTests.csproj`
at line 38, The ProjectReference element on the given csproj exceeds 120 chars;
split its attributes into multiple lines to keep each line under the limit — for
example, break the single <ProjectReference
Include="..\CompositeKey.Analyzers\CompositeKey.Analyzers.csproj"
SetTargetFramework="TargetFramework=netstandard2.0" /> into a multi-line element
by placing Include and SetTargetFramework on separate lines (or convert
SetTargetFramework to a child element) so the ProjectReference tag and its
attributes no longer exceed 120 characters.
src/CompositeKey.SourceGeneration.FunctionalTests/CompositeKey.SourceGeneration.FunctionalTests.csproj (1)

36-36: Line 36 exceeds the 120-character limit — consider splitting the attributes.

The updated ProjectReference element runs to ~207 characters. As per coding guidelines, **/*.{cs,csproj,props} files enforce a 120-character maximum line length.

♻️ Proposed split
-        <ProjectReference Include="..\CompositeKey.Analyzers.Common\CompositeKey.Analyzers.Common.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" GlobalPropertiesToRemove="TargetFramework" />
+        <ProjectReference
+            Include="..\CompositeKey.Analyzers.Common\CompositeKey.Analyzers.Common.csproj"
+            OutputItemType="Analyzer"
+            ReferenceOutputAssembly="false"
+            GlobalPropertiesToRemove="TargetFramework" />
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/CompositeKey.SourceGeneration.FunctionalTests/CompositeKey.SourceGeneration.FunctionalTests.csproj`
at line 36, The ProjectReference element on line 36 is over the 120-character
limit; replace the single long attribute list with a multi-line ProjectReference
element so each setting is on its own line. Change the one-line tag that
contains
Include="..\CompositeKey.Analyzers.Common\CompositeKey.Analyzers.Common.csproj"
OutputItemType="Analyzer" ReferenceOutputAssembly="false"
GlobalPropertiesToRemove="TargetFramework" into a block like <ProjectReference
Include="..."> with child elements for OutputItemType, ReferenceOutputAssembly,
and GlobalPropertiesToRemove so the attributes are split across multiple lines
and no line exceeds 120 characters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@src/CompositeKey.Analyzers.UnitTests/CompositeKey.Analyzers.UnitTests.csproj`:
- Line 38: The ProjectReference element on the given csproj exceeds 120 chars;
split its attributes into multiple lines to keep each line under the limit — for
example, break the single <ProjectReference
Include="..\CompositeKey.Analyzers\CompositeKey.Analyzers.csproj"
SetTargetFramework="TargetFramework=netstandard2.0" /> into a multi-line element
by placing Include and SetTargetFramework on separate lines (or convert
SetTargetFramework to a child element) so the ProjectReference tag and its
attributes no longer exceed 120 characters.

In
`@src/CompositeKey.SourceGeneration.FunctionalTests/CompositeKey.SourceGeneration.FunctionalTests.csproj`:
- Line 36: The ProjectReference element on line 36 is over the 120-character
limit; replace the single long attribute list with a multi-line ProjectReference
element so each setting is on its own line. Change the one-line tag that
contains
Include="..\CompositeKey.Analyzers.Common\CompositeKey.Analyzers.Common.csproj"
OutputItemType="Analyzer" ReferenceOutputAssembly="false"
GlobalPropertiesToRemove="TargetFramework" into a block like <ProjectReference
Include="..."> with child elements for OutputItemType, ReferenceOutputAssembly,
and GlobalPropertiesToRemove so the attributes are split across multiple lines
and no line exceeds 120 characters.

@DrBarnabus DrBarnabus merged commit d922876 into main Feb 22, 2026
6 checks passed
@DrBarnabus DrBarnabus deleted the build/fix-analyzers-common-parallel-build-race branch February 22, 2026 17:05
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.

1 participant