From 0ad32ea5a2c71cbd6ea2dd788055c52ba2a8dcb2 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Tue, 14 Jul 2026 00:40:51 +0200 Subject: [PATCH] fix(fs): rewrite symlink paths to native separators on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A scoped dependency's `node_modules/@scope/name` link path is built by joining the whole `@scope/name` alias as one segment. Rust's `Path::join` appends segments verbatim (unlike Node's `path.join`, which normalizes), so the `/` inside the alias survived into an otherwise `\`-separated Windows path and reached `CreateSymbolicLinkW`, which rejects forward-slash paths — the long store paths reach it in verbatim `\\?\` form where `/` is a literal filename byte — with `ERROR_DIRECTORY` (os error 267), aborting the install. This is the same failure class as the global-virtual-store slot-path fix in https://github.com/pnpm/pnpm/pull/12976, which normalized the slot suffix at its construction sites. Rather than chase every join site, rewrite paths to native separators at the symlink writer's own choke points: `symlink_dir` (covering the hoisted linker's direct calls) and `force_symlink_dir`'s entry (so its read_link / remove_dir / rename / create_dir_all retry steps all see a native path too). The rewrite only allocates when a `/` is actually present and is a no-op on Unix. The TypeScript CLI is unaffected: Node's `path.join` already normalizes separators on Windows, so no counterpart change is needed. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...cquet-scoped-symlink-windows-separators.md | 5 ++ pnpm/crates/fs/src/symlink_dir.rs | 46 ++++++++++++++- pnpm/crates/fs/src/symlink_dir/tests.rs | 58 +++++++++++++++++++ 3 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 .changeset/pacquet-scoped-symlink-windows-separators.md diff --git a/.changeset/pacquet-scoped-symlink-windows-separators.md b/.changeset/pacquet-scoped-symlink-windows-separators.md new file mode 100644 index 0000000000..a379cf170d --- /dev/null +++ b/.changeset/pacquet-scoped-symlink-windows-separators.md @@ -0,0 +1,5 @@ +--- +"pacquet": patch +--- + +Fixed installs failing on Windows when a scoped dependency (`@scope/name`) had to be symlinked. Its `node_modules/@scope/name` link path was built by joining the whole alias as one segment, which left a `/` in the otherwise `\`-separated path; that forward slash reached `CreateSymbolicLinkW`, which rejects forward-slash paths with `ERROR_DIRECTORY` (os error 267). Paths are now rewritten to native separators before every filesystem call in the symlink writer. diff --git a/pnpm/crates/fs/src/symlink_dir.rs b/pnpm/crates/fs/src/symlink_dir.rs index bb1a059a50..b7ed6a5b24 100644 --- a/pnpm/crates/fs/src/symlink_dir.rs +++ b/pnpm/crates/fs/src/symlink_dir.rs @@ -1,4 +1,5 @@ use std::{ + borrow::Cow, fs, io, path::{Path, PathBuf}, }; @@ -26,10 +27,44 @@ pub fn symlink_dir(original: &Path, link: &Path) -> io::Result<()> { } #[cfg(windows)] { - windows::create(original, link) + let original = to_native_separators(original); + let link = to_native_separators(link); + windows::create(&original, &link) } } +/// Rewrite `path` so every directory separator is the platform-native +/// one. +/// +/// [`Path::join`] appends each segment verbatim, so an alias that is +/// itself a `/`-bearing string — a scoped package like `@scope/name`, +/// joined into `node_modules` as one segment — leaves a forward slash +/// in an otherwise `\`-separated Windows path. That slash survives into +/// `CreateSymbolicLinkW`, which rejects forward-slash paths (the long +/// store paths reach it in verbatim `\\?\` form, where `/` is a literal +/// filename byte rather than a separator) with `ERROR_DIRECTORY` +/// (os error 267). Collecting the path's components re-emits each one +/// behind [`std::path::MAIN_SEPARATOR`]. +/// +/// Borrows unless a rewrite is actually needed. A no-op on Unix, where +/// `/` is already native. +#[cfg(windows)] +fn to_native_separators(path: &Path) -> Cow<'_, Path> { + // WTF-8 keeps ASCII bytes verbatim, so a literal `/` (0x2F) shows up + // here iff the path really carries a forward slash — cheaper than + // allocating a `String` to scan. + if path.as_os_str().as_encoded_bytes().contains(&b'/') { + Cow::Owned(path.components().collect()) + } else { + Cow::Borrowed(path) + } +} + +#[cfg(not(windows))] +fn to_native_separators(path: &Path) -> Cow<'_, Path> { + Cow::Borrowed(path) +} + /// Compute the symlink contents for a true symlink: the path from the /// link's parent directory to `original`, equivalent to /// `path.relative(path.dirname(dest), src)`. @@ -156,7 +191,14 @@ pub struct ForceSymlinkOutcome { /// the `AlreadyExists` and the rename, the initial `AlreadyExists` /// error is surfaced rather than the rename's `NotFound`. pub fn force_symlink_dir(target: &Path, link: &Path) -> io::Result { - force_symlink_inner(target, link, false) + // Normalize separators once, up front, so every filesystem operation + // the retry loop performs on `link` (read_link, remove_dir, rename, + // create_dir_all) — not just the symlink syscall — sees a native + // path. See [`to_native_separators`] for why a stray `/` is fatal on + // Windows. + let target = to_native_separators(target); + let link = to_native_separators(link); + force_symlink_inner(&target, &link, false) } fn force_symlink_inner( diff --git a/pnpm/crates/fs/src/symlink_dir/tests.rs b/pnpm/crates/fs/src/symlink_dir/tests.rs index 567c49b44b..e2ac88a978 100644 --- a/pnpm/crates/fs/src/symlink_dir/tests.rs +++ b/pnpm/crates/fs/src/symlink_dir/tests.rs @@ -8,6 +8,8 @@ use super::relative_target_for; #[cfg(unix)] use super::symlink_dir; +#[cfg(windows)] +use super::to_native_separators; use super::{ForceSymlinkOutcome, force_symlink_dir, read_symlink_dir}; use std::fs; #[cfg(windows)] @@ -131,6 +133,42 @@ fn windows_cross_drive_symlink_target_falls_back_to_absolute() { assert_eq!(relative_target_for(target, link), target); } +/// Regression for the Windows CI failure where a scoped dependency's +/// `node_modules/@scope/name` symlink path — built by joining the +/// `@scope/name` alias as a single segment — kept its forward slash and +/// was rejected by `CreateSymbolicLinkW` with `ERROR_DIRECTORY` +/// (os error 267). The writer must rewrite it to a native `\` path +/// before the syscall. +#[cfg(windows)] +#[test] +fn windows_scoped_alias_path_gets_native_separators() { + let mixed = Path::new(r"C:\store\v11\links\@\pkg\1.0.0\hash\node_modules").join("@scope/name"); + assert!( + mixed.as_os_str().to_string_lossy().contains('/'), + "the join must leave a forward slash for the rewrite to remove: {mixed:?}", + ); + + let native = to_native_separators(&mixed); + assert!( + !native.as_os_str().to_string_lossy().contains('/'), + "no forward slash may survive into the symlink syscall: {native:?}", + ); + assert_eq!( + native.as_ref(), + Path::new(r"C:\store\v11\links\@\pkg\1.0.0\hash\node_modules\@scope\name"), + ); +} + +/// A path that already uses native separators must be returned +/// unchanged (and borrowed, not reallocated). +#[cfg(windows)] +#[test] +fn windows_native_path_is_borrowed_unchanged() { + let native = Path::new(r"C:\store\v11\links\@\pkg\1.0.0\hash\node_modules\dep"); + assert!(matches!(to_native_separators(native), std::borrow::Cow::Borrowed(_))); + assert_eq!(to_native_separators(native).as_ref(), native); +} + #[cfg(windows)] #[test] fn windows_same_drive_symlink_target_stays_relative() { @@ -149,6 +187,26 @@ fn windows_verbatim_and_plain_disk_resolve_to_same_root() { assert!(relative_target_for(target, link).is_relative()); } +/// A scoped dependency's link path is built by joining the whole +/// `@scope/name` alias as one segment. `force_symlink_dir` must create +/// the intervening `@scope` directory and the link regardless of the +/// separator the join left behind. +#[test] +fn force_symlink_dir_links_a_scoped_alias() { + let root = tempdir().expect("create temp dir"); + let target = root.path().join("store").join("node_modules").join("@scope").join("name"); + let modules = root.path().join("app").join("node_modules"); + let link = modules.join("@scope/name"); + fs::create_dir_all(&target).expect("create target dir"); + + let outcome = force_symlink_dir(&target, &link).expect("force_symlink_dir succeeds"); + assert!(!outcome.reused); + + let resolved_link = fs::canonicalize(&link).expect("canonicalize the scoped symlink"); + let resolved_target = fs::canonicalize(&target).expect("canonicalize target"); + assert_eq!(resolved_link, resolved_target); +} + #[test] fn read_symlink_dir_reads_back_what_force_symlink_dir_wrote() { let root = tempdir().expect("create temp dir");