fix: preserve foreground stdin during vcvars bootstrap - #182
Conversation
Extract only the reviewed VisualStudioEnvironment stdin repair and its toolchain/ordinary CLI regressions from held PR #180. Redirect the noninteractive vcvars call from NUL without changing the generic runner, foreground inheritance, environment decoding or bootstrap exit status. Preserve seven real file/pipe/error17/installed-vcvars controls and cold/warm stdin, argv, cwd, environment and failure-no-launch checks. No BuildCompletion/Application/module refactor or #180 ancestry. VERSION remains5.5.0; original performance holds and release/lease limits remain.
Actual current-head stdin and foreground controls inspected in Debug and ReleaseExact head Read actual native compile/link/execution logs in Debug job102845095770 (Native#742/34468575640) and Release job102845117586 (Release#626/34468575666), outer shard3/4, subshards6/8 and7/8. Both In BOTH configurations all seven stdin evidence records pass: deliberate reader unprotected/protected on file and pipe; original bootstrap failure17 retained with unchanged input; installed-vcvars unprotected/protected. In these actual host observations, the installed unprotected call leaves 0of29bytes, and the protected product call preserves all29bytes exactly. The deliberate reader calibrates consumption. The vendor-script raw result is recorded, not assumed to occur on every vendor version. Captured environment values are never printed. Both ordinary CLI cold and warm foreground calls return the original child 37 with the expected stdout/stderr. Cold actually compiles and links; warm reports up-to-date and does not compile/link. Empty/spaced/quoted/trailing-backslash/Unicode argv, cwd, inherited environment and stdin are checked by the actual child. Both injected compile failures retain C2338/CLI4; both missing-symbol links retain LNK2019/LNK1120/CLI5, without running the previous artifact. The CLI child deliberately uses --debug --runtime MTd in both wrapper-test configurations; this is not a claim that its own build switches to Release. The two native runner images differ: Debug20260907.229.1, Release20260824.214.3, both Windows10.0.26100. Native C4996/D9025 warnings remain; no warning-free or historical-identical-host claim. Selected exact log excerpts are retained in delivery, not advertised as full CI log archives. The current independent ABBA#608 has completed and is independently recomputed:76pairs/72instrumented complete semantic identities, external no-op paired median −1.39135ms/−9.6286596%, original conjunction not crossed. Every unfavorable/favorable sample remains. The current native Release self-host/package and remaining core regression artifact reviews are still separate gates; no merge authorization is given by this intermediate comment. Original #180/#599 HOLD and VERSION5.5.0 remain unchanged. |
Independent original ABBA #608 — exact isolated fix, all samples retainedBase All76pairs/19scenarios and AB/BA/AB/BA order checked. All72instrumented complete counters/breakdowns/compile-link-archive hit/miss match. Four external timing counter sets are unavailable, not zero. Only the final row uses timings-off external elapsed; all other rows use mqb.timings. Four-pair nearest-rank P95 is merely the maximum, not a population-tail estimate. Medians and percentages are computed from paired deltas, not differences of marginal medians.
Unrounded external paired medians −1.39135ms/−9.6286596% do not cross the unchanged regression gate BOTH>1ms AND>10%. No speedup/zero-overhead claim. Cold/no-op/build-run3/4slower, link-only median+22.195ms, scale-cold+819.012ms and single-TU+539.134ms tails remain. Causes are not isolated and favorable rows do not erase adverse ones. No presumed-noise deletion, AA subtraction, threshold change or same-head remeasurement. Both unmodified historical #180 ABBA archives were separately replayed again. Original #599 remains +2.07175ms/+16.2700359633%, gate crossed; #601 remains +0.39145ms/+3.3932964684%. Neither this isolated fix nor the later measurements explain the original crossing. #180/#172 remain held; #181's fixed later investigation is not a reconstruction of old missing traces. Current comparison JSON does not archive the actual ABBA executable pair or OS trace. This closes only this head's ABBA gate. All declared current native/ownership/runtime gates still apply before merge. VERSION5.5.0, existing positive controls and release/lease limits are unchanged. |
Iviesever
left a comment
Iviesever
left a comment
There was a problem hiding this comment.
Final review — accept isolated vcvars stdin repair; do not release #180
Source/evidence self-review with separate read-only source reconstruction and artifact/statistics replay, not an independent human approval.
Base: 02f8fc9e03db789ebfea7a2462b1208a1be3ee45.
Reviewed head: 0f20c10feca7060ac9c94ea7c720d17595eb441a.
Candidate tree: 6ef8457242fa92bd076eefd396f23ac02602f540.
Native test-merge: 1eb1a707d62fc97a098e1f8f9b50dd041ad92444, identical tree and exactly those base/head parents.
Full diff is three existing files,269 additions/1 deletion. Of420files,417are byte-identical. Complete canonical source archives and Git patch replay reproduce both trees. The selected blobs exactly match the reviewed stdin-fix versions in #180, but this commit has only current main as its parent: no #180 ancestry, BuildCompletion, Application/ModuleCliTarget refactor, manifest/layout/native79 inventory or benchmark changes are imported.
Verified blobs: VisualStudioEnvironment 13a855f51f8e23fc3b0ba0d0db25e8305d0ceb6b; VS test ec38657c33487c95bcdaa2e0b9dd24d7c1fde11f; ordinary CLI test 3391f5e56072f565fe4fbca30eb695ba5e159738.
Product correctness boundary
The noninteractive environment-bootstrap call reads NUL using <nul while retaining the original stdout/stderr redirection and following errorlevel propagation. The fix does not copy/rewind user input, change generic WindowsProcessRunner stdin inheritance, alter compile/link ProcessSpecs, modify environment parsing or add cancellation/Job policy. Existing foreground consumers still inherit their input. This is one call-level product correction, not a redesign of foreground ownership.
The tests exercise the actual capture function, native cmd.exe, file and nonseekable pipe handles. Native STD_INPUT_HANDLE changes are scoped and restored; no environment dump is printed. The reader control must actually consume unprotected input. Installed-vcvars raw consumption is recorded, not assumed universal or used to identify a particular historical vendor subcommand.
Actual Windows execution — all14triggered workflows completed
- Native Debug #742/34468575640: actual Debug build, all four paired shard jobs, runtime/ambient/include/parameter, transitive/default-library/include/has-include freshness and side-output checks, final gate passed.
- Native Release #626/34468575666: Stage0, shared library, all four Release shard jobs, full self-host and exact runtime-only package validation passed. Self-host/package job102845976343 completed every step. publish-release job102848999277 was skipped.
- Default Endpoint #29/34468575587 and **Private Ownership #31/34468575611:**70/70 and42/42complete, all original required positive controls pass.
- Typed MSVC Batch #7/34468575617 and **Real Target Admission #5/34468575600:**each original fixed six-case native study passed and was replayed from its own raw current-head artifacts.
- **Performance #608/34468575612:**actual exact base/head checkouts, both Release builds, instrumentation verification, original19x4ABBA and upload completed.
- Documentation#187,Cross-Stage#217 and five ancillary incremental/PCH/module/reporting workflows completed successfully. Ancillary success statuses are not described as complete independent raw-artifact audits.
Untriggered File Event/PDB Open/fixed historical investigation workflows are not labeled new passes. No post-submission source revision, same-head CI/native/benchmark retry or extra measurement was requested.
Actual stdin/foreground evidence, not only green statuses
Detailed Debug/Release native execution inspection
Inspected compile/link/run/PASS in Debug job102845095770 and Release job102845117586, subshards6/8 and7/8. In each configuration all seven records pass: raw/protected deliberate readers on file and pipe, protected failure17 with unchanged input, and raw/protected installed vcvars. Both actual installed-vcvars unprotected observations consumed all29bytes; both protected calls preserved all29bytes exactly. Those two observations do not establish every installed vendor version behaves identically.
Both cold run and warm source-first --run return the original foreground child37 with expected stdout/stderr; cold actually compiles/links, warm does neither. The child checks empty/spaced/quoted/trailing-backslash/Unicode arguments, cwd, inherited environment and stdin. Original compile failure4/C2338 and link failure5/LNK2019/LNK1120 do not launch the previous artifact. These failure exits remain failures, not successful child runs. The child explicitly uses Debug/MTd in both Debug and Release wrapper suites, avoiding the earlier fixture CRT mismatch without changing product linker policy.
Debug image20260907.229.1 and Release image20260824.214.3 differ; no identical historical-host claim. Existing C4996/D9025 warnings are retained, not a warning-free/WX claim. Delivery keeps selected exact log excerpts, not full native job archives. Protected file/pipe tests verify raw bytes using the source-defined assertions; excerpt replay is not a new native execution.
Existing regression evidence independently checked
Default70 retains all14scheduler and14direct-drain positive controls. All14reference pairs match56source files and70argv records after case-directory normalization. Original seven-helper and refusal outputs plus actual two prebuilt executable hashes/provenance were inspected. Ownership contracts 150/150 match113accepted/37rejected and exact Windows CRLF collector hash. Target contracts 32/32 match7accepted/25rejected.
Default preserves two original managed-policy B failures: Zi Debug A-started normal and Zi Release A-started cancel, both C1090/B0+B1 exit2. Private preserves one, PCH Debug A-started cancel, B1 exit2/C1090. Their failed link/run are unattempted, separate recovery0 does not replace original failure. Across both matrices all28A-started managed endings still kill the original service. Audited719default/415private original results and1438/830diagnostic files. Resource identity appears only6/70 and1/42; it is not a no-writer certificate.920/537generated-binary inventory entries are post-case hashes, not failure-time snapshots.
The true-MSVC batch study preserves its two injected C2338 originals as failed despite simultaneous admission cancellation and does not enter successors.18original results/36diagnostics and actual input bytes checked. The true-target study replays48results/96diagnostics,96raw cache copies and120snapshot records: stopped target EXE/link-cache remains unchanged while actual freshness reports unfinished work dirty; valid sibling TU caches may remain. Actual current-head input executables match provenance. These inputs are not the independent ABBA binary pair. No CLI cancellation, transaction rollback or writer lease is inferred.
Independent ABBA and historical risks kept distinct
Full19-row current comparison, all samples and historical replay
Current artifact10148871468 SHA256 fdd462ded9e6308f43785cca6a331a083bfc07c176d28dc0bb706f8fa3e6915b was checked against GitHub metadata/CRC and all76pairs independently recomputed.72complete instrumented counter/breakdown/compile-link-archive hit/miss identities agree;four external counters unavailable,not zero. Only the external row uses timings-off elapsed;four-pairP95 is a maximum,not a population-tail estimate.
External no-op paired median −1.39135ms/−9.6286596% does not cross the original BOTH>1ms AND>10% gate. Preserve cold median+124.296ms,build-run+1.314ms,link-only+22.195ms,scale-cold+819.012ms andsingle-TU+539.134ms adverse tails and all other unfavorable/favorable samples. No source-causal attribution, zero-overhead/speedup claim, presumed-noise deletion, AA subtraction or historical-debt closure.
The two unmodified historical #180 ABBA archives were replayed separately: #599 still crosses,+2.07175ms/+16.2700359633%;#601 remains+0.39145ms/+3.3932964684%. This stdin repair is not a performance repair and does not authorize the held build/run refactor. #181's later fixed evidence did not recreate the missing historical binary/OS trace. Current comparison JSON likewise is not an archived ABBA binary pair or OS trace.
Decision and next handoff
Accept ordinary expected-head merge of 0f20c10feca7060ac9c94ea7c720d17595eb441a, subject to unchanged main/protection. The isolated current correctness and performance gates pass with no waiver. Keep VERSION5.5.0, #180/#172 Draft/HOLD and historical performance/C1041 debt unresolved. No M1b completion, CLI cancellation, project transaction/lease or formal release authority is granted.
Next return to the remaining foreground build/run separation on the then-current main: subtract the now-independent stdin fix/tests from #180's net change, preserve its original failures and #599, review the reduced structural diff and declare its risk/acceptance scope before any new exact-base/head validation. Do not present this merge as #180 approval, reuse old-base greens, add another observer, or infer safe write-lock release from a child exit. Final VERSION/release remains separate.
Reproducibility
Delivery retains 10current core ZIPs plus2historical ABBA ZIPs, exact base/head source archives, patch, SHA256 manifest and an actually completed portable offline replay. Default10149157078 SHA256 4f81312e238946fe9123dc867640a57ba14d1bc1463013982e78ddceb20a244a;private10148974399 599739fdd2168ea88267f26dcfa8a7cf320c70fa0033f9e342953b99eeacf027. This is not every artifact or full log from all14workflows. Actions retention30days; generated compiler/target bodies are excluded by the original diagnostic contracts, separate probe inputs are retained.
Local repository network resolution failed; exact source was reconstructed from existing connector-provided archives and verified by Git tree/blob hashes, including preserved binary install.bat bytes. Source reconstruction/offline Python replay is not local MSVC/PowerShell execution. The three remote blobs were reused directly, not retranscribed; there was no corrected published head. A filtered artifact-name lookup initially returned empty and unfiltered listing located it; no job was rerun. Later title/ready-triggered skipped perf jobs cannot replace actual#608 and post-merge push CI is not claimed already completed.
Iviesever
changed the title
…din fix Preserve the complete #180 history and original failures. Bring in accepted #181 diagnostic entry points and #182's identical stdin/CLI test file versions without rewriting history. Relative to current main, retain only the nine-file application-private build completion/foreground execution boundary and its completion/module tests. No new cancellation, lease, observer or VERSION change. This combined source needs its own full native/self-host/package and unchanged independent ABBA; historical #599 remains an unresolved risk, not a repaired measurement.
1 participant
Accepted isolated correctness repair — #180 remains held
Final exact-source review, native execution, artifact replay and acceptance decision
Accept ordinary expected-head merge of this three-file stdin fix. It does not authorize #180's build/run refactor, explain historical #599, complete M1b or grant transaction/lease/release authority.
Exact source
02f8fc9e03db789ebfea7a2462b1208a1be3ee45.0f20c10feca7060ac9c94ea7c720d17595eb441a.6ef8457242fa92bd076eefd396f23ac02602f540.1eb1a707d62fc97a098e1f8f9b50dd041ad92444, exact base/head parents.13a855f51f8e23fc3b0ba0d0db25e8305d0ceb6b; VS testsec38657c33487c95bcdaa2e0b9dd24d7c1fde11f; CLI tests3391f5e56072f565fe4fbca30eb695ba5e159738.Fix
The noninteractive environment call reads NUL:
call "%MQB_VCVARS%" %MQB_VC_TARGET% <nul >nul 2>&1The following errorlevel propagation and environment decoding stay unchanged. No input copying/rewinding, generic runner stdin change, compiler/link policy change, cancellation token or Job ownership change. Input remains available to the eventual foreground consumer.
Current-head validation
Native stdin and foreground inspection
Seven actual stdin records pass in BOTH Debug and Release: raw/protected deliberate reader on file/pipe, protected bootstrap error17, raw/protected installed vcvars. In both actual vendor-script observations unprotected input goes29to0bytes while protected input remains29bytes exactly. Raw installed behavior is observed, not assumed invariant across all vendor versions. Environment dumps are not printed.
Cold and warm foreground calls each return the original child37 with verified stdin/cwd/env/empty-spaced-quoted-trailing-backslash-Unicode argv and stdout/stderr. Cold compiles/links;warm does neither. Injected compile4/C2338 and link5/LNK2019/LNK1120 never launch the old artifact. The child uses Debug/MTd in both wrapper configurations. Native images differ, Debug20260907.229.1/Release20260824.214.3;C4996/D9025 warnings remain. Selected native log excerpts are retained, not described as full job logs.
Original default/private matrices retain two/one adverse managed-policy B first-build failures(C1090), separate recoveries and all28A-started managed-service deaths. No unsafe policy is authorized. Audited719/415results and1438/830diagnostics; sparse owner snapshots do not prove writer quiescence. The original batch failures and target-cache dirty-state checks remain intact. No post-submission source correction or same-head retry; untriggered observer research is not labeled current rerun evidence.
Independent performance and unchanged historical limits
Full19-scenario paired table and historical replay
Current external no-op paired median −1.39135ms/−9.6286596% does not cross the original BOTH>1ms AND>10% gate. Four external counter sets remain unavailable, not zero;72instrumented complete counters/breakdowns/hit-miss agree. Four-pairP95 is a maximum, not a population-tail estimate.
Keep cold+124.296ms/build-run+1.314ms/link-only+22.195ms medians and scale-cold+819.012ms/single-TU+539.134ms tails, with all favorable/adverse samples. No speedup/zero-overhead claim, noise deletion or AA subtraction. Original #599 still measures +2.07175ms/+16.2700359633%, gate crossed when replayed from its unmodified archive. Neither #601 nor this lower median resolves that incident. Current comparison JSON does not archive an actual performance binary pair or OS trace.
Decision and next handoff
The original predeclared exact-source native/selfhost/package, seven input/four foreground controls in both configurations, applicable original positive regressions and independent19x4ABBA gates all pass; none was weakened or waived. Accept this isolated fix; keep #180/#172 held and historic C1041/performance debts open.
Next review the remaining #180 foreground build/run structural diff on the new main, removing these now-independent stdin files from its net scope, preserving original failures/#599 and declaring any risk/acceptance policy before fresh exact-base/head validation. This is not preauthorization of that PR. No new observer expansion, CLI cancellation, project leases/transactions or final release is included here.
Delivery includes10current core ZIPs+2historical ABBA ZIPs, exact source archives, patch, SHA256 manifest and completed portable offline replay. Not all workflow artifacts/full logs. Actions retention30days; diagnostic binary-body exclusions and finite snapshot limits remain explicit. Local source reconstruction/offline replay is not localMSVC/PowerShell execution. Temporaryperf title was removed only after actual#608finished; any title/ready-triggered skipped perf and post-merge push CI do not replace the completed evidence.