fix(trampoline): address review regressions

Author: fengmk2

crates/vp_trampoline/src/cmdline.rs

@@ -74,14 +74,12 @@ pub fn parse_shim_pointer(bytes: &[u8]) -> Option<ShimPointer<'_>> {
 /// Index where the raw command line's first (program) argument ends.
 ///
 /// This follows the MSVC parsing rule for the program name: a quote toggles
-/// quoted mode and backslashes have no escaping effect. The remainder
-/// (`&cmdline[result..]`, leading whitespace included) is the argument tail to
-/// forward to the child verbatim.
+/// quoted mode, backslashes have no escaping effect, and leading whitespace
+/// terminates an empty program argument. The remainder (`&cmdline[result..]`,
+/// leading whitespace included) is the argument tail to forward to the child
+/// verbatim.
 pub fn skip_program_argument(cmdline: &[u16]) -> usize {
     let mut i = 0;
-    while i < cmdline.len() && (cmdline[i] == SPACE || cmdline[i] == TAB) {
-        i += 1;
-    }
     let mut quoted = false;
     while i < cmdline.len() {
         let c = cmdline[i];
@@ -196,8 +194,15 @@ mod tests {
     }
 
     #[test]
-    fn skips_leading_whitespace_and_bare_program() {
-        let cl = wide("  node");
+    fn treats_leading_whitespace_as_an_empty_program() {
+        for cl in [wide("  script.js --flag"), wide("\tscript.js --flag")] {
+            assert_eq!(skip_program_argument(&cl), 0);
+        }
+    }
+
+    #[test]
+    fn skips_bare_program() {
+        let cl = wide("node");
         assert_eq!(skip_program_argument(&cl), cl.len());
         assert_eq!(skip_program_argument(&[]), 0);
     }

justfile

@@ -97,15 +97,11 @@ snapshot-test *args='': _install_chromium _build-trampoline
   cargo test -p vp_cli_snapshots -- {{args}}
 
 # The trampoline is excluded from the workspace; build it from its own
-# directory so its .cargo/config.toml (build-std) applies. Artifacts still
-# land in the repo-root target/ directory.
-[unix]
-_build-trampoline:
-  cd crates/vp_trampoline && cargo build
-
-[windows]
+# directory so its .cargo/config.toml (build-std) applies. The helper anchors
+# relative CARGO_TARGET_DIR values to the repo root so all binaries stay in the
+# same artifact directory.
 _build-trampoline:
-  Set-Location crates/vp_trampoline; cargo build
+  node packages/tools/src/build-trampoline.ts
 
 # Browser-mode snapshot cases run with PLAYWRIGHT_BROWSERS_PATH=0, so the
 # browser must be installed into node_modules with the same setting.

package.json

@@ -5,7 +5,7 @@
   "type": "module",
   "scripts": {
     "build": "pnpm -F rolldown build-binding:release && pnpm -F rolldown build-node && pnpm -F vite build-types && pnpm -F @voidzero-dev/* -F vite-plus build",
-    "bootstrap-cli": "pnpm build && cargo build -p vp_global_cli --release && cd crates/vp_trampoline && cargo build --release && cd ../.. && pnpm install-global-cli",
+    "bootstrap-cli": "pnpm build && cargo build -p vp_global_cli --release && node packages/tools/src/build-trampoline.ts --release && pnpm install-global-cli",
     "bootstrap-cli:ci": "pnpm install-global-cli",
     "install-global-cli": "tool install-global-cli",
     "local-registry": "node packages/tools/src/local-npm-registry.ts",

packages/tools/src/__tests__/build-trampoline.spec.ts

@@ -0,0 +1,23 @@
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+import { describe, expect, test } from 'vitest';
+
+import { resolveCargoTargetDir } from '../build-trampoline.ts';
+
+const repoRoot = fileURLToPath(new URL('../../../..', import.meta.url));
+
+describe('resolveCargoTargetDir', () => {
+  test('anchors relative paths to the repository root', () => {
+    expect(resolveCargoTargetDir('artifacts')).toBe(path.join(repoRoot, 'artifacts'));
+  });
+
+  test('uses the repository target directory by default', () => {
+    expect(resolveCargoTargetDir(undefined)).toBe(path.join(repoRoot, 'target'));
+  });
+
+  test('preserves absolute paths', () => {
+    const absolute = path.resolve(repoRoot, 'custom-artifacts');
+    expect(resolveCargoTargetDir(absolute)).toBe(absolute);
+  });
+});

packages/tools/src/build-trampoline.ts

@@ -0,0 +1,25 @@
+import { execFileSync } from 'node:child_process';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+const repoRoot = fileURLToPath(new URL('../../..', import.meta.url));
+
+export function resolveCargoTargetDir(configured: string | undefined): string {
+  return path.resolve(repoRoot, configured || 'target');
+}
+
+export function buildTrampoline(args: string[] = process.argv.slice(2)) {
+  const cargo = process.platform === 'win32' ? 'cargo.exe' : 'cargo';
+  execFileSync(cargo, ['build', ...args], {
+    cwd: path.join(repoRoot, 'crates/vp_trampoline'),
+    env: {
+      ...process.env,
+      CARGO_TARGET_DIR: resolveCargoTargetDir(process.env.CARGO_TARGET_DIR),
+    },
+    stdio: 'inherit',
+  });
+}
+
+if (import.meta.main) {
+  buildTrampoline();
+}

rfcs/trampoline-exe-for-shims.md

@@ -168,8 +168,10 @@ symbol, so neither the CRT startup nor `std` runtime init runs. The flow in
 
 1. `GetModuleFileNameW` gives the shim path and tool name. Replacing the `.exe`
    extension with `.shim` locates the per-tool sidecar.
-2. `CreateFileW` and `ReadFile` load the UTF-8 sidecar. The parser requires the
-   versioned header and accepts the `single-root` and `split` layouts.
+2. `CreateFileW` and `ReadFile` load the UTF-8 sidecar. Long paths are made
+   absolute with `GetFullPathNameW`, then use the `\\?\` drive prefix or the
+   `\\?\UNC\` network prefix. The parser requires the versioned header and
+   accepts the `single-root` and `split` layouts.
 3. `SetEnvironmentVariableW` pins the sidecar's layout. A single-root pointer
    sets `VP_HOME`. A split pointer removes `VP_HOME` and sets `VP_DATA_DIR`,
    `VP_BIN_DIR`, and `VP_CACHE_DIR`. Tool shims also set `VP_SHIM_TOOL` and
@@ -181,6 +183,7 @@ symbol, so neither the CRT startup nor `std` runtime init runs. The flow in
 5. `SetConsoleCtrlHandler` installs a handler that ignores Ctrl+C and
    Ctrl+Break; the child decides how to react.
 6. `CreateProcessW` spawns the child with inherited handles and startup info.
+   The payload path uses the same extended-length normalization as the sidecar.
    When the parent redirected stdio (`STARTF_USESTDHANDLES`), the standard
    handles are forced inheritable first, as in uv-trampoline and distlib.
 7. `WaitForSingleObject`, `GetExitCodeProcess`, and `ExitProcess` propagate the
@@ -207,7 +210,7 @@ infer the layout from directory paths.
 | `#![no_main]` + `mainCRTStartup` (no CRT startup, no `std` runtime init) | Done   |
 | Raw `CreateProcessW` instead of `std::process::Command`                  | Done   |
 
-**Binary size**: 13,312 B on x86_64-pc-windows-msvc and 13,824 B on
+**Binary size**: 14,336 B on both x86_64-pc-windows-msvc and
 aarch64-pc-windows-msvc, including sidecar parsing and error diagnostics. The
 sidecar-aware `std::process::Command` implementation was 221,696 B on x86_64.
 See Future Optimizations for the measured size ladder. The executable imports
@@ -307,17 +310,17 @@ When installing a pre-trampoline version (no `vp-shim.exe` in the package):
 | **Data embedding**  | PE resources (kind, path, script ZIP)    | Adjacent directory-layout sidecar |
 | **Dependencies**    | `windows` crate (unsafe, no CRT)         | Zero (raw FFI declaration)        |
 | **Toolchain**       | Nightly Rust (`panic="immediate-abort"`) | Nightly Rust (same technique)     |
-| **Binary size**     | 39-47 KiB                                | 13-14 KiB                         |
+| **Binary size**     | 39-47 KiB                                | 14 KiB                            |
 | **Entry point**     | `#![no_main]` + `mainCRTStartup`         | Same approach                     |
 | **Error output**    | `ufmt` (no `core::fmt`)                  | `WriteFile` + Win32 error codes   |
 | **Ctrl+C handling** | `SetConsoleCtrlHandler` → ignore         | Same approach                     |
 | **Exit code**       | `GetExitCodeProcess` → `exit()`          | Same approach                     |
 
-The Vite+ trampoline is smaller because it embeds no PE resources and needs no
-path canonicalization, job objects, or GUI subsystem support. It reads a small
-sidecar next to its own filename, resolves `vp.exe` under the recorded data
-root, and starts it. Both projects share the same build recipe and entry-point
-structure.
+The Vite+ trampoline is smaller because it embeds no PE resources and only
+normalizes long file and process paths. It needs no job objects or GUI
+subsystem support. It reads a small sidecar next to its own filename, resolves
+`vp.exe` under the recorded data root, and starts it. Both projects share the
+same build recipe and entry-point structure.
 
 ## Alternatives Considered
 
@@ -357,7 +360,7 @@ last row is the current sidecar-aware raw implementation.
 | Raw Win32 rewrite, normal `main` + build-std                                 | nightly   | 13,824 B  |
 | Raw Win32 rewrite + `#![no_main]`, no diagnostics                            | nightly   | 6,656 B   |
 | Fixed-layout raw Win32 + `#![no_main]` + full diagnostics                    | nightly   | 8,192 B   |
-| Sidecar-aware raw Win32 + `#![no_main]` + full diagnostics (shipped)         | nightly   | 13,312 B  |
+| Sidecar-aware raw Win32 + `#![no_main]` + full diagnostics (shipped)         | nightly   | 14,336 B  |
 
 For comparison: uv-trampoline ships 45,056 B (x64 console), Scoop's default
 kiennq shim is 136,192 B (statically linked MSVC C), and Scoop once vendored