fix(v8): ensure a git shim exists in depot_tools on Windows

[lif9e1]

Follow-up to #127/#130. With DEPOT_TOOLS_UPDATE=0 the Windows build never runs depot_tools' first-run bootstrap, so the bundled git.bat is never installed — and gclient's git_cache calls git.bat catching only CalledProcessError, turning the missing shim into an unhandled FileNotFoundError during the V8 sync.

This creates a git.bat shim pointing at the system git the script already requires. Since build_and_install() runs git clean -fd on depot_tools (which deletes the untracked shim), the helper is called from both build paths.

Verified on a from-scratch Windows x64 build (VS2022 Build Tools, desktop-editors CMake pipeline); details in Euro-Office/DesktopEditors#32.

🤖 Generated with Claude Code

[chrip]

Summary

The diagnosis is right and I could reproduce it from the depot_tools sources at the exact
revision this repo pins: with DEPOT_TOOLS_UPDATE=0, gclient.bat jumps straight to
:CALL_GCLIENT and never runs the bootstrap that generates git.bat, while git_cache.py
hardcodes git.bat on Windows and guards only CalledProcessError — so the missing shim
surfaces as an unhandled FileNotFoundError during the V8 sync. The fix is minimal,
Windows-guarded, and consistent with the rest of the script.

Two things to fix before merge: the commit is missing DCO sign-off (CI is red on it), and
the justification given for calling the helper from both build paths is factually wrong —
git.bat is gitignored in depot_tools, so git clean -fd does not delete it. Calling
it twice is still the right call, just for a different reason. Both are one amend away.

Verification

Claim / Item	Reality	Status
DEPOT_TOOLS_UPDATE=0 suppresses the first-run bootstrap on Windows	depot_tools gclient.bat @f394ab2c9: IF "%DEPOT_TOOLS_UPDATE%" == "0" GOTO :CALL_GCLIENT, skipping update_depot_tools.bat — which is the only thing that calls bootstrap\win_tools.bat (the generator of the *.bat wrappers)	✓
git.bat is not in a fresh depot_tools clone	.gitignore @f394ab2c9: "Ignore the batch files produced by the Windows bootstrapping." → /git.bat	✓
gclient's git_cache invokes git.bat and only catches CalledProcessError	git_cache.py:68 git_exe = "git.bat" if sys.platform.startswith("win"); git_cache.py:217-225 subprocess.check_output([cls.git_exe, "config", …]) / except subprocess.CalledProcessError → a missing executable raises FileNotFoundError, uncaught	✓
…and that path is reached by a plain gclient sync	gclient_scm.py:283-285 cache_dir property → git_cache.Mirror.GetCachePath(); the generated .gclient sets no cache_dir, so SetCachePath() is never called and GetCachePath() shells out to git.bat	✓
DEPOT_TOOLS_UPDATE=0 is set "in the depot env below"	nc-build.py build_and_install(), depot_env["DEPOT_TOOLS_UPDATE"] = "0" (added in #127/#130)	✓
"the system git this script already requires"	check_prequisites() aborts if shutil.which("git") is None, and build_all() runs it before build_and_install() — so the new git_exe is None branch is defensive only, and its message matches the existing wording	✓
git clean -fd on depot_tools removes the untracked shim (nc-build.py:371-372, and the PR body)	No. git.bat is listed in depot_tools' .gitignore, and git clean -fd skips ignored files (-x would be needed; the depot_tools clean, unlike the v8 one, is -fd). The shim survives both git reset --hard and git clean -fd.	❌
Calling the helper from both build paths is nevertheless needed	Yes, but because of work-dir reuse: build_all() skips fetch_and_patch() when work_dir_looks_ok(), so an existing pre-#132 work dir would otherwise never get a shim	✓
"Verified on a from-scratch Windows x64 build … details in Euro-Office/DesktopEditors#32"	DesktopEditors#32 exists and does carry your Windows/Linux findings in the 2026-08-15 comments — but it is titled "DesktopEditors: macOS build". #31 is the Windows-build issue.	⚠️
#127 / #130 exist and are the referenced predecessors	#127 MERGED 2026-07-24, #130 MERGED 2026-08-04	✓
CI can validate this	No. Only DCO has reported; build-windows.yml (on: pull_request) hasn't run — fork PR awaiting maintainer approval — and even when it runs it goes through ensure_dep, which prefers a prebuilt V8 from the remote cache (creds come from secrets, unavailable to fork PRs), so it is unlikely to exercise this code path at all. Manual verification is the only realistic evidence here.	⚠️

Issues & Suggestions

🔴 Blocking

• Missing DCO sign-off — commit dd9105b has Co-Authored-By: but no Signed-off-by:;
  the DCO check is red (action_required). Please amend with your sign-off.

• The code comment states something untrue (Common/3dParty/v8/nc-build.py:371-372):
  "build_and_install() runs git clean -fd on depot_tools, which removes the untracked
  shim". git.bat is in depot_tools' .gitignore, so git clean -fd leaves it in place —
  only git clean -fdx (used for v8, not for depot_tools) would remove it. The double call
  site is still correct, so this is a comment fix, not a code fix, but it's exactly the kind
  of comment someone will later act on ("clean doesn't remove it → drop the second call").
  Suggested wording:

  # Called from both build paths on purpose: build_all() skips fetch_and_patch()
  # when the work dir is already ok, so a work dir created before this fix would
  # otherwise never get a shim. (git.bat is gitignored in depot_tools, so it
  # survives the `git reset --hard` / `git clean -fd` there.)
  

  Please fix the same claim in the PR/commit body while amending for DCO.

ℹ️ Minor

• write_text doubles the CRs on Windows (nc-build.py:381). The literal already
  contains \r\n, and Path.write_text() opens in text mode with newline=None, which
  translates \n → os.linesep, so the file lands as
  b'@echo off\r\r\n"…git.exe" %*\r\r\n'. cmd.exe evidently tolerates it (your build
  passed), but it is accidental. Simplest fix: pass newline="" so the literal is written
  verbatim (build_3rdparty_common.py already uses Path | None annotations, so ≥3.10 is
  assumed and the parameter is available), or drop the explicit \rs and let text mode
  produce native CRLF.
• A stale shim is never refreshed (nc-build.py:376-377). Because the shim genuinely
  survives the depot_tools clean (see above), an existing work dir keeps whatever absolute
  git path was resolved on the first build. If git is later upgraded/moved (or the first
  build ran with a different git on PATH), every subsequent sync fails with cmd.exe's
  'C:\old\path\git.exe' is not recognized, which is a poor diagnostic. Writing it
  unconditionally is cheaper than the check and removes that failure mode; the only thing
  the exists() guard buys you is not clobbering a real bootstrapped git.bat, which
  DEPOT_TOOLS_UPDATE=0 guarantees will never be there. Your call, but I'd overwrite.
• Style — the file consistently uses one blank line between top-level defs and spaced
  call parens (nc.abort_op( … ), f"Tool not found: {tool}"); the new code adds two blank
  lines and uses unspaced calls plus "a" + x + "b" concatenation. Matching the local
  idiom (f-string, spaced parens) would keep the diff invisible.

💡 Suggestions

• The shim can be avoided entirely: adding a top-level cache_dir = None to the
  generated .gclient makes gclient.py:2108-2114 call Mirror.SetCachePath(None), so
  GetCachePath() short-circuits and never shells out to git.bat — and with no cache dir,
  no Mirror instances are created, which are the only other users of git_exe. That's one
  config line instead of a Windows-only file-writing helper. I'm not asking you to switch:
  the shim is what depot_tools itself expects and stays correct if some other depot_tools
  code path starts wanting git.bat, whereas cache_dir = None leans on an internal
  short-circuit. Worth a sentence in the commit body that you considered it, though.
• shutil.which("git") inherits the script's existing assumption, but note this repo fights
  Cygwin git shadowing explicitly in build-windows.yml (add-to-path: false, "native
  tools > Cygwin"). On a dev box with Cygwin first on PATH the shim would hand gclient a
  Cygwin git — an unsupported combination that would fail confusingly. A one-line comment,
  or a git --version sanity check, would document the assumption.

Verdict

Request changes — the fix is correct, minimal and genuinely well-diagnosed; I verified
every load-bearing claim against depot_tools at the pinned revision. Two mechanical things
stand between it and merge: the missing DCO sign-off, and a comment (repeated in the PR body)
that misstates why the helper is called twice — git clean -fd does not remove the
gitignored git.bat. Fix both in one amend and this is an approve from me.

Assisted-by: ClaudeCode:claude-opus-5