Skip to content

compile: probe the artifact from a short path when --out is deep - #776

Open
colinhacks wants to merge 3 commits into
mainfrom
fix-win-longpath-probe
Open

compile: probe the artifact from a short path when --out is deep#776
colinhacks wants to merge 3 commits into
mainfrom
fix-win-longpath-probe

Conversation

@colinhacks

@colinhacks colinhacks commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

nub compile --out <deep path> builds a working binary then fails verifying it: the self-probe cannot spawn the staged artifact.

Measured on Windows Server 2022 with the gate's own binary — --out at 285 chars compiles clean, at 351 the staged image fails with os error 3. It reproduces both where LongPathsEnabled is 0 (a default Windows box) and 1 (GitHub runners), so long-path support does not decide it. \\?\ makes it worse: measured from Rust it turns a working spawn into os error 123.

The probe asks whether the produced bytes run, not where they sit, so it retries from a short copy. The staged artifact does not move — its directory shares a filesystem with the destination so publish stays an atomic rename. An ordinary --out copies nothing.

Evidence: branch probe-win-longpath-spawn.

Copilot AI lite review requested due to automatic review settings August 20, 2026 01:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@vercel

vercel Bot commented Aug 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nub Ready Ready Preview Aug 24, 2026 4:54pm

Request Review

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No defect found in the code as written — two verification questions worth settling before the next tag.

Reviewed changes — the whole diff (one file, one commit) plus the surrounding compile pipeline and the release gate it targets.

  • Long-path retry in the self-probeverify_artifact now matches on the spawn result and, on Windows only, retries from a short temp copy when the failure looks like a path-length rejection. The staged artifact itself stays put, preserving the atomic publish rename.
  • probe_once extracted — the spawn becomes a helper so the first attempt and the retry share one invocation.
  • is_path_too_long_to_spawn — Windows-only predicate matching raw OS codes 3 and 206, deliberately on raw_os_error() rather than ErrorKind because 206 has no ErrorKind mapping.
  • ShortProbeCopy — RAII copy to %TEMP%\nub-compile-probe-<pid>-<seq>.exe, removed on drop through the existing FileGuard.
  • Predicate unit test — Windows-gated; asserts 3/206 divert and 2/5 do not.

Two things I checked and found sound, so they need no follow-up: probe mode (crates/nub-launcher/src/main.rs:911-928) reads its own section via libsui::find_section and never resolves a cache, so a copy is a valid probe subject; and no other step in the compile flow breaks on a deep --out — staging, injection, sync_file and fs::read go through std fs, and publish already uses windows_verbatim_path + MoveFileExW.

ℹ️ The gate this unblocks spawns the same over-long artifact one step later

.github/workflows/release.yml:1290-1293 runs "$LONG_OUT" (~320 chars) directly from the bash step and asserts its stdout is windows-long-second. Git Bash spawns through CreateProcessW as well, so if "Windows cannot CreateProcess an image past MAX_PATH, and no prefix lifts it" is universally true, that step fails one line after the one this PR fixes and the gate stays red. If bash can run it, the premise is narrower than the comment states. Exactly one is true, and the gate only runs on a v* tag — so the answer currently arrives after the publish is already in motion.

Technical details
# Windows long-path gate: the artifact execution step

## Affected sites
- `.github/workflows/release.yml:1279``LONG_OUT` is built to exceed 260 chars (asserted at :1280).
- `.github/workflows/release.yml:1290-1293``WINDOWS_REPLACED_ACTUAL="$( cd "$FOREIGN_CWD"; ... "$LONG_OUT" )"` executes that path. `$FOREIGN_CWD` is short, so this is the image path, not the cwd.

## Required outcome
- Know, before the next `v*` tag, whether the gate can reach `✓ Windows long-path compile replaced an existing executable` with this PR applied — rather than discovering it during a release.

## Suggested approach (optional)
- The `ci-adhoc-test` skill runs a branch-scoped Windows probe with no PR: compile to a >260-char `--out` on `windows-latest` and then try to execute it from the same bash step, printing the exit status either way.
- If bash cannot spawn it either, the gate needs the execution step to go through a short path too (a copy, a `subst` drive, or an 8.3 name) — the same shape as the fix in this diff.

## Open questions for the human
- Was the gate's execution step ever observed passing on a >260-char path, or has it only ever been reached in theory (the PR body says both Windows legs failed at the compile step)?

ℹ️ Nitpicks

  • ShortProbeCopy::new (new line 1601): SEQUENCE is always 0 — verify_artifact has a single call site and runs once per process, and the pid already separates concurrent compiles. StagedArtifact::new needs its counter for the 128-attempt collision loop; there is nothing here for this one to disambiguate.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread crates/nub-cli/src/compile/mod.rs
@colinhacks
colinhacks marked this pull request as draft August 20, 2026 04:41
@colinhacks

Copy link
Copy Markdown
Contributor Author

Closing: the premise is disproven. Windows CAN CreateProcess past MAX_PATH here — a Rust binary spawns a 297-char image fine, LongPathsEnabled=1, and the \?\ prefix breaks a working spawn (os error 123). The real failure reproduces on a fresh compile, but path length, the .exe.tmp name, the staging dir and the artifact are each individually exonerated. Evidence on branch probe-win-longpath-spawn.

