[Build] Skip XamlC when no XAML files need XamlC processing - #34972
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34972Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34972" |
b38cbcf to
00912c6
Compare
28daf7d to
921a0c2
Compare
Replace the single-line header comment with a block that makes explicit the contract the surviving tests protect: SourceGen is the net11.0 default and the XamlC target must not run on default builds. Addresses reviewer feedback on dotnet#34972 that future contributors had no in-file guidance that these are negative tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review -b feature/refactor-copilot-yml |
|
|
AI code review for net11.0 targetVerdict: Needs changes (also still a Draft) Adds a condition so the legacy Key findings (description ↔ implementation mismatches)
CI note: Confidence: Medium-high on the description/implementation mismatches; the AOT failures need confirmation. This is an automated, non-approval review comment, not a GitHub approval/request-changes. A human must make the merge decision. |
AI code review refresh for net11.0 targetHead reviewed: 710be9ba7fca7cd8f9ff2d3cf5a4496e73ebf946 (this resolves to the current PR head Verdict: Needs changes — the newly added test is failing in CI for a PR-caused reason. This round-two pass focuses on the new commit since the previous fleet review: "Add positive test: XamlC runs when XamlC inflator is used" (which added Blocking finding
Production change assessment (non-blocking)
CI noteRequired Confidence: High on the failing-test root cause (deterministic This is an automated, non-approval review comment — not a GitHub approval or request-changes. A human must make the merge decision. |
XamlC's validate-only pass (Debug default) is the only build-time XAML validation for files using the Runtime or XamlC inflators. SourceGen-only files are validated by XSG itself, so XamlC is redundant for them. Gate the XamlC target on '@(_MauiXaml_XC)' != '' OR '@(_MauiXaml_RT)' != '' (equivalent to '@(_MauiXaml_AsEmbeddedResource)' != ''). Projects using SourceGen exclusively — the default in net11.0 — now skip XamlC entirely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Tests that don't care about XamlC now lock in the SourceGen default: they assert the build still succeeds and XamlC.stamp is absent. Tests whose subject is the XamlC target itself (incrementality, Clean, DesignTimeBuild contrast, AddNewFile) opt in via -p:MauiXamlInflator=XamlC -p:NoWarn=MAUI1001 so the existing contract is verifiable. Plan originally called for the SourceGen,XamlC combo, but for these synthesised projects the source generator emits a duplicate InitializeComponent (CS0111) when both inflators are active, so the bare XamlC form with NoWarn=MAUI1001 is used instead to silence the deprecation warning under the repo-wide TreatWarningsAsErrors setting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extract XamlC opt-in args + rationale to a single const so the 'why bare XamlC, not SourceGen,XamlC' explanation lives in one place. Apply the same opt-in to TouchXamlFile's body for future-proofing. Reword the misleading inline comment on the DesignTimeBuild precondition. Note: the prior commit message's TreatWarningsAsErrors rationale for NoWarn=MAUI1001 is slightly inaccurate — the synthesized projects under tempDirectory write a fresh Directory.Build.props that does not chain in the repo-root TWAE setting. NoWarn is kept anyway for noise reduction and to be robust if the chain ever changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The XamlC target is gated to SourceGen-default-empty projects in this PR's targets change. Tests that simulated XamlC running on the synthesised single-project scaffold (TargetsShouldSkip, Clean, DesignTimeBuild, AddNewFile, TouchXamlFile) were exercising a code path that no longer executes by default. Delete them along with the XamlCOptIn constant. The contract that XamlC does not run by default is already locked in by BuildAProject, LinkedFile, RandomXml, RandomEmbeddedResource (AssertDoesNotExist on XamlC.stamp), and by NoXamlFiles (log-scrape that the target isn't scheduled). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the single-line header comment with a block that makes explicit the contract the surviving tests protect: SourceGen is the net11.0 default and the XamlC target must not run on default builds. Addresses reviewer feedback on dotnet#34972 that future contributors had no in-file guidance that these are negative tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds XamlCRunsWhenXamlCInflatorUsed to verify the condition gate correctly activates XamlC when _MauiXaml_XC is populated via -p:MauiXamlInflator=XamlC. This is the positive counterpart to BuildAProject which proves XamlC is skipped under SourceGen. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The positive test passed '-warnaserror-:MAUI1001' to opt out of treating the XamlC-deprecation warning as an error. The '/warnaserror-:CODE' negation form is rejected by the rolled-forward .NET 11 preview.5 MSBuild (MSB1001: Unknown switch), failing the synthesized build before XamlC could run. Replace it with the valid '-p:WarningsNotAsErrors=MAUI1001' property, which expresses the same defensive intent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
✅ LGTM — no blocking issues foundLGTM — unanimous across all four review models (4/4), no code-level findings. This is a clean build optimization: it skips the XamlC MSBuild step when no XAML files actually need XamlC processing, avoiding unnecessary work. The change is well-scoped and correct.
Multi-model review (gpt-5.5 · opus-4.8 · opus-4.6 · gemini-3.1-pro). Comments only — not a formal approval. |
|
@StephaneDelcroix hello, could you please check if the test coverage looks good? |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
6 participants
Summary
Skip the XamlC MSBuild target when no XAML files use the XamlC or Runtime inflator — i.e., when all XAML uses the default SourceGen inflator.
Motivation
With SourceGen as the default inflator in .NET 11, the XamlC MSBuild target has nothing to do on a default build: there are no
_MauiXaml_XCor_MauiXaml_RTitems to process. Previously XamlC still ran (inValidateOnly=Truemode for Debug), loading the assembly via Cecil, parsing XAML, validating, and discarding results — all redundant because the Source Generator already validates XAML at compile time.Changes
Microsoft.Maui.Controls.targets(1 line): AddedAND ('@(_MauiXaml_XC)' != '' OR '@(_MauiXaml_RT)' != '')to the XamlC targetCondition, so it only runs when there are files that actually need XamlC processing.MSBuildTests.cs:BuildAProject,LinkedFile,RandomXml,RandomEmbeddedResource,NoXamlFiles) assertXamlC.stampdoes NOT exist under the default SourceGen inflator.XamlCRunsWhenDeprecatedInflatorUsed— a[Theory]covering bothMauiXamlInflator=XamlCand=Runtime, asserting XamlC still runs (covers both branches of the newORcondition).TargetsShouldSkip,Clean,DesignTimeBuild,AddNewFile;TouchXamlFilekept[Skip]), adapted to explicitly opt into the XamlC inflator via the newXamlCOptInconstant.Target Ordering
_MauiXamlComputeInflator(which populates_MauiXaml_XC/_MauiXaml_RT) runs before Compile viaPrepareResourcesDependsOn. XamlC runsAfterTargets="AfterCompile", so the condition is evaluated after the item groups are fully populated.Impact
MauiXamlInflator=XamlCorMauiXamlInflator=Runtimebuilds continue to work unchanged.