mirror of
https://github.com/pnpm/pnpm.git
synced 2026-07-14 01:32:40 -04:00
fix(fs): rewrite symlink paths to native separators on Windows
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) <noreply@anthropic.com>
This commit is contained in:
5
.changeset/pacquet-scoped-symlink-windows-separators.md
Normal file
5
.changeset/pacquet-scoped-symlink-windows-separators.md
Normal file
@@ -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.
|
||||
@@ -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<ForceSymlinkOutcome> {
|
||||
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(
|
||||
|
||||
@@ -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");
|
||||
|
||||
Reference in New Issue
Block a user