Enable -mt in PR CIs with 2 stages#13678
Conversation
Mirrors the existing Linux BootstrapMSBuildWithMTMode job for Windows so
that PR CI exercises stage-2 builds with /mt on both platforms.
Two parts:
1. eng/cibuild_bootstrapped_msbuild.ps1
* Export DOTNET_HOST_PATH (mirrors cibuild_bootstrapped_msbuild.sh:96).
/mt routes every unmigrated task to a sidecar TaskHost via
NodeProviderOutOfProcTaskHost.ResolveAppHostOrFallback, which prefers
the SDK apphost (MSBuild.exe) and needs DOTNET_ROOT in the child env.
The SDK CLI (`dotnet msbuild`) sets DOTNET_HOST_PATH automatically;
`dotnet exec MSBuild.dll` (used by this script) does not, so we set
it here explicitly.
* Add -stage2Properties parameter (mirrors --stage2Properties in the
bash script). Lets callers pass /mt to stage 2 only without
contaminating the stage 1 build that uses the SDK MSBuild.
* Add -skipTests switch (mirrors --skipTests in the bash script).
2. .vsts-dotnet-ci.yml
* Add BootstrapMSBuildWithMTModeOnWindows job, mirroring the existing
BootstrapMSBuildWithMTMode (Linux) job.
Local verification: the Debug stage-2 build with /mt produces shipping
binaries that are byte-for-byte identical to the same build without /mt
across all 1,628 files in artifacts/bin/{Microsoft.Build*, MSBuild*,
StringTools, MSBuild.Bootstrap, MSBuildTaskHost} and across every file
inside the four shipping nupkgs. The C# compiler is invoked the same
242 times with byte-identical command lines.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds an `additionalBuildArgs` template parameter to `.vsts-dotnet-build-jobs.yml`, defaulted to empty, and sets it to `/mt` from `.vsts-dotnet-exp-perf.yml` only. The official `.vsts-dotnet.yml` pipeline is intentionally untouched in this commit; it will follow as a stacked change once the experimental run validates parity end-to-end. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Follow-up on review feedback: drop the two dedicated BootstrapMSBuildWithMTMode* jobs and instead append `-stage2Properties /mt` (`--stage2Properties /mt` for bash) to every existing bootstrapped PR CI job so each job exercises /mt in its stage 2 (with the test suite still running, unlike the dedicated MT job which had to skip tests). Affected jobs: * BootstrapMSBuildOnFullFrameworkWindows * BootstrapMSBuildOnCoreWindows * CoreBootstrappedOnLinux * CoreOnMac Removed jobs (now redundant): * BootstrapMSBuildWithMTMode (Linux dedicated MT mode) * BootstrapMSBuildWithMTModeOnWindows (added earlier in this PR) No /p:BuildAnalyzer=true added — the analyzer is independent of /mt and out of scope for this change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Correctness & Edge Cases — ISSUE
-skipTests incorrectly suppresses bootstrap creation (diverges from .sh reference)
The new PS1 code collapses $onlyDocChanged and $skipTests into one branch that always passes /p:CreateBootstrap=false:
if ($onlyDocChanged -or $skipTests) {
& Build.ps1 -restore -build -ci /p:CreateBootstrap=false /nr:false ...
}The shell-script reference (cibuild_bootstrapped_msbuild.sh) keeps them separate:
onlyDocChanged=1→ passes/p:CreateBootstrap=false(bootstrap skipped)skipTests=true→ does not pass/p:CreateBootstrap=false(bootstrap IS created, tests omitted)
Concrete failing call: .\cibuild_bootstrapped_msbuild.ps1 -skipTests
Observed: /p:CreateBootstrap=false is injected → bootstrap artifacts are not created.
Expected: Bootstrap is created, -test is simply omitted (matching .sh behaviour).
Any downstream step that consumes the bootstrap (including the /mt task-host scenario this PR specifically enables) will fail with a missing-bootstrap error.
Fix — separate the two cases to match the shell script:
if ($onlyDocChanged) {
& $PSScriptRoot\Common\Build.ps1 -restore -build -ci /p:CreateBootstrap=false /nr:false `@properties` `@stage2Args`
}
elseif ($skipTests) {
& $PSScriptRoot\Common\Build.ps1 -restore -build -ci /nr:false `@properties` `@stage2Args`
}
else {
& $PSScriptRoot\Common\Build.ps1 -restore -build -test -ci /nr:false `@properties` `@stage2Args`
}All other items reviewed (empty/whitespace $stage2Properties guard, [bool] -or short-circuit, env-var assignment) are correct.
Note
🔒 Integrity filter blocked 1 item
The following item were blocked because they don't meet the GitHub integrity level.
- #13678
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
To allow these resources, lower min-integrity in your GitHub frontmatter:
tools:
github:
min-integrity: approved # merged | approved | unapproved | noneGenerated by Expert Code Review (on open) for issue #13678 · ● 5.7M
|
Refreshing PR (head should be e05c9c0: switch every PR CI bootstrapped job to -mt in stage 2 and remove the dedicated MT jobs). |
…fix -skipTests branch (dotnet#13602) Three follow-ups from review: 1. Restore the dedicated `BootstrapMSBuildWithMTMode` (Linux Core) job that runs `--stage2Properties /mt /p:BuildAnalyzer=true --skipTests`. This validates the ThreadSafeTaskAnalyzer in addition to /mt; the regular bootstrapped jobs do not enable the analyzer because it is more expensive. 2. Add `/mt` to `FullReleaseOnWindows` (both cibuild.cmd invocations). Now every PR-CI job that builds the repo runs with /mt, except the Source-Build template (which is intentionally untouched). 3. Fix a regression in `cibuild_bootstrapped_msbuild.ps1` where `-skipTests` accidentally also passed `/p:CreateBootstrap=false`, diverging from the bash reference. Restored the bash semantics: - onlyDocChanged=1 → bootstrap not created - skipTests → bootstrap IS created, tests omitted - default → bootstrap created, tests run Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…le-mt-windows-pr-ci
…dotnet#13602) Revert /mt from pipelines that run a single MSBuild invocation against the agent's stable VS MSBuild (no bootstrap). Those would test whatever /mt support the agent's pre-installed MSBuild has, not the freshly built one. Reverted: * .vsts-dotnet-ci.yml: FullReleaseOnWindows (cibuild.cmd, single-stage with VS MSBuild) * azure-pipelines/.vsts-dotnet-exp-perf.yml: experimental insertion (CIBuild.cmd, single-stage with VS MSBuild) * azure-pipelines/.vsts-dotnet-build-jobs.yml: drop the now-unused additionalBuildArgs template parameter Kept: * All 4 bootstrapped PR-CI jobs (Linux/macOS/Windows Core/Windows Full) — these do stage 1 with VS/SDK MSBuild then stage 2 with the freshly built bootstrapped MSBuild, which is the correct surface to exercise /mt. * Dedicated BootstrapMSBuildWithMTMode (Linux + BuildAnalyzer=true) — unchanged. Also clarify the script comment to call out that the apphost path requires DOTNET_HOST_PATH, and explain why we don't switch to `dotnet msbuild` (the SDK CLI mangles `/mt` argument splitting). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e token (dotnet#13602) `stage2Properties -split '\s+'` returns a string when there is exactly one token, and `& cmd @stringVar` then splats that string's characters one per argument — so `-stage2Properties "/mt"` reaches MSBuild as `/`, `m`, `t`. Wrapping the result in @() keeps it an array (length 0/1/N all behave the same), and `/mt` flows through unchanged. Found by interactive repro: $a = "/mt" -split '\s+' | Where-Object { $_ } # → string "/mt" & some.ps1 @A # → arg list: "/", "m", "t" $a = @("/mt" -split '\s+' | Where-Object { $_ }) # → array ["/mt"] & some.ps1 @A # → arg list: "/mt" Verified end-to-end with `cibuild_bootstrapped_msbuild.ps1 -skipTests -stage2Properties "/mt"` succeeding (Build succeeded, 0 errors). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Per Rainer's suggestion. Cleaner: the SDK CLI sets DOTNET_HOST_PATH for the MSBuild process automatically, so we don't need to export it ourselves and can drop the Versions.props lookup for the bootstrap SDK MSBuild.dll path. Verified locally that /mt flows through unmangled (no MSB1001, no `/m t` argument mangling) and the build produces the same outputs: binlog shows 243 Csc invocations, no errors. The earlier suspicion that `dotnet msbuild` mangled /mt was a separate bug — fixed in 73362b3 (stage2Args splat). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CI on the previous commit revealed Windows test failures (MSB4216 launching .NET task hosts, mostly net472 x86 testhosts spawning .NET Core MSBuild grandchildren under /mt). Linux/macOS pass because cibuild_bootstrapped_msbuild.sh has been exporting DOTNET_HOST_PATH at line 96 all along. Restore the export in the .ps1 to match. Keep the `dotnet msbuild` invocation from the prior cleanup — it's orthogonal: the SDK CLI sets DOTNET_HOST_PATH for the MSBuild process it spawns, but does not propagate it into the parent script environment, which is what test grandchildren inherit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…dotnet#13602) Option A (restoring DOTNET_HOST_PATH script-env export) didn't fix the Windows Core / Windows Full failures. Both jobs still fail in the same SDK-style EndToEnd tests (net10.0 x64) under /mt, while the same tests pass on Linux Core. The non-SDK variants and net472 x86 testhost all pass. Reverting `dotnet msbuild` → `dotnet exec MSBuild.dll` brings the .ps1 back to the bash script's exact shape (which works on Linux/macOS). This is the lowest-risk way to determine whether the SDK CLI's argument forwarding contributes to the test process tree breakage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Closes #13602 (PR-CI portion). Turns on
/mtin the bootstrapped stage 2 of every existing PR-CI job that already runs a 2-stage build with the freshly built MSBuild.The original PR also touched
.vsts-dotnet.yml(official build) and.vsts-dotnet-exp-perf.yml(experimental insertion) and the cibuild.cmd-basedFullReleaseOnWindowsjob; those were reverted because they all run a single MSBuild invocation against the agent's stable VS MSBuild (no bootstrap re-invocation), so adding/mtthere would test whatever MT support the pre-installed MSBuild has, not the freshly built one. Converting those to true 2-stage flows is a separate, larger change. The previously stacked PR #13679 was closed for the same reason.What this PR does
.vsts-dotnet-ci.ymlBootstrapMSBuildOnFullFrameworkWindows(Windows Full)-stage2Properties /mtBootstrapMSBuildOnCoreWindows(Windows Core)-stage2Properties /mtCoreBootstrappedOnLinux(Linux Core)--stage2Properties '/mt'CoreOnMac(macOS Core)--stage2Properties '/mt'BootstrapMSBuildWithMTMode(Linux dedicated MT + ThreadSafeTaskAnalyzer)/p:BuildAnalyzer=trueFullReleaseOnWindows(single-stagecibuild.cmd)Tests still run in every job that ran them before. The dedicated
BootstrapMSBuildWithMTModekeeps--skipTests(as onmain).Files changed
eng/cibuild_bootstrapped_msbuild.ps1-stage2Propertiesparameter (mirrors--stage2Propertiesin the bash script). Lets callers pass/mtto stage 2 only, not to the SDK MSBuild used in stage 1.-skipTestsswitch (mirrors--skipTests).cibuild_bootstrapped_msbuild.sh):-onlyDocChanged→/p:CreateBootstrap=false(bootstrap not created, downstream doesn't need it).-skipTests→ bootstrap is created (downstream may consume it), tests omitted.dotnet.exe msbuild(SDK CLI verb) instead ofdotnet.exe exec MSBuild.dll. The CLI setsDOTNET_HOST_PATHfor the MSBuild process automatically, which/mtneeds so the apphost-based child task host (NodeProviderOutOfProcTaskHost.ResolveAppHostOrFallback) can locate the runtime. Removes theVersions.propslookup for the bootstrap SDK'sMSBuild.dllpath./mtargument splatting:"/mt" -split '\s+' | Where-Object { $_ }returns astring(notstring[]) when there is exactly one token, and& cmd @stringVarthen splats the string's characters one per arg — so/mtreached MSBuild as/,m,t. Wrapping with@()keeps the result an array for 0/1/N tokens. This was caught locally; would have failed CI..vsts-dotnet-ci.yml/mtto every stage 2 invocation listed above. Linux MT-with-analyzer job unchanged.Switched to
dotnet msbuild(per Rainer's suggestion)The script now runs the bootstrap as
dotnet.exe msbuild(the SDK CLI verb) rather thandotnet exec MSBuild.dll. The CLI setsDOTNET_HOST_PATHfor the MSBuild process automatically, so the explicit env-var export is gone, along with theVersions.propslookup for the bootstrap SDK's MSBuild.dll path.I initially thought the SDK CLI mangled
/mtinto/mt, but that turned out to be the splat bug above — once fixed,/mtflows throughdotnet msbuildunchanged. Verified by parsing the resulting binlog: 243 Csc invocations, noMSB1001, no error.Parity proof
Built
main@0f09213370(Debug) end-to-end three times on Windows, with the bootstrapped stage 2 each time:cd repo)baseline_no_mteng/cibuild_bootstrapped_msbuild.ps1 -msbuildEngine dotnet -onlyDocChanged $true -configuration Debugwith_mt/mt: same script, with-buildStage1 $false -stage2Properties "/mt"baseline2_no_mt/mt: same script, with-buildStage1 $falseAll three reuse the same stage-1 bootstrap, same source, same configuration, same
ContinuousIntegrationBuild=true. The only varying input inwith_mtis/mt.baseline2_no_mtis the control to measure pre-existing build non-determinism.Hash-compared every file in
artifacts/(excludinglog/,tmp/,obj/,SymStore/-staging,xsd/cache,toolset/,TestResults/).Result matrix (
baseline_no_mtvswith_mt)artifacts/)bin\Microsoft.Build*,bin\MSBuild*,bin\StringTools,bin\MSBuild.Bootstrap,bin\MSBuildTaskHost)psmdcpfilename randomization).nupkgZIP raw SHASymStore/(the symbol-server layout)xsd/(IntelliSense schemas published to VSSetup)Repeating the same comparison between
baseline_no_mtvsbaseline2_no_mt(both-mp) yielded the exact same 15 differing files: 4 shipping nupkgs, 4 test CustomCheck nupkgs, 6 test DLLs, 1.lnkshortcut. So none of the 15 diffs is/mt-induced — they reproduce between two-mpbuilds.Per-shipping-nupkg breakdown
Each
.nupkgwas unzipped and compared file-by-file. After normalizing the random<32hex>.psmdcpfilename and the randomId="R<16hex>"in_rels/.rels:Microsoft.Build.18.7.0-ci-26254-01.nupkgMicrosoft.Build.Framework.18.7.0-ci-26254-01.nupkgMicrosoft.Build.Templates.18.7.0-ci-26254-01.nupkgMicrosoft.NET.StringTools.18.7.0-ci-26254-01.nupkgCovers every shipping
.dll,.pdb, ref assembly, XML doc, nuspec, transitive build target, and.tlbwe ship in the package family.Compiler-invocation proof from the binlogs
Both builds emit
artifacts/log/Debug/Build.binlog. I parsed each with the freshly-builtMicrosoft.BuildBinaryLogReplayEventSource, captured everyTaskCommandLineEventArgs, filtered toCsc/Vbc/Fsc, sorted by(project file, command line), and dumped to a text file:Macro stats from each binlog:
ProjectStartedeventsCsc+Vbc+FscinvocationsMSBuildtask invocationsCopytask invocationsWriteCodeFragmentWhat the 15 file diffs are (all pre-existing non-determinism, not
/mt)Verified by the
baseline_no_mtvsbaseline2_no_mt(-mpvs-mp) comparison:psmdcpfilename randomization in 4 shipping + 4 test nupkgs. NuGet writespackage/services/metadata/core-properties/<random32hex>.psmdcpand a matching randomIdin_rels/.rels. Same diff between two-mpbuilds. The.psmdcpcontent is identical; only its filename and the relationship Id differ.TimeDateStampin 6 test DLLs (Microsoft.Build.Engine.UnitTests,Microsoft.Build.Tasks.UnitTests,Microsoft.Build.CommandLine.UnitTests, each innet10.0andnet472). Test DLLs are not shipped. Verified withSystem.Reflection.Metadata.PEReaderthat PdbChecksum, CodeView GUID, embedded-PDB size, and every other byte of the assembly match — only the MVID and timestamp differ. Reproduces between two-mpbuilds.VS with MSBuild.slnx.lnkWindows shortcut file regenerated each build.Local verification of the script change
After all script changes, with no environment hacks:
Result:
(213 warnings are pre-existing analyzer warnings; same set/count appears in the no-
/mtbuild.)