[fix] Skip MsCoverageReferencedPathMaps target when building with MSBuild graph build/isolation#16032
[fix] Skip MsCoverageReferencedPathMaps target when building with MSBuild graph build/isolation#16032nohwnd wants to merge 2 commits into
Conversation
…olation Fixes #5190 When MSBuild is invoked with -graphBuild:true and -isolateProjects:true, the MsCoverageReferencedPathMaps target triggers an isolation violation by calling MSBuild on referenced projects with Properties= (global properties). Add IsGraphBuild condition to skip the target in that mode. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes MSBuild project isolation failures that occur when building with -graphBuild:true -isolateProjects:true by skipping the MsCoverageReferencedPathMaps target (and its supporting properties) during graph builds, avoiding sub-MSBuild invocations that alter global properties.
Changes:
- Add
and '$(IsGraphBuild)' != 'true'to the conditionalPropertyGroupthat configures source-root mapping behavior. - Add
and '$(IsGraphBuild)' != 'true'to theMsCoverageReferencedPathMapstarget condition so it is not executed during graph builds.
nohwnd
left a comment
nohwnd
left a comment
There was a problem hiding this comment.
Review Summary
The fix is correct and well-structured for the primary reported scenario (graph build mode). Two notes:
✅ Condition symmetry
Both the <PropertyGroup> (which sets _msCoverageSourceRootTargetName) and the <Target> guard are updated with the same condition. This is consistent — if the PropertyGroup is skipped, the Target is too, preventing any reference to an undefined property.
⚠️ Possible incomplete coverage for pure isolation mode
$(IsGraphBuild) is only set to true when MSBuild is invoked with -graphBuild. The PR title mentions "graph build/isolation", but a user who uses -isolateProjects:true without -graphBuild:true would still hit the same isolation violation — the <MSBuild ... Properties="TargetFramework=..."> sub-project call still sets a global property that violates the isolation contract.
There is no well-known stable MSBuild property for pure isolation mode (analogous to IsGraphBuild), so this may be an acceptable limitation. The existing $(DisableMsCoverageReferencedPathMaps) escape hatch covers this remaining case if users encounter it. Consider documenting this in the PR description or a comment in the .targets file so future maintainers understand the known gap.
✅ No package content changes
No files added or removed from the package; eng/expected-nupkg-file-counts.json does not need updating.
🧠 Reviewed by Expert Code Reviewer
🧠 Reviewed by Expert Code Reviewer 🧠
This comment has been minimized.
This comment has been minimized.
|
The latest CI run (build 1424027, 2026-05-18) shows the macOS job was cancelled after ~60 minutes while Windows Release ✅ and Ubuntu ✅ both passed successfully. This appears to be an infrastructure timeout/flake — the change in this PR ( When CI is re-queued, this PR should go green.
|
…ojects without -graphBuild) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Commit pushed:
|
nohwnd
left a comment
nohwnd
left a comment
There was a problem hiding this comment.
Review Summary
The updated diff addresses the .targets file documenting why $(IsGraphBuild) is used, the trade-off (no msCoverageSourceRootsMapping_* files in graph build mode), and the $(DisableMsCoverageReferencedPathMaps) escape hatch for users on -isolateProjects without -graphBuild.
✅ All prior findings resolved
- Condition symmetry between
<PropertyGroup>and<Target>remains correct. - Comment now documents the known gap for pure isolation mode — future maintainers have the context they need.
- No package content changes;
eng/expected-nupkg-file-counts.jsonstill does not need updating.
No new issues found. The fix is correct and the documentation is clear.
🧠 Reviewed by Expert Code Reviewer
🧠 Reviewed by Expert Code Reviewer 🧠
🧠 Reviewed by Expert Code Reviewer 🧠
Fixes #5190
Root Cause
The
MsCoverageReferencedPathMapstarget inMicrosoft.CodeCoverage.targetscallsMSBuildon referenced projects withProperties="TargetFramework=...". This sets a global property on those sub-project invocations, which violates MSBuild's project isolation contract when building with-isolateProjects:true -graphBuild:true.MSBuild pre-computes the project graph with fixed global properties per project. Any build step that sets new global properties on a project outside the pre-computed graph triggers the isolation error: "global properties set for each project have been altered during the build."
Fix
Add
'$(IsGraphBuild)' != 'true'to the conditions on both the<PropertyGroup>and the<Target>. MSBuild sets$(IsGraphBuild)totruewhen invoked with-graphBuild:true, so this skips the deterministic source mapping step in graph build mode.This is consistent with the existing
$(DisableMsCoverageReferencedPathMaps)escape hatch — the trade-off is thatmsCoverageSourceRootsMapping_*files are not generated during graph builds, but the build succeeds without isolation violations.Changes
src/package/Microsoft.CodeCoverage/Microsoft.CodeCoverage.targets: Addedand '$(IsGraphBuild)' != 'true'condition toMsCoverageReferencedPathMapstarget and its associated<PropertyGroup>.