[Extensibility] SingleProject: Fix dual-path backend activation and Compile filtering - #36957
Conversation
Default backend activation for dual-path registrations and constrain platform Compile removal to the true metadata batch without perturbing unrelated items. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ac6265fe-09a0-473c-b3f9-9ec6d6dd97cd
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36957Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36957" |
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
This PR adjusts SingleProject’s platform-folder activation/removal logic to support dual-path backend activation (recognized TPI vs neutral-TFM selector) and to make platform Compile removal respect MSBuild batching semantics to avoid removing explicitly-included items.
Changes:
- Default
ActivationValuefromBackendIdentityeven whenTargetPlatformIdentifier(s)are present, enabling a single registration to activate via either recognized TPI or neutral-TFM backend selection. - Rework
_MauiRemovePlatformCompileItemsto start from the@(Compile)batch and intersect with physicalPlatforms/**candidates, preventing “false” metadata buckets from being re-discovered and removed.
Show a summary per file
| File | Description |
|---|---|
| src/Controls/tests/Xaml.UnitTests/MSBuild/MSBuildTests.cs | Adds regressions covering dual-path backend activation and Compile removal batching correctness. |
| src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.SingleProject.targets | Updates platform Compile removal to intersect the true @(Compile) batch with physical platform-folder candidates. |
| src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.SingleProject.Before.targets | Changes normalization so ActivationValue is derived from BackendIdentity regardless of TPI metadata presence. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 2
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 66f84348-6476-4097-8b7f-f240338e85c3
This comment has been minimized.
This comment has been minimized.
|
Prior main CI |
|
Current-head main |
kubaflo
left a comment
There was a problem hiding this comment.
Multi-model adversarial review — #36957 (round 1, head 256a4728)
Verdict: code is LGTM on the merits — but held from approval by a pre-existing net11.0 base breakage in CI (not caused by this PR).
Reviewed with Claude Opus 4.8 + GPT-5.5 + Gemini 3.1 Pro + GPT-5.6 Sol, each independent, then cross-pollinated and independently re-verified against the code at 256a4728.
Context
Sibling PR #36956 (the near-duplicate competing implementation) was closed/consolidated into this one, exactly as expected — so this is now the single surviving fix. This head ("Harden SingleProject backend filtering") adds a perf guard + two new tests on top of the reviewed cab41ff0.
Code review — the crux is correct
- Fix B (
_MauiRemovePlatformCompileItems) now uses consistentMatchOnMetadata="FullPath" MatchOnMetadataOptions="PathLike"on every<Compile Remove>, including the final one. This is precisely what the closed #36956 got wrong (it mixed a plain identity match on the final remove withFullPathelsewhere, leaking absolute /./-prefixed excluded platform files). Opus empirically reproduced viadotnet msbuildthat all four path forms (relative, absolute,./-prefixed, back/forward-slash) are removed here. - Fix A (activation back-fill in
.Before.targets) — the shared "over-activation" risk raised earlier is disproven: the collect-time inclusion condition's neutral clause is gated by'$(TargetPlatformIdentifier)' == '', so it is dead in any real (TPI-set) Windows/Android/iOS build. - Harden delta (
cab41ff0 → 256a4728) is production-safe: the filesystem glob is now gated behind'@(_MauiPlatformCompileToRemove)' != ''(pure optimization — only globs when there's something to remove), plus a strengthened metadata-preservation assertion and two new tests (MatchesAbsoluteCompilePath,NeutralTfmCanActivateBuiltInBackendByIdentity). - The new
MatchesAbsoluteCompilePathtest passes on the Windows Helix Unit Tests leg at this head (it was RED on the closed #36956), empirically settling the absolute-path concern.
Ensemble: Opus LGTM, Gemini LGTM, GPT-5.5 0 blocking. Sol raises one non-blocking, theoretical concern (below).
💡 Non-blocking (Sol) — same-file dual-alias
Sol notes the final FullPath remove would also drop a keep-bucket Compile alias if the same physical file were referenced by both a keep alias and a remove alias. In practice a SingleProject platform file lives under exactly one Platforms/<platform>/ folder, so it is in the keep set or the remove set — never both. This requires a pathological hand-authored duplicate Compile item and doesn't apply to the SDK's normal layout. Flagging only as a discussion point.
CI status — the only failure is a confirmed base breakage ⚠️
maui-pr will go red, but the sole failing leg is Run Integration Tests – Samples (macOS + windows), and it is not caused by this PR:
- The failure is
CS8622nullability-mismatch errors inOrdersPage.xaml.xsg.cs/ProductPage.xaml.xsg.cs(XAML source-generated) insideControls.Sample.Sandbox, tripped by the Samples test's-p:TreatWarningsAsErrors=true. - This PR changes only 3 files —
SingleProject.targets,SingleProject.Before.targets,MSBuildTests.cs— and does not touch the Sandbox at all. - The identical
CS8622errors onOrdersPage.OnOrderTapped/ProductPage.OnGoToReviewappear on thenet11.0base branch (build1533853). Base was green at7e601062(21:59Z) and red ate45600b0(23:30Z) — a Sandbox nullability regression was introduced on base this evening, independent of this PR.
Every other relevant leg is green at 256a4728: macOS Build/Pack ×Debug/Release, Windows Build/Pack ×Debug/Release, Windows Helix Unit Tests ×Debug/Release (incl. the new absolute-path test), RunOnAndroid, all RunOniOS variants, MultiProject (macOS+windows), AOT/Blazor/Build (macOS).
Recommendation
The fix itself is ready — correct, well-tested, and the harden delta is safe. It is blocked only by an unrelated net11.0 base Sandbox breakage that will drag the overall gate red. Once the base Sandbox CS8622 regression is fixed (or this branch is rebased onto a green base), this is safe to merge. Holding approval purely on the CI gate, not on any code concern.
|
@Redth — final handoff: head |
AI Review Summary
🗂️ Review Sessions — click to expand🚦 Gate — Test Before & After FixGate Result: ✅ PASSEDPlatform: ANDROID · Base: net11.0 · Merge base:
🔴 Without fix — 📄 MSBuildTests: FAIL ✅ · 481sError-relevant lines (filtered from the build log): 🟢 With fix — 📄 MSBuildTests: PASS ✅ · 312s(no coded error found; showing last 1200 chars) 📁 Fix files reverted (2 files)
📱 UI Tests — Button,Label,LayoutDetected UI test categories: ✅ Deep UI tests — 359 passed, 0 failed across 3 categories on platform-pool agent (replaces in-process counts above). 🧪 UI Test Execution Results (deep, platform pool)
📋 Pre-Flight — Context & ValidationIssue: #36650 - SingleProject: Register and activate external backends for neutral TFMs Key Findings
Code Review SummaryVerdict: NEEDS_DISCUSSION Key code review findings:
Fix Candidates
🔬 Code Review — Deep AnalysisCode Review — PR #36957Independent AssessmentWhat this changes: Updates SingleProject MSBuild target behavior so Inferred motivation: Fix backend activation and compile-filtering regressions while preserving explicit metadata and item ordering. Reconciliation with PR NarrativeAuthor claims: This is a focused follow-up for dual-path backend activation and compile removal batching, with targeted MSBuild tests. Prior Review Reconciliation
Blast Radius Assessment
CI Status
External Output Contract
FindingsNo high-confidence code findings. Failure-Mode Probing
Verdict: NEEDS_DISCUSSIONConfidence: low 🛠️ Fix — Analysis & ComparisonFix Candidates
Candidate Detailstry-fix-1 — Split Activation TargetsApproach: Decomposed the monolithic three-branch collect condition into separate targets for legacy shared folders, recognized TPI matches, and neutral backend activation. This reduced activation-condition coupling but retained the PR's compile-removal intersection pattern. Diff: Saved at Test Results: PASS. The targeted SingleProject MSBuild/XAML unit filter passed 46/46 tests. Failure Analysis: Not failed. However, the candidate is not selected because its compile-filtering half is not meaningfully different from the PR and candidate 2 provides a simpler distinct strategy. try-fix-2 — Path-Predicate Compile FilteringApproach: Kept the PR's activation normalization, but replaced compile-removal set intersection with direct predicate filtering on current Diff: Saved at Test Results: PASS. The targeted SingleProject MSBuild/XAML unit filter passed 46/46 tests on the first run. Failure Analysis: Not failed. Self-review recorded 0 findings. The candidate directly avoids the fresh filesystem glob that caused the explicit-false compile item regression, while still handling absolute compile identities via canonical Cross-Pollination
Exhausted: No — stopped early because candidate 2 passed all targeted tests and is demonstrably better on simplicity for the compile-filtering half of the PR. 📝 Recommended PR Title & DescriptionAssessment: ✏️ Recommend updating — the current description says neutral activation of built-in backend identities is intentional, but the winning Recommended title Recommended description worked for recognized TPI Built-in platform folders continue to activate through their recognized platform TFMs. They are not selected by Before this change, the presence of any true Compile metadata batch caused a fresh TestsThe real shipping SingleProject targets are covered by regressions for:
Validation:
The unfiltered local graph is unavailable on this Mac because the platform project graph requires the iOS workload ( Candidate selection and final reviewThe selected implementation is Issues FixedPart of #34099 |
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description of Change
This focused follow-up addresses two empirically validated gaps found by the GPT-5.6 Sol post-merge verification of #36654.
BackendIdentitynow defaultsActivationValueeven whenTargetPlatformIdentifierorTargetPlatformIdentifiersis also present. A single registration can therefore activate through a recognized TPI or throughMauiActiveBackendon a neutral TFM, while explicitActivationValueandActivationPropertymetadata still win.Compileitems in theExcludeFromCurrentConfiguration=truemetadata batch and intersects them with physicalPlatforms/**candidates. A fresh filesystem glob can no longer pull an explicitly false Compile item from another metadata bucket into the removal set. The helper-item removal pattern remains in place, preserving active item order and avoiding remove/re-add duplication.Before and After
Before this change, a registration such as:
worked for recognized TPI
macosbut did not activate on a neutral TFM withMauiActiveBackend=macos. It now supports both paths and remains inactive for another recognized TPI or a mismatched backend.Before this change, the presence of any true Compile metadata batch caused a fresh
Platforms/**glob to include physical files explicitly marked false by a downstream target. Removal is now constrained to the actual true Compile identities under the platform folder.Tests
The real shipping SingleProject targets are covered by regressions for:
Validation:
MSBuildTests.SingleProject_*matrix: 46 passed, 0 failed;Controls.Build.Tasks.csprojbuild: succeeded with 0 warnings and 0 errors;dotnet format: completed successfully.The unfiltered local graph is unavailable on this Mac because the platform project graph requires the iOS workload (
NETSDK1147) and the full BuildTasks solution filter includes .NET Framework 4.7.2 projects without local reference assemblies (MSB3644). The workload-neutral matrix above used the repository-pinned .NET 11 SDK with platform TFMs disabled and imported the exact shipping targets.Candidate selection and final review
#36957 is the selected implementation. Alternatives #36955 and #36956 both fail their own new Windows Helix XAML regression by removing the explicit-false Compile item. This branch uses FullPath/PathLike intersection, passes the expanded 46-case matrix, preserves metadata and duplicate count, handles absolute Compile identities, and skips the platform filesystem glob when no removal batch exists.
Neutral activation of built-in backend identities is intentional: #36654 documented that built-ins use the same registration shape and that ActivationValue defaults from BackendIdentity. A dedicated regression now locks that contract.
Issues Fixed
Part of #34099
Part of #35021
Fixes #36650
Follow-up to #36654