[quaesitor-scientiam]

Summary

Two issues found while diagnosing an -msvc build failure caused by
the vlang/vc snapshot's stale small/char bug (see #29025,
#29098).

1. -msvc's bootstrap stage never tried MSVC at all. Building
v_win_bootstrap.exe from vc/v_win.c (before compiling the real
v.exe) went straight to Clang, then GCC, then the bundled TCC - even
though MSVC is what -msvc explicitly asked for and is already
confirmed present at that point (vsdevcmd.bat has already run).
Reordered to TCC → MSVC → Clang → GCC: TCC first since it's
fastest and needs no external toolchain, then MSVC (the explicit
request), then the remaining fallbacks. Each is still tried in turn,
since any one of them can fail to compile a given vc/v_win.c
snapshot on its own - two concrete examples hit this session: TCC
needs vlang/tccbin#96's intrin.h/dbghelp.h headers; MSVC and TCC
hit the small/char macro collision differently depending on
whether their own windows.h chain happens to pull in rpcndr.h.

2. A noisy, benign cleanup failure. Every "%V_STAGE%" cleanup
(if exist "%V_STAGE%" del "%V_STAGE%", 5 call sites) could
intermittently print a native, unhandled Access is denied. -
Windows (or a real-time antivirus scanner, which the script's own
success message already warns about) can briefly hold a lock on an
executable right after the process running it exits. The delete's own
exit code was never checked (build success/failure is determined by
the compile step's saved errorlevel beforehand), so this was always
harmless, just confusing. Replaced all 5 sites with a new
:try_delete helper that retries twice with short waits (matching the
existing ping-based sleep trick already used in :move_updated_to_v)
before giving up silently. In the common case (no lock), this adds no
delay - the retries only run when the file was actually still locked,
which is exactly the case that used to fail visibly anyway.

Test plan

• Verified end-to-end on Windows across several rebuilds:
  ./makev.bat -msvc --local now bootstraps via TCC first
  (falling back to MSVC/Clang/GCC as needed depending on which
  toolchain-specific fixes are locally present), and the final
  v.exe is genuinely MSVC-built (-cc msvc).
• Confirmed the previously-intermittent Access is denied.
  message no longer appears across multiple consecutive runs.
• Confirmed the retry helper adds no measurable delay in the
  normal (unlocked) case by inspection of its early-exit branches.

🧙 Built with WOZCODE

[medvednikov]

Reviewed 4420fc186de40cb4af04eefedd496c6cfd378e1d; CI status/results were not considered.

One bootstrap correctness concern is noted inline: the new MSVC compilation command omits the compatibility-compiler define that all three existing C-bootstrap commands pass. Please preserve that bootstrap contract in the new branch.

I also traced the five cleanup replacements: callers either preserve the compilation result in stage_error/msvc_error or are already taking the explicit failure branch, so the best-effort helper's successful return does not replace those compilation decisions.

Validation: source review of the diff, bootstrap selection/stage flow, cleanup call sites, and the shared bootstrap-define setup. No Windows build was run here; the missing-define finding is based on the script's documented portable-snapshot requirement and the other bootstrap commands, not a claimed runtime reproduction.

[quaesitor-scientiam]

Ran an adversarial review pass on this PR (4 lenses, 3 adversarial refuters per candidate). 3 of 7 candidates survived:

1. Sibling-parity gap (fixed in 9683eda): try_delete was only applied to the 5 %V_STAGE% cleanup sites; the sibling %ObjFile%/%V_STAGE_C% cleanups (5 more sites) were left as plain if exist ... del, which can hit the same transient-lock "Access is denied" noise under different filenames. Converted all remaining sites.

2. Zero automated test coverage for the MSVC bootstrap define (fixed in 9683eda): cmd/tools/makev_test.py's test_all_portable_bootstraps_select_v1 never checked msvc (added as a separate test, since cl.exe is invoked directly and doesn't fit the existing loop's line-matching pattern for the other three compilers). Also added real execution tests for try_delete itself (previously zero coverage) - including a genuinely-verified locked-file case, not just an assumption: confirmed Python's default open() on Windows actually blocks deletion, so the test exercises the real retry-then-give-up-cleanly path, not a no-op.

3. windows_ci_msvc.yml never exercises the new MSVC-bootstrap branch in CI - the real Windows CI job runs .\makev.bat -msvc, but TCC succeeds first every time on that runner, so build_bootstrap_with_msvc (the actual subject of this PR) is never compiled through in CI, only in this session's manual testing (forcing TCC/Clang unavailable by hand). This is a CI workflow change, which I'm leaving for an explicit ask rather than bundling in - happy to take it on if wanted.

Verified: python cmd/tools/makev_test.py -v passes all 15 tests (12 existing + 3 new), and .\makev.bat -msvc --local still builds a working v.exe with everything combined.

[quaesitor-scientiam]

Ran another adversarial review pass on 9683eda. 1 finding survived - and unusually well-verified: one of the adversarial refuters didn't just reason about it, they deliberately reintroduced the exact regression (reading !ERRORLEVEL! after a call :try_delete instead of the variable saved before it), ran the full python cmd/tools/makev_test.py -v suite on real Windows, and confirmed all 15 tests still passed - empirically proving the gap rather than just asserting it.

The gap: no test exercises msvc_strap/build_bootstrap_with_msvc/build_stage_with_clang's actual control flow through cmd.exe with a real or stubbed failing compiler. try_delete is only unit-tested in isolation, and test_msvc_bootstrap_selects_v1 is a static regex check on source text, not an execution. All three refuters confirmed this is not a present bug - the current code is correct at every site - but a future edit violating try_delete's "capture errorlevel into a variable before calling this" contract would silently swallow a real compile failure and the test suite would stay green.

Per direction, documenting this as a known limitation rather than building a harder execution-level test right now (these routines need real MSVC/Clang/TCC, which the existing test harness's V-command-stubbing approach can't reach without a larger change). Added the caller contract and this gap directly as a comment at try_delete's definition in 86cf97b, so a future editor sees it before making exactly this mistake.

[medvednikov]

Re-reviewed the fixups since 4420fc186de40cb4af04eefedd496c6cfd378e1d, at head 86cf97ba7640f84541e45bd90f2b52b4162698b2. CI status/results were not considered.

The previous P2 about the missing portable-bootstrap define is addressed. No new actionable correctness issue found in these fixups.

build_bootstrap_with_msvc now passes %VC_BOOTSTRAP_DEFINE% to cl before %V_C_FILE%, matching the other bootstrap paths. Using the shared -D spelling is valid: Microsoft's compiler command-line documentation explicitly permits either / or - for options. The new test_msvc_bootstrap_selects_v1 specifically checks the direct cl invocation, which the existing TCC/Clang/GCC loop did not cover.

I also checked the expanded cleanup changes. build_bootstrap_with_msvc preserves msvc_bootstrap_error, build_stage_with_clang preserves stage_error, and the final MSVC build preserves msvc_error before calling try_delete. The other changed cleanup sites are already on explicit failure branches. Thus the helper's unconditional success does not mask the compilation decisions in the current code. The new deletion tests cover an unlocked file, a missing file, and a persistently locked file.

Validation limits: source-level re-review of the updated script, its callers, and the complete Python test file; I did not execute the Python suite or Windows bootstrap/locked-file tests. This environment has neither a V compiler nor a Windows command interpreter, and direct checkout downloads fail DNS resolution. The source-contract test is not an end-to-end MSVC-bootstrap test; I have not independently reproduced the author's reported Windows run. The original finding is resolved as a bootstrap-contract inconsistency, not as a newly claimed reproduction of the old failure.

Reference: https://learn.microsoft.com/en-us/cpp/build/reference/compiler-command-line-syntax?view=msvc-170

[quaesitor-scientiam]

Closing the loop on @medvednikov's disclosed validation limit from the last review ("I have not independently reproduced the author's reported Windows run" - they have no Windows/V-compiler access, so that review was source-level only).

Re-verified on real Windows at current head (86cf97b):

• Confirmed 86cf97b is comment-only relative to 9683eda (the last commit with actual code changes) - nothing rebuild-relevant has changed since the last verified build.
• python cmd/tools/makev_test.py -v: 15/15 pass.
• Forced the real MSVC bootstrap path end-to-end: temporarily renamed thirdparty/tcc/tcc.exe out of the way, ran .\makev.bat -msvc --local. Output: TCC not found -> Attempting to build "./v_win_bootstrap.exe" (from ./vc/v_win.c) with MSVC -> V built successfully. Restored tcc.exe afterward; git status clean.

This is a genuine reproduction of the bootstrap path the previous review could only assess from source, not a claim carried over from an earlier commit. No issues found - MSVC bootstrap works as intended when TCC is unavailable.

[medvednikov]

Previous P2 resolved. The MSVC invocation passes %VC_BOOTSTRAP_DEFINE%, the regression test checks it, and the cleanup callers preserve compiler exit codes. The posted follow-up found no new actionable correctness issue.