Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixed

- **`wt step copy-ignored` refused a tree that crossed Windows' 260-character path limit**: the containment check canonicalizes the destination and the worktree root, and `dunce` strips the `\\?\` prefix only from paths of 260 characters or fewer — so a deep `node_modules` came back spelled `\\?\C:\…` while its root stayed `C:\…`, which `starts_with` reads as a different drive. The canonicalizer now returns one spelling at any length, and the copy proceeds as it does elsewhere. The same split silently dropped the subdirectory that `wt switch` (and `remove`, `merge`, `step relocate`) `cd`s you into from a directory that deep, landing you at the worktree root instead; both call sites now share the one canonicalizer. (fixes [#3898](https://github.com/max-sixty/worktrunk/issues/3898), thanks @Persedes for reporting)
Comment thread
worktrunk-bot marked this conversation as resolved.
Resolved

## 0.74.0

### Improved
Expand Down
38 changes: 38 additions & 0 deletions src/copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,44 @@ mod tests {
assert_eq!(fs::read(dest.join("nested").join("b")).unwrap(), b"b");
}

#[test]
fn test_copy_dir_recursive_copies_a_tree_deeper_than_windows_max_path() {
// A `node_modules` tree nests deep enough to push the destination past
// Windows' 260-character path limit. The containment check canonicalizes
// both ends, and used to get one spelling for the long destination and
// another for its short root — so the copy was refused as "outside" a
// worktree it was inside (#3898). Off Windows the depth is unremarkable
// and this just exercises a deep tree.
let dir = tempfile::tempdir().unwrap();
let src_root = dir.path().join("src");
let dest_root = dir.path().join("dest");

let mut relative = PathBuf::from("node_modules");
while dest_root.join(&relative).as_os_str().len() < 320 {
relative.push("nested-dependency-directory");
}
let deep_src = src_root.join(&relative);
fs::create_dir_all(&deep_src).unwrap();
// The real destination root is an existing worktree.
fs::create_dir_all(&dest_root).unwrap();
fs::write(deep_src.join("payload.txt"), b"deep file").unwrap();

let skipped = copy_dir_recursive(
&src_root.join("node_modules"),
&dest_root.join("node_modules"),
Some(&dest_root),
false,
&Progress::disabled(),
)
.expect("a tree inside the destination root should copy");

assert_eq!(skipped, 0);
assert_eq!(
fs::read(dest_root.join(&relative).join("payload.txt")).unwrap(),
b"deep file"
);
}

#[test]
fn test_copy_dir_recursive_missing_source_root_errors() {
// A subdirectory that vanishes mid-walk is skipped, but the root the
Expand Down
35 changes: 30 additions & 5 deletions src/output/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use worktrunk::git::{
BranchDeletionMode, BranchDeletionOutcome, BranchDeletionResult, RemoveOptions,
execute_branch_deletion, remove_worktree_with_cleanup, stage_worktree_removal,
};
use worktrunk::path::format_path_for_display;
use worktrunk::path::{canonicalize_with_parents, format_path_for_display};
use worktrunk::progress::{Progress, format_stats_paren};
use worktrunk::remove_dir::remove_dir_with_progress;
use worktrunk::styling::{
Expand Down Expand Up @@ -998,10 +998,13 @@ pub(crate) fn resolve_subdir_in_target(
cwd: &Path,
) -> PathBuf {
if let Some(source_root) = source_root {
// Canonicalize both paths to handle symlinks (e.g., /var -> /private/var on macOS)
let cwd = dunce::canonicalize(cwd).unwrap_or_else(|_| cwd.to_path_buf());
let source_root =
dunce::canonicalize(source_root).unwrap_or_else(|_| source_root.to_path_buf());
// Canonicalize both paths to handle symlinks (e.g., /var -> /private/var
// on macOS). Through `canonicalize_with_parents` rather than `dunce`
// directly, so a cwd past Windows' 260-character limit is spelled like
// the short worktree root containing it — otherwise `strip_prefix` below
// fails and the shell silently lands at the target root (#3898).
let cwd = canonicalize_with_parents(cwd);
let source_root = canonicalize_with_parents(source_root);
if let Ok(relative) = cwd.strip_prefix(&source_root)
&& !relative.as_os_str().is_empty()
{
Expand Down Expand Up @@ -2549,6 +2552,28 @@ prunable gitdir file points to non-existent location
assert_eq!(result, target.join("apps/gateway"));
}

#[test]
fn test_resolve_subdir_in_target_subdir_deeper_than_windows_max_path() {
// A cwd nested past Windows' 260-character path limit comes back from
// `dunce` spelled `\\?\C:\…` while its short worktree root stays `C:\…`,
// so `strip_prefix` fails and the shell lands at the target root with no
// error rather than at the mirrored subdirectory (#3898). Off Windows
// the depth is unremarkable and this just exercises a deep tree.
let dir = tempfile::tempdir().unwrap();
let source = dir.path().join("source");
let target = dir.path().join("target");

let mut relative = PathBuf::from("node_modules");
while source.join(&relative).as_os_str().len() < 320 {
relative.push("nested-dependency-directory");
}
std::fs::create_dir_all(source.join(&relative)).unwrap();
std::fs::create_dir_all(target.join(&relative)).unwrap();

let result = resolve_subdir_in_target(&target, Some(&source), &source.join(&relative));
assert_eq!(result, target.join(&relative));
}

#[test]
fn test_resolve_subdir_in_target_subdir_missing() {
let dir = tempfile::tempdir().unwrap();
Expand Down
102 changes: 101 additions & 1 deletion src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,21 @@ pub fn expand_tilde(path: &Path) -> Cow<'_, Path> {
/// and `WorkingTree::ensure_holds_this_worktree` compares a worktree
/// registration's recorded path against the directory sitting at it to decide
/// whether removal may delete that directory.
pub(crate) fn canonicalize_with_parents(path: &Path) -> PathBuf {
///
/// On Windows a drive path's result never carries a `\\?\` prefix — a
/// `VerbatimUNC` share keeps its own — because every caller compares it against
/// another result of this function. `dunce` strips that
/// prefix only from paths of 260 characters or fewer, so a deep path and the
/// short root containing it come back spelled `\\?\C:\…` and `C:\…` — one
/// `Prefix::VerbatimDisk` component and one `Prefix::Disk`, which
/// [`Path::starts_with`] reads as unrelated drives. That split made
/// `wt step copy-ignored` refuse to copy a deep `node_modules` tree into a
/// worktree it was squarely inside (#3898).
pub fn canonicalize_with_parents(path: &Path) -> PathBuf {
strip_verbatim_disk_prefix(resolve_with_parents(path))
}

fn resolve_with_parents(path: &Path) -> PathBuf {
if let Ok(canonical) = canonicalize(path) {
return canonical;
}
Expand All @@ -196,6 +210,46 @@ pub(crate) fn canonicalize_with_parents(path: &Path) -> PathBuf {
result
}

/// Drop the `\\?\` prefix from a canonicalized drive path. A no-op off Windows.
///
/// Only a `VerbatimDisk` prefix is stripped: `\\?\UNC\server\share` names a
/// share, and cutting the same four characters off it would leave the relative
/// `UNC\server\share`. Unlike `dunce`, nothing about the path's contents keeps
/// the prefix — not its length, and not the reserved DOS names (`con`, `aux`,
/// `com1`) or trailing dots and spaces that `dunce`'s `is_safe_to_strip_unc`
/// also declines to strip. [`canonicalize_with_parents`] feeds comparisons and
/// messages, not the legacy APIs those rules protect, and `std::fs` re-applies
/// a verbatim prefix itself for any path it opens. What that trades is confined
/// to the message case: `registration_worktree_path` shows its result in
/// `GitError::WorktreePathNotOurs`, and for a path a reserved component makes
/// unreachable under the legacy spelling, the reported spelling is one the user
/// can't `cd` into.
#[cfg(windows)]
fn strip_verbatim_disk_prefix(path: PathBuf) -> PathBuf {
use std::ffi::OsString;
use std::os::windows::ffi::{OsStrExt, OsStringExt};
use std::path::{Component, Prefix};

let verbatim_disk = matches!(
path.components().next(),
Some(Component::Prefix(prefix)) if matches!(prefix.kind(), Prefix::VerbatimDisk(_))
);
if !verbatim_disk {
return path;
}
// Cut the prefix in UTF-16 rather than slicing a `&str`, so a path `to_str`
// can't render is spelled like every other result instead of keeping the
// split this function exists to close. `\\?\` is four ASCII characters, so
// four UTF-16 units.
let stripped: Vec<u16> = path.as_os_str().encode_wide().skip(4).collect();
PathBuf::from(OsString::from_wide(&stripped))
}

#[cfg(not(windows))]
fn strip_verbatim_disk_prefix(path: PathBuf) -> PathBuf {
path
}

/// Compare two paths for equality, canonicalizing to handle symlinks and relative paths.
///
/// Returns `true` if the paths resolve to the same location.
Expand Down Expand Up @@ -600,6 +654,52 @@ mod tests {
);
}

/// A path long enough that `dunce` keeps its `\\?\` prefix must still
/// compare as a descendant of the short root it sits under — the split that
/// made `wt step copy-ignored` refuse a deep `node_modules` tree (#3898).
#[test]
#[cfg(windows)]
fn canonicalize_with_parents_agrees_across_the_verbatim_threshold() {
let tmp = tempfile::tempdir().expect("temp dir");
let root = tmp.path();

// 260 characters is `dunce`'s cutoff for stripping the prefix; overshoot
// it so the deep end is unambiguously on the other side.
let mut deep = root.to_path_buf();
while deep.as_os_str().len() < 320 {
deep.push("nested-dependency-directory");
}
std::fs::create_dir_all(&deep).expect("create deep tree");

// Pin the mechanism, so a `dunce` that stopped splitting the two
// spellings shows up here rather than leaving the assertion below
// passing for a reason it was not written for.
let verbatim = |p: &Path| {
dunce::canonicalize(p)
.expect("canonicalize")
.to_string_lossy()
.starts_with(r"\\?\")
};
assert!(verbatim(&deep), "expected dunce to keep the prefix here");
assert!(!verbatim(root), "expected dunce to strip the prefix here");

// The path `ensure_path_within_root` actually checks is a destination
// directory that has not been created yet, so ask about a child.
let canonical_root = canonicalize_with_parents(root);
let canonical_deep = canonicalize_with_parents(&deep.join("not-yet-created"));
assert!(
!canonical_deep.to_string_lossy().starts_with(r"\\?\"),
"expected the deep result to drop the verbatim prefix, got {}",
canonical_deep.display()
);
assert!(
canonical_deep.starts_with(&canonical_root),
"{} should sit under {}",
canonical_deep.display(),
canonical_root.display()
);
}

#[test]
#[cfg(target_os = "macos")]
fn test_paths_match_macos_var_symlink() {
Expand Down
Loading