Skip to content

Preserve host-injected ProjectData targets - #56516

Closed
rmarinho wants to merge 1 commit into
mainfrom
rmarinho-preserve-host-projectdata
Closed

rmarinho wants to merge 1 commit into
mainfrom
rmarinho-preserve-host-projectdata

Conversation

@rmarinho

@rmarinho rmarinho commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Summary

Why

The SDK imports its bundled ProjectData targets after the common-targets hooks used by IDE hosts. That replaces an already-injected writer with the SDK's copy. With .NET 11 RC2 and C# Dev Kit, this drops generated assembly platform attributes and iOS/MacCatalyst project slices, producing false-positive CA1416 warnings and missing platforms.

The unconditional SDK-bundled ProjectData import was introduced by #56243, merged on September 18, 2026. This is a likely source of the RC2 recurrence because the later import can replace the host writer. The original August 24 CA1416 report predates that change, so this attribution concerns the writer-precedence regression and recurrence rather than the original issue.

Addresses the RC2 recurrence investigated from microsoft/vscode-dotnettools#3437.

What

Import the SDK-bundled writer only when _ProjectDataTaskAssembly is unset. This preserves an already-imported host writer without requiring host changes and retains the bundled writer for standalone SDK builds.

Add regressions for single-target, multi-target outer, and multi-target inner evaluations. Keep the existing standalone emission controls and update SDK architecture guidance.

Context freshness

  • Architecture guidance updated for writer ownership.
  • Regression tests cover host precedence and standalone behavior.
  • Existing commands, settings, dependencies, and diagnostic contracts are unchanged.

Validation

  • Full SDK build passed on macOS ARM64.
  • All 7 ProjectData cases and 8 targeted ILLink cases passed.
  • Removing only the import guard fails all 3 host regressions; all 4 standalone controls still pass.
  • Real MAUI editor validation with unmodified extensions and a private RC2 SDK copy: MainPage CA1416 warnings dropped from 4 to 0, generated AssemblyInfo was retained, and Android/iOS/MacCatalyst slices all loaded. The global SDK and project source were unchanged.
  • Windows/Linux and simulator/device launch were not tested.

Rebase update (October 7, 2026): rebased onto main at a893b313ed0806745e7b315c249ce651cd377acf; the new PR head is 5620c74ab8e3d24b392fe1329bc8ed2e88cab38e. The fix is unchanged (git range-diff reports an identical patch), and the working tree is clean. The build, test, and editor observations above predate this rebase; CI for the rebased head remains pending.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 2 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@genlu

genlu commented Oct 6, 2026

Copy link
Copy Markdown
Member

I think this is a reasonable fix. Eventually there might be pick which ever copy that's newer, but I don't see a need for that right now.

Another thing that's not mentioned in this PR so I'd like to point out is, with existing behavior, we'd end up using SDK's targets (imported later, overrides already imported ones from CDK) with CDK's task implementation (since our UsingTask have no Override="true", so the first ones from CDK win), this change would also fix this too.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@rmarinho
rmarinho force-pushed the rmarinho-preserve-host-projectdata branch from 7266093 to 5620c74 Compare October 7, 2026 11:34
rmarinho added a commit that referenced this pull request Oct 7, 2026
Backport the ProjectData host-writer precedence fix from #56516 (7266093) to release/11.0.1xx.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@rmarinho
rmarinho marked this pull request as ready for review October 7, 2026 13:09
Copilot AI balanced review requested due to automatic review settings October 7, 2026 13:09
@rmarinho
rmarinho requested review from a team, lbussell and mthalman as code owners October 7, 2026 13:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Rebased-head CI and Windows/Linux validation remain outstanding for this shared SDK import change.

Review effort: Balanced
Findings: None

What changed in this PR

Preserves host-injected ProjectData writers while retaining the SDK’s bundled writer for standalone builds.

Changes:

  • Guards the bundled import when _ProjectDataTaskAssembly is already set.
  • Adds host-precedence regressions for single-target, multi-target outer, and inner evaluations.
  • Documents writer precedence.
File Description
test/​TestAssets/​TestProjects/​ProjectDataHostTargets/​HostProjectData.targets Supplies a host-writer test fixture.
test/​Microsoft.NET.Build.Tests/​GivenThatWeWantToUseProjectData.cs Tests host precedence across three evaluation shapes.
src/​Tasks/​Microsoft.NET.Build.Tasks/​sdk/​Sdk.targets Conditionally imports the bundled writer.
.github/​memory/​ARCHITECTURE.md Documents host and SDK writer selection.

@rmarinho

rmarinho commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Closing in favor of #56522, which carries the same complete production fix and regression tests on release/11.0.1xx. The release change is intended to flow back to main through the normal inter-branch merge flow.

@rmarinho rmarinho closed this Oct 7, 2026
@rmarinho
rmarinho temporarily deployed to copilot-pat-pool October 7, 2026 13:59 — with GitHub Actions Inactive
@rmarinho
rmarinho temporarily deployed to copilot-pat-pool October 7, 2026 13:59 — with GitHub Actions Inactive

This branch was previously deployed

1 inactive deployment
copilot-pat-pool — 5620c74a Deployed Oct 7, 2026 by rmarinho via pat_pool #7981
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.

4 participants