@colinhacks colinhacks closed this Aug 20, 2026
@colinhacks
colinhacks deleted the fix-win-longpath-probe branch August 20, 2026 09:30
@colinhacks colinhacks reopened this Aug 20, 2026
@colinhacks
colinhacks marked this pull request as ready for review August 20, 2026 13:00

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ The new commit is comment-only and its rewritten claims hold up. One inline nit, and the gate question from the previous review is now sharper rather than settled.

Reviewed changes — both commits against main, plus the surrounding compile staging/publish path, the launcher's probe mode, and the release gate this targets.

  • Long-path retry in the self-probeverify_artifact matches on the spawn result and, on Windows only, retries from a short %TEMP% copy when the failure looks like a path-length rejection. The staged artifact stays put, preserving the atomic publish rename.
  • probe_once extracted — the first attempt and the retry share one invocation.
  • is_path_too_long_to_spawn — Windows-only predicate on raw OS codes 3 and 206, deliberately not on ErrorKind.
  • ShortProbeCopy — RAII copy to %TEMP%\nub-compile-probe-<pid>-<seq>.exe, removed on drop via the existing FileGuard.
  • f28f3d1 (new since the last review) is comment-only — no executable line changes. It replaces the retry's rationale and two doc comments with measured numbers. The replacement is a genuine improvement: the old text asserted flatly that "NO prefix lifts it", and the new text scopes that to "measured from Rust", which is the correct scope (see below).

Re-verified and needing no follow-up: the launcher's probe mode (crates/nub-launcher/src/main.rs:911-928) reads its own section via libsui::find_section and has no dependence on its image path, directory, or any sidecar, so a copy is a valid probe subject; and FileGuard removes the temp copy on every path including the ? early-returns inside the match arm.

ℹ️ The gate this unblocks executes a path ~32 characters longer, one step later

.github/workflows/release.yml:1290-1293 runs "$LONG_OUT" directly from bash and asserts its stdout. Working the lengths out from staging_dirs() — which prefers output_dir.parent(), since same_filesystem is hardcoded true on Windows — the staged path this PR fixes drops one --out segment and gains .nub-compile-artifact-<pid>-<seq>/<name>.tmp, landing about 32 characters shorter than $LONG_OUT itself. So the artifact the gate executes is strictly more exposed than the one the retry rescues, and this PR does not touch that step.

Worth noting the arithmetic reproduces the PR body's measurements exactly under a plain 260 ceiling on the staged path: --out 285 → staged ~253 (clean), --out 351 → staged ~319 (fails). The gate's staged path computes to ~289.

Research this run did not settle whether bash can spawn it, and the evidence genuinely cuts both ways — details below. Since the gate only fires on a v* tag, the answer currently arrives mid-release either way.

Technical details
# Windows long-path gate: the artifact execution step

## Affected sites
- `.github/workflows/release.yml:1276-1281``LONG_SEGMENT` is 64 chars, nested 4 deep
  under `WORKDIR` (`$RUNNER_TEMP/nub-release-compile-<platform>`, ~40 chars), so
  `$LONG_OUT` is ~321 chars.
- `crates/nub-cli/src/compile/mod.rs:1817-1829``staging_dirs()` returns
  `output_dir.parent()` first on Windows, dropping one 65-char segment; the container
  and `.tmp` suffix add back ~54. Staged path ~289.
- `.github/workflows/release.yml:1290-1293``WINDOWS_REPLACED_ACTUAL="$( cd "$FOREIGN_CWD"; ... "$LONG_OUT" )"` executes the ~321-char
  path. `$FOREIGN_CWD` is short, so this is the image path, not the cwd.

## What the research found (unresolved, evidence split)
- FOR bash succeeding: MSYS2/Cygwin's `winsup/cygwin/spawn.cc` (`child_info_spawn::worker`)
  passes a `\\?\`-prefixed path as `lpApplicationName` and deliberately KEEPS the prefix
  at `>= MAX_PATH + 4`, only erroring past `NT_MAX_PATH`. Microsoft's `CreateProcessW` docs
  scope the MAX_PATH restriction on the module name to the case where `lpApplicationName`
  is NULL. This is a real mechanism difference from Rust's `Command`, and it means the
  comment's `\\?\` finding being scoped to "measured from Rust" is the correct scope.
- AGAINST: `CreateProcessW` is absent from both function lists on
  https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation, and
  a Microsoft engineer states it does not honor `LongPathsEnabled`
  (https://learn.microsoft.com/en-us/answers/questions/924906).
- No primary source confirms the Cygwin path actually succeeds for an image path this long
  in practice. Only an execution settles it.

## Required outcome
- Know, before the next `v*` tag, whether the gate reaches
  `✓ Windows long-path compile replaced an existing executable` with this PR applied.

## Suggested approach (optional)
- The `ci-adhoc-test` skill runs a branch-scoped Windows probe with no PR: compile to a
  >260-char `--out` on `windows-latest`, then execute the published artifact from the same
  bash step and print the exit status either way. That one run resolves both halves.
- If bash cannot spawn it, the gate's execution step needs a short path too — a copy, a
  `subst` drive, or an 8.3 name — the same shape as the fix in this diff.

## Open questions for the human
- Because the staged path is ~32 chars shorter than `--out`, there is a band of `--out`
  lengths where compile succeeds with no retry at all and the published artifact still
  cannot be spawned from its destination. `nub compile` reports success in that band. Is
  warning on an unspawnable destination in scope for this PR, a follow-up, or deliberately
  the user's problem? The comment at mod.rs:1542-1543 scopes the probe to the BYTES, which
  reads like a deliberate answer — worth stating it as one.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread crates/nub-cli/src/compile/mod.rs Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found. 13d0743 is a minimal, correct fix for the one open inline finding.

Reviewed changes13d0743, the only commit since the previous pullfrog review, plus the surrounding retry arm and the error chain it produces.

  • The triggering spawn error is now carried into both failure pathsShortProbeCopy::new gains a with_context naming the in-place failure ({bin} did not spawn in place: {error}), and the short-copy probe_once context gains the same clause. The raw OS code now survives whether the copy fails or the copy also refuses to spawn, which is what keeps a non-length os 3 (a genuinely missing image) naming itself rather than surfacing as an unexplained copy error.
  • Six executable lines, no behavior change on the success path — the predicate, probe_once, ShortProbeCopy and the non-Windows arm are untouched; only the two context strings inside the #[cfg(windows)] arm changed.

Checked and sound, so no follow-up: the arm still borrow-checks — with a match guard, error binds by reference during the guard and by value in the body, and each format! closure takes it by shared reference and is consumed inside its own with_context, so the borrows never overlap. anyhow::Context does apply to ShortProbeCopy::new's anyhow::Result. rustfmt --edition 2024 --check on the file exits 0 at this head, including the 105-character string-literal line at crates/nub-cli/src/compile/mod.rs:1557 — rustfmt does not split string literals, so cargo fmt --check will not trip on it.

Worth knowing rather than acting on: the whole arm is #[cfg(windows)], so nothing but the Windows CI leg compiles these lines — a green Linux or macOS run is not evidence about them.

Both prior pullfrog threads are now retired. The predicate's code set is settled by f28f3d1 recording os 3 as measured; 206 remains an inert extra arm rather than a defect. The two open questions from the previous review — whether the gate's own execution step at .github/workflows/release.yml:1290-1293 can spawn $LONG_OUT (~32 characters longer than the path this PR rescues), and whether an unspawnable --out destination deserves a warning — are unchanged by this commit and still worth an answer before the next v* tag. They are argued in full in the previous review; I am not restating them here.

Pullfrog  | View workflow run | Using Claude Opus𝕏

`nub compile --out <260+ char path>` produced a working binary and then failed
verifying it, because the self-probe spawns the staged artifact and Windows
cannot CreateProcess an image past MAX_PATH. Nothing lifts that: the `\\?\`
prefix fails with ERROR_INSUFFICIENT_BUFFER, a long lpCurrentDirectory fails,
and LongPathsEnabled does not cover process creation. It is the one long-path
site windows_verbatim_path cannot rescue the way it does the publish rename.

Every file API still reaches the artifact, and the probe asks whether the
produced bytes run — a property of the file, not of its directory — so it now
retries from a short copy in the temp dir, removed on drop. The staged artifact
does not move: its directory is chosen to share a filesystem with the
destination so publishing stays an atomic rename. An ordinary --out spawns in
place and copies nothing.

The retry is keyed on the raw OS codes rather than ErrorKind, because 206 has no
kind an arm can name — the trap aube-linker's is_transient_fs_error documents
for os 32.

Surfaced by the release pre-publish gate, whose Windows legs build a >260
character output path on purpose.
The code is unchanged; only its explanation was wrong, and I closed the PR over
it. The claim that Windows cannot CreateProcess past MAX_PATH does not hold: a
Rust binary spawns a 297-character image fine on GitHub's runners, where
LongPathsEnabled is 1.

What actually holds, measured on Windows Server 2022 with this crate's own
artifact: --out at 285 characters compiles clean and at 351 the staged image
fails to start with os error 3. The failure reproduces both where
LongPathsEnabled is 0 (a default Windows box) and where it is 1, so long-path
support does not decide it. The exact threshold is unpinned, which is why the
retry keys on the error rather than on a length.

Also drops two claims that were never verified: that 206 has no ErrorKind
mapping, and that the artifact is ~100 MB (a measured embed shape is ~25 MB).
os 3 is ERROR_PATH_NOT_FOUND, which is not exclusively a length signal, so the
code that triggered the retry has to survive into whatever fails next. It did
not: a full disk or unwritable TEMP surfaced only as 'copying ... so the
self-probe can spawn it', and a short copy that also refused to start surfaced
only as 'running the self-probe on a short copy of ...'. In both cases a
genuinely missing image was reported as an unexplained copy problem.

Both failures now chain the original error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants