diff --git a/AGENTS.md b/AGENTS.md index 0342b30c..e3e8911f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -112,7 +112,9 @@ and `cargo build --release` directly; CI runs the gate on both systems. mounts in the selected machine's namespace without probing them; scans stop at nested mounts, including bind mounts and same-device subvolumes. Switching machines clears marks and identity. The Unix SSH helper streams - records and never retains the tree. + records and never retains the tree. SSH hosts are Unix, so remote paths + are POSIX on every client: join, split and compare them through + `PathStyle`, never `std::path`, which uses `\` on Windows. 11. **A live overview is not an actionable tree.** Keep provisional, bounded display summaries separate from `Disktree::tree` and all path, @@ -125,6 +127,7 @@ and `cargo build --release` directly; CI runs the gate on both systems. | --- | --- | | bounded Unix SSH traversal | `crates/disktree-core/src/remote_walk.rs` | | SSH protocol, discovery and transport | `crates/disktree-core/src/remote.rs` | +| joining, splitting or comparing a scanned path | `crates/disktree-core/src/path_style.rs` | | measurement, filtering, parallelism | `crates/disktree-core/src/scan.rs` | | what a node is, or a derived total | `crates/disktree-core/src/tree.rs` | | tile geometry, nesting, the merged tail | `crates/disktree-core/src/treemap.rs` | diff --git a/crates/disktree-app/src/main.rs b/crates/disktree-app/src/main.rs index 904a9319..f0bdd84e 100644 --- a/crates/disktree-app/src/main.rs +++ b/crates/disktree-app/src/main.rs @@ -177,6 +177,8 @@ fn run() -> Result<()> { let root = args.root.clone(); let depth = args.depth; let title_root = root.clone(); + let title_style = + disktree_core::path_style::PathStyle::of(args.remote.is_some()); gpui_kit::application() .with_assets(gpui_kit::assets::Assets) @@ -206,6 +208,7 @@ fn run() -> Result<()> { marks::display_path( &title_root, home.as_deref(), + title_style, ) ) .into(), @@ -438,6 +441,14 @@ fn parse_args_with_power( ); let choose_root = root.is_none() && !disk && !check; if remote.is_some() { + // SSH hosts are Unix: a root such as `C:\x` names nothing there, and + // a relative one would be resolved against an unknown directory. + if let Some(root) = &root { + anyhow::ensure!( + disktree_core::remote::is_remote_absolute(root), + "an SSH path must start with /, as on the Unix host" + ); + } return Ok(Args { root: root.unwrap_or_default(), remote, @@ -593,6 +604,17 @@ mod argument_tests { assert_eq!(args.options.threads.max_threads, 4); assert_eq!(args.root, PathBuf::from("/only-on-the-remote-machine")); assert!(args.power.is_none()); + for root in ["relative", r"C:\Users", r"\srv"] { + assert!( + parse_args( + ["ssh", "dev", root] + .into_iter() + .map(std::ffi::OsString::from), + ) + .is_err(), + "{root} is not a path on a Unix host" + ); + } let args = parse_args( ["ssh", "dev", "--disk"] diff --git a/crates/disktree-app/src/marks.rs b/crates/disktree-app/src/marks.rs index 7a03e476..483bfc49 100644 --- a/crates/disktree-app/src/marks.rs +++ b/crates/disktree-app/src/marks.rs @@ -4,22 +4,28 @@ //! re-scanned after a removal, and a mark must survive that (or be reported as //! gone) rather than silently pointing at a different node. -use std::path::{Path, PathBuf}; +use std::ffi::OsString; +use std::path::Path; +use disktree_core::path_style::PathStyle; use disktree_core::removal::Target; use disktree_core::tree::{Metric, Node}; use rustc_hash::FxHashSet; /// Marked paths, in the order they were marked. +/// +/// The index holds the exact text of each path. `PathBuf` equality on +/// Windows compares components and treats `\` as a separator, so a Unix +/// host's `/a/b\c` would equal `/a/b/c` there. #[derive(Debug, Default)] pub struct Marks { items: Vec, - index: FxHashSet, + index: FxHashSet, } impl Marks { pub fn contains(&self, path: &Path) -> bool { - self.index.contains(path) + self.index.contains(path.as_os_str()) } pub fn items(&self) -> &[Target] { @@ -38,19 +44,22 @@ impl Marks { /// /// Returns `true` when the path ended up marked. pub fn toggle(&mut self, target: Target) -> bool { - if self.index.remove(&target.path) { - self.items.retain(|item| item.path != target.path); + if self.index.remove(target.path.as_os_str()) { + self.items.retain(|item| { + item.path.as_os_str() != target.path.as_os_str() + }); false } else { - self.index.insert(target.path.clone()); + self.index.insert(target.path.clone().into_os_string()); self.items.push(target); true } } pub fn remove(&mut self, path: &Path) { - if self.index.remove(path) { - self.items.retain(|item| item.path != path); + if self.index.remove(path.as_os_str()) { + self.items + .retain(|item| item.path.as_os_str() != path.as_os_str()); } } @@ -61,14 +70,20 @@ impl Marks { /// Re-read sizes from a freshly scanned tree and drop marks whose path no /// longer exists, so the tally never claims space that is already gone. - pub fn refresh(&mut self, root_path: &Path, root: &Node, _metric: Metric) { + pub fn refresh( + &mut self, + root_path: &Path, + root: &Node, + _metric: Metric, + style: PathStyle, + ) { let mut resolved = Vec::with_capacity(self.items.len()); for item in &self.items { - match find(root_path, root, &item.path) { + match find(root_path, root, &item.path, style) { Some(node) => resolved.push(Target { bytes: node.bytes, is_dir: node.is_dir(), - hidden: is_hidden(&item.path), + hidden: is_hidden(&item.path, style), path: item.path.clone(), }), None => resolved.push(Target { @@ -81,17 +96,17 @@ impl Marks { } } -/// Walk the tree to the node at an absolute path. Path components are compared -/// one at a time, so a name containing a path separator cannot confuse it. +/// Walk the tree to the node at an absolute path. Names are compared one at a +/// time, split by the scanned machine's rules, so a name containing another +/// platform's separator cannot confuse it. pub fn find<'a>( root_path: &Path, root: &'a Node, path: &Path, + style: PathStyle, ) -> Option<&'a Node> { - let relative = path.strip_prefix(root_path).ok()?; let mut node = root; - for component in relative.components() { - let name = component.as_os_str().to_string_lossy(); + for name in style.relative(root_path, path)? { node = node .children .iter() @@ -101,22 +116,29 @@ pub fn find<'a>( } /// A dotfile or dot-directory by name. -pub fn is_hidden(path: &Path) -> bool { - path.file_name() +pub fn is_hidden(path: &Path, style: PathStyle) -> bool { + style + .file_name(path) .is_some_and(|name| name.to_string_lossy().starts_with('.')) } /// Shorten a path for display: `~` for the home directory, and the path with -/// the home prefix replaced when it is below it. The separator after `~` is -/// the platform's, so Windows shows `~\AppData\Local`, not `~/AppData\Local`. -pub fn display_path(path: &Path, home: Option<&Path>) -> String { - match home - .and_then(|home| path.strip_prefix(home).ok().map(|rest| (home, rest))) - { - Some((_, rest)) if rest.as_os_str().is_empty() => "~".to_string(), - Some((_, rest)) => { - format!("~{}{}", std::path::MAIN_SEPARATOR, rest.display()) - } +/// the home prefix replaced when it is below it. The separator is the scanned +/// machine's, so Windows shows `~\AppData\Local`, not `~/AppData\Local`, and +/// a Unix host shows `~/.cache` on every client. +pub fn display_path( + path: &Path, + home: Option<&Path>, + style: PathStyle, +) -> String { + let separator = style.separator(); + match home.and_then(|home| style.relative(home, path)) { + Some(names) if names.is_empty() => "~".to_string(), + Some(names) => names.iter().fold("~".to_string(), |mut text, name| { + text.push(separator); + text.push_str(name); + text + }), None => path.display().to_string(), } } @@ -125,6 +147,9 @@ pub fn display_path(path: &Path, home: Option<&Path>) -> String { mod tests { use super::*; use disktree_core::tree::{NodeKind, aggregate}; + use std::path::PathBuf; + + const NATIVE: PathStyle = PathStyle::Native; fn file(name: &str, bytes: u64) -> Node { Node::entry(name, NodeKind::File, bytes) @@ -176,7 +201,7 @@ mod tests { marks.toggle(target("/home/tobi/.cache", 0)); marks.toggle(target("/home/tobi/gone", 500)); - marks.refresh(root_path, &root, Metric::Bytes); + marks.refresh(root_path, &root, Metric::Bytes, NATIVE); assert_eq!(marks.items()[0].bytes, 900); assert!(marks.items()[0].is_dir); assert!(marks.items()[0].hidden, "the .cache mark is hidden"); @@ -190,7 +215,8 @@ mod tests { find( Path::new("/home/tobi"), &root, - Path::new("/home/tobi/.cache") + Path::new("/home/tobi/.cache"), + NATIVE ) .is_some() ); @@ -198,7 +224,8 @@ mod tests { find( Path::new("/home/tobi"), &root, - Path::new("/home/tobi/.cache/blob.bin") + Path::new("/home/tobi/.cache/blob.bin"), + NATIVE ) .is_some() ); @@ -206,7 +233,8 @@ mod tests { find( Path::new("/home/tobi"), &root, - Path::new("/home/tobi/cache") + Path::new("/home/tobi/cache"), + NATIVE ) .is_none() ); @@ -214,7 +242,8 @@ mod tests { find( Path::new("/elsewhere"), &root, - Path::new("/home/tobi/notes.bin") + Path::new("/home/tobi/notes.bin"), + NATIVE ) .is_none() ); @@ -225,11 +254,56 @@ mod tests { let home = Path::new("/home/tobi"); let separator = std::path::MAIN_SEPARATOR; assert_eq!( - display_path(&home.join(".cache").join("npm"), Some(home)), + display_path(&home.join(".cache").join("npm"), Some(home), NATIVE), format!("~{separator}.cache{separator}npm") ); - assert_eq!(display_path(home, Some(home)), "~"); - assert_eq!(display_path(Path::new("/var/log"), Some(home)), "/var/log"); - assert_eq!(display_path(Path::new("/var/log"), None), "/var/log"); + assert_eq!(display_path(home, Some(home), NATIVE), "~"); + assert_eq!( + display_path(Path::new("/var/log"), Some(home), NATIVE), + "/var/log" + ); + assert_eq!( + display_path(Path::new("/var/log"), None, NATIVE), + "/var/log" + ); + } + + /// A Unix host's paths read the same on every client: `/` after `~`, and + /// a name holding `\` is one name, not two. + #[test] + fn remote_paths_display_and_resolve_by_posix_rules() { + let posix = PathStyle::Posix; + let home = Path::new("/home/tobi"); + assert_eq!( + display_path(Path::new("/home/tobi/.cache/npm"), Some(home), posix), + "~/.cache/npm" + ); + assert_eq!( + display_path(Path::new("/home/tobix"), Some(home), posix), + "/home/tobix" + ); + let mut root = Node::directory("home"); + root.children.push(file(r"a\b", 7)); + assert!( + find(home, &root, Path::new(r"/home/tobi/a\b"), posix).is_some() + ); + assert!( + find(home, &root, Path::new("/home/tobi/a/b"), posix).is_none() + ); + assert!(!is_hidden(Path::new(r"/srv/x\.hidden"), posix)); + assert!(is_hidden(Path::new("/srv/.cache"), posix)); + } + + /// Marks are keyed by the exact path text, so `/a/b\c` and `/a/b/c` + /// stay two marks even where `PathBuf` would call them equal. + #[test] + fn marks_compare_the_exact_path() { + let mut marks = Marks::default(); + marks.toggle(target(r"/srv/b\c", 1)); + assert!(!marks.contains(Path::new("/srv/b/c"))); + assert!(marks.toggle(target("/srv/b/c", 1))); + assert_eq!(marks.len(), 2); + marks.remove(Path::new("/srv/b/c")); + assert!(marks.contains(Path::new(r"/srv/b\c"))); } } diff --git a/crates/disktree-app/src/state.rs b/crates/disktree-app/src/state.rs index efb9730a..18928a8e 100644 --- a/crates/disktree-app/src/state.rs +++ b/crates/disktree-app/src/state.rs @@ -13,6 +13,7 @@ use std::time::{Duration, Instant}; use disktree_core::access::file_table_readable; use disktree_core::filter::{Keep, Matches, filter}; use disktree_core::insights::{Candidate, worth_a_look}; +use disktree_core::path_style::PathStyle; use disktree_core::removal::{ Plan, RemovalEvent, RemovalHandle, RemovalMode, Target, TrashBackend, detect_trash_backend, plan, @@ -36,7 +37,7 @@ use rustc_hash::{FxHashMap, FxHashSet}; use crate::git::GitState; -use crate::marks::{Marks, display_path, is_hidden}; +use crate::marks::{Marks, is_hidden}; use crate::treemap_view::{Mosaic, TileDeco}; /// What a tile's colour says. @@ -663,7 +664,12 @@ impl Disktree { scan.cancel(); } app.scan_epoch += 1; - app.marks.refresh(&app.root_path, &tree, app.options.metric); + app.marks.refresh( + &app.root_path, + &tree, + app.options.metric, + app.path_style(), + ); app.tree = Some(Arc::new(tree)); app.cache = None; app.refresh_insights(); @@ -948,7 +954,12 @@ impl Disktree { cx.notify(); return; } - if !path.is_absolute() { + let absolute = if self.volumes_source.is_some() { + disktree_core::remote::is_remote_absolute(&path) + } else { + path.is_absolute() + }; + if !absolute { self.picker_error = Some("Enter an absolute directory path.".into()); cx.notify(); @@ -1363,6 +1374,7 @@ impl Disktree { &self.root_path, &node, self.options.metric, + self.path_style(), ); self.tree = Some(Arc::new(node)); self.refresh_insights(); @@ -1411,7 +1423,12 @@ impl Disktree { self.file_table = file_table_readable(&self.root_path); } let metric = self.options.metric; - self.marks.refresh(&self.root_path, &node, metric); + self.marks.refresh( + &self.root_path, + &node, + metric, + self.path_style(), + ); discard(self.tree.replace(Arc::new(node))); self.cache = None; self.refresh_insights(); @@ -1516,7 +1533,22 @@ impl Disktree { pub fn path_at(&self, crumbs: &[usize]) -> Option { let tree = self.tree.as_ref()?; - Some(path_of(&self.root_path, tree, crumbs)) + Some(path_of(&self.root_path, tree, crumbs, self.path_style())) + } + + /// The path rules of the machine on screen: an SSH host is Unix, so its + /// paths are POSIX on every client. + pub const fn path_style(&self) -> PathStyle { + PathStyle::of(self.remote.is_some()) + } + + /// `path` shortened under the scanned machine's home, in its separators. + pub fn display_path(&self, path: &Path) -> String { + crate::marks::display_path( + path, + self.home.as_deref(), + self.path_style(), + ) } /// Path of the directory currently drawn. @@ -1529,16 +1561,21 @@ impl Disktree { /// The trail, from `/`: the directories above the scanned root, which /// widen the scan, then the root and the path into the tree. pub fn breadcrumbs(&self) -> Vec<(String, Crumb)> { - let mut trail: Vec<(String, Crumb)> = self - .root_path - .ancestors() + let style = self.path_style(); + let mut trail: Vec<(String, Crumb)> = style + .ancestors(&self.root_path) .skip(1) .collect::>() .into_iter() .rev() - .map(|path| (crumb_label(path), Crumb::Above(path.to_path_buf()))) + .map(|path| { + (crumb_label(path, style), Crumb::Above(path.to_path_buf())) + }) .collect(); - trail.push((crumb_label(&self.root_path), Crumb::Tree(Vec::new()))); + trail.push(( + crumb_label(&self.root_path, style), + Crumb::Tree(Vec::new()), + )); let Some(tree) = &self.tree else { return trail; }; @@ -2119,8 +2156,8 @@ impl Disktree { self.notice = Some(( format!( "{} goes with the marked {}; unmark that to keep it", - display_path(&target.path, self.home.as_deref()), - display_path(&ancestor, self.home.as_deref()) + self.display_path(&target.path), + self.display_path(&ancestor) ), Status::Warning, )); @@ -2128,15 +2165,14 @@ impl Disktree { return; } let path = target.path.clone(); + let style = self.path_style(); if self.marks.toggle(target) { self.space_baseline = self.space_baseline.or(self.space); let inside: Vec = self .marks .items() .iter() - .filter(|item| { - item.path != path && item.path.starts_with(&path) - }) + .filter(|item| style.is_below(&item.path, &path)) .map(|item| item.path.clone()) .collect(); for inner in &inside { @@ -2146,7 +2182,7 @@ impl Disktree { self.notice = Some(( format!( "{} now covers {} mark{} inside it", - display_path(&path, self.home.as_deref()), + self.display_path(&path), inside.len(), if inside.len() == 1 { "" } else { "s" } ), @@ -2162,7 +2198,7 @@ impl Disktree { self.marks .items() .iter() - .filter(|item| item.path != path && path.starts_with(&item.path)) + .filter(|item| self.path_style().is_below(path, &item.path)) .map(|item| item.path.clone()) .min_by_key(|ancestor| ancestor.as_os_str().len()) } @@ -2171,7 +2207,7 @@ impl Disktree { let node = self.node_at(crumbs)?; let path = self.path_at(crumbs)?; Some(Target { - hidden: is_hidden(&path), + hidden: is_hidden(&path, self.path_style()), path, bytes: node.bytes, is_dir: node.is_dir(), @@ -2200,8 +2236,7 @@ impl Disktree { .items() .iter() .filter(|target| { - target.path != self.root_path - && target.path.starts_with(&self.root_path) + self.path_style().is_below(&target.path, &self.root_path) }) .cloned() .collect(); @@ -2350,12 +2385,13 @@ impl Disktree { pub fn crumbs_for_path(&self, path: &Path) -> Option> { // The path is relative to the scanned root, so the walk starts there, // not at the directory currently drawn. - let relative = path.strip_prefix(&self.root_path).ok()?; + // Names are split by the scanned machine's rules, so a Unix host's + // `a\b` stays one name on a Windows client. + let relative = self.path_style().relative(&self.root_path, path)?; let tree = self.tree.clone()?; let mut node: &Node = &tree; let mut crumbs = Vec::new(); - for component in relative.components() { - let name = component.as_os_str().to_string_lossy(); + for name in relative { let index = node .children .iter() @@ -2508,10 +2544,12 @@ impl Disktree { disktree_core::tree::aggregate(&mut tree, self.options.metric); discard(self.tree.replace(Arc::new(tree))); let metric = self.options.metric; + let style = self.path_style(); self.marks.refresh( &self.root_path, self.tree.as_ref().unwrap(), metric, + style, ); } self.crumbs = self.crumbs_for_path(&directory).unwrap_or_default(); @@ -3023,10 +3061,7 @@ impl Disktree { // GPUI's reveal cannot report failure, so check what it can't. if std::fs::symlink_metadata(&path).is_err() { self.notice = Some(( - format!( - "{} is no longer on disk", - crate::marks::display_path(&path, self.home.as_deref()) - ), + format!("{} is no longer on disk", self.display_path(&path)), Status::Warning, )); cx.notify(); @@ -3561,8 +3596,8 @@ const HEADER_REMS: f32 = 1.375; const HEADER_INNER_REMS: f32 = 1.0; /// A trail step's label: the directory's own name, or `/` for the root. -fn crumb_label(path: &Path) -> String { - path.file_name().map_or_else( +fn crumb_label(path: &Path, style: PathStyle) -> String { + style.file_name(path).map_or_else( || path.display().to_string(), |name| name.to_string_lossy().into_owned(), ) @@ -3725,10 +3760,7 @@ impl Render for Disktree { self.remote.as_ref().map_or_else(String::new, |host| { format!(" · {host} · read-only") }), - crate::marks::display_path( - &self.current_path(), - self.home.as_deref() - ) + self.display_path(&self.current_path()) ) }; if title != self.window_title { diff --git a/crates/disktree-app/src/tests.rs b/crates/disktree-app/src/tests.rs index 0520e4cb..7b2ec47b 100644 --- a/crates/disktree-app/src/tests.rs +++ b/crates/disktree-app/src/tests.rs @@ -7,6 +7,7 @@ use std::path::{Path, PathBuf}; +use disktree_core::path_style::PathStyle; use disktree_core::removal::RemovalMode; use disktree_core::scan::{ScanOptions, scan}; use disktree_core::space::{SpaceInfo, Volume}; @@ -65,6 +66,29 @@ fn view_over<'a>( (view, cx) } +/// The fixture as an SSH session holds it. Remote paths are POSIX on every +/// client, so the root must be `/`-rooted: the temporary directory itself on +/// Unix, which keeps "nothing on disk changed" checks meaningful, and a Unix +/// path on Windows, whose temporary directory is `C:\…`. +fn remote_view_over<'a>( + temp: &Path, + cx: &'a mut TestAppContext, +) -> (Entity, &'a mut Window, PathBuf) { + let root = if cfg!(unix) { + temp.to_path_buf() + } else { + PathBuf::from("/srv/fixture") + }; + let tree = scan(temp, options()).expect("scan the fixture"); + let root_path = root.clone(); + let (view, cx) = cx.add_window_view(move |_, cx| { + Disktree::with_tree(root_path.clone(), tree, options(), 3, cx) + }); + let focus = view.read_with(cx, |app, _| app.focus.clone()); + cx.update(|window, cx| window.focus(&focus, cx)); + (view, cx, root) +} + fn draw(cx: &mut Window) { cx.update(|window, cx| { window.draw(cx).clear(cx); @@ -305,14 +329,15 @@ fn switching_from_remote_to_local_clears_machine_specific_marks( cx.update(gpui_omarchy::init); let temp = fixture(); let target = fixture(); - let (view, cx) = view_over(temp.path(), cx); + let (view, cx, root) = remote_view_over(temp.path(), cx); + let junk = PathStyle::Posix.join(&root, "junk"); update(&view, cx, |app, cx| { app.remote = Some("old-source".into()); app.remote_identity = Some(disktree_core::identity::HostIdentity { hostname: "old-source".into(), machine_id: None, }); - let crumbs = app.crumbs_for_path(&temp.path().join("junk")).unwrap(); + let crumbs = app.crumbs_for_path(&junk).unwrap(); app.select(Some(crumbs), cx); app.toggle_mark_selected(cx); assert!(!app.marks.is_empty()); @@ -474,7 +499,8 @@ fn repointing_an_ssh_alias_never_reuses_old_machine_marks( ) { cx.update(gpui_omarchy::init); let temp = fixture(); - let (view, cx) = view_over(temp.path(), cx); + let (view, cx, root) = remote_view_over(temp.path(), cx); + let junk = PathStyle::Posix.join(&root, "junk"); update(&view, cx, |app, cx| { let old = disktree_core::identity::HostIdentity { hostname: "same-name".into(), @@ -482,7 +508,7 @@ fn repointing_an_ssh_alias_never_reuses_old_machine_marks( }; app.remote = Some("stable-alias".into()); app.update_remote_identity(Some(old.clone())); - let crumbs = app.crumbs_for_path(&temp.path().join("junk")).unwrap(); + let crumbs = app.crumbs_for_path(&junk).unwrap(); app.select(Some(crumbs), cx); app.toggle_mark_selected(cx); app.update_remote_identity(Some(old.clone())); @@ -507,10 +533,11 @@ fn remote_review_never_deletes_even_through_direct_actions( ) { cx.update(gpui_omarchy::init); let temp = fixture(); - let (view, cx) = view_over(temp.path(), cx); + let (view, cx, root) = remote_view_over(temp.path(), cx); + let junk = PathStyle::Posix.join(&root, "junk"); update(&view, cx, |app, cx| { app.remote = Some("dev".into()); - let crumbs = app.crumbs_for_path(&temp.path().join("junk")).unwrap(); + let crumbs = app.crumbs_for_path(&junk).unwrap(); app.select(Some(crumbs), cx); app.toggle_mark_selected(cx); app.screen = Screen::Review; @@ -1014,7 +1041,10 @@ fn enter_in_the_focused_volume_picker_scans_the_selected_root( ) { cx.update(gpui_omarchy::init); let temp = fixture(); - let target = temp.path().join("junk"); + // Accepting a local root canonicalizes it, and the temporary directory + // is behind a link on macOS (/var -> /private/var) and a short 8.3 name + // on Windows runners, so compare against the resolved path. + let target = dunce::canonicalize(temp.path().join("junk")).unwrap(); let (view, cx) = view_over(temp.path(), cx); update(&view, cx, |app, _| { app.volumes = vec![disktree_core::space::Volume { @@ -1266,7 +1296,8 @@ fn remote_review_copies_host_guarded_script_and_prompt( ) { cx.update(gpui_omarchy::init); let temp = fixture(); - let (view, cx) = view_over(temp.path(), cx); + let (view, cx, root) = remote_view_over(temp.path(), cx); + let junk = PathStyle::Posix.join(&root, "junk"); update(&view, cx, |app, cx| { app.remote = Some("dev-alias".into()); app.remote_identity = Some(disktree_core::identity::HostIdentity { @@ -1274,7 +1305,7 @@ fn remote_review_copies_host_guarded_script_and_prompt( machine_id: Some("abcdefabcdefabcdefabcdefabcdefab".into()), }); app.marks.toggle(disktree_core::removal::Target { - path: temp.path().join("junk"), + path: junk.clone(), bytes: 300_000, is_dir: true, hidden: false, @@ -1304,6 +1335,66 @@ fn remote_review_copies_host_guarded_script_and_prompt( assert!(temp.path().join("junk/blob.bin").exists()); } +/// A Unix host's paths are POSIX on every client. A Windows client must not +/// join them with `\`, split a name such as `logs\old` or `a:b`, or export a +/// delete script that names a different file on the host. +#[gpui_kit::test] +fn remote_paths_keep_the_hosts_separators_on_every_client( + cx: &mut TestAppContext, +) { + use disktree_core::tree::{Metric, Node, NodeKind, aggregate}; + + cx.update(gpui_omarchy::init); + let mut root = Node::directory("data"); + let mut logs = Node::directory(r"logs\old"); + logs.children + .push(Node::entry("a:b", NodeKind::File, 4_000)); + root.children.push(logs); + root.children + .push(Node::entry("notes", NodeKind::File, 1_000)); + aggregate(&mut root, Metric::Bytes); + let root_path = PathBuf::from("/srv/data"); + let (view, cx) = cx.add_window_view(move |_, cx| { + let mut app = + Disktree::with_tree(root_path.clone(), root, options(), 3, cx); + app.remote = Some("dev".into()); + app + }); + let focus = read(&view, cx, |app| app.focus.clone()); + cx.update(|window, cx| window.focus(&focus, cx)); + draw(cx); + update(&view, cx, |app, cx| { + let file = Path::new(r"/srv/data/logs\old/a:b"); + let crumbs = app.crumbs_for_path(file).expect("one name, not two"); + assert_eq!(app.path_at(&crumbs).as_deref(), Some(file)); + assert!( + app.crumbs_for_path(Path::new("/srv/data/logs/old")) + .is_none() + ); + app.select(Some(crumbs), cx); + app.toggle_mark_selected(cx); + assert!(app.marks.contains(file)); + let labels: Vec<_> = app + .breadcrumbs() + .into_iter() + .map(|(label, _)| label) + .collect(); + assert_eq!(labels, ["/", "srv", "data"]); + app.home = Some(PathBuf::from("/srv")); + assert_eq!(app.display_path(file), r"~/data/logs\old/a:b"); + app.screen = Screen::Review; + cx.notify(); + }); + draw(cx); + press(cx, "y"); + let script = cx + .read_from_clipboard() + .and_then(|item| item.text()) + .expect("script on clipboard"); + assert!(script.contains(r"'/srv/data/logs\old/a:b'"), "{script}"); + assert!(!script.contains(r"data\logs"), "{script}"); +} + /// Escape in the alert dialog cancels: the dialog closes, the review screen /// stays, and nothing on disk changes. #[gpui_kit::test] diff --git a/crates/disktree-app/src/views.rs b/crates/disktree-app/src/views.rs index 32aedf5f..ec263778 100644 --- a/crates/disktree-app/src/views.rs +++ b/crates/disktree-app/src/views.rs @@ -6,6 +6,7 @@ use disktree_core::classify::Category; use disktree_core::insights::{Candidate, Finding, STALE_DAYS}; +use disktree_core::path_style::PathStyle; use disktree_core::removal::{Plan, RemovalMode, Target}; use disktree_core::size::human_bytes; use disktree_core::tree::Metric; @@ -341,7 +342,7 @@ fn delete_dialog( let title = match plan.targets.as_slice() { [only] => format!( "Delete \u{201c}{}\u{201d} permanently?", - short_name(&only.path) + short_name(&only.path, app.path_style()) ), targets => format!("Delete {} items permanently?", targets.len()), }; @@ -625,18 +626,19 @@ fn trail(app: &Disktree, theme: &Theme, cx: &Context<'_, Disktree>) -> Div { for (index, (label, step)) in steps.into_iter().enumerate() { if hidden.contains(&index) { if index == hidden.start { - row = row.child(separator_glyph(theme)).child( - div() - .px(space::XS) - .text_color(theme.secondary.opacity(0.6)) - .child("…"), - ); + row = + row.child(separator_glyph(theme, app.path_style())).child( + div() + .px(space::XS) + .text_color(theme.secondary.opacity(0.6)) + .child("…"), + ); } continue; } // The root is its own separator: "/" then "home", not "/ / home". if index > 1 { - row = row.child(separator_glyph(theme)); + row = row.child(separator_glyph(theme, app.path_style())); } let id = ElementId::Name(SharedString::from(format!("crumb-{index}"))); let crumb = match step { @@ -668,13 +670,13 @@ fn trail(app: &Disktree, theme: &Theme, cx: &Context<'_, Disktree>) -> Div { /// Steps a trail shows before it folds its middle into an ellipsis. const TRAIL_STEPS: usize = 7; -/// The platform's path separator, so the trail reads like the paths shown -/// elsewhere: `/` here, `\` on Windows. -fn separator_glyph(theme: &Theme) -> Div { +/// The scanned machine's path separator, so the trail reads like the paths +/// shown elsewhere: `/` here and on SSH hosts, `\` on Windows. +fn separator_glyph(theme: &Theme, style: PathStyle) -> Div { div() .text_color(theme.secondary.opacity(0.5)) .text_size(text::BODY) - .child(std::path::MAIN_SEPARATOR_STR) + .child(style.separator().to_string()) } /// A crumb in the tree. Its label goes there (the current one opens the @@ -772,7 +774,7 @@ fn sibling_menu( let largest = rows.first().map_or(1, |row| row.value.max(1)); let parent_name = app.node_at(parent).map_or_else(String::new, |node| { if parent.is_empty() { - crate::marks::display_path(&app.root_path, app.home.as_deref()) + app.display_path(&app.root_path) } else { node.name.to_string() } @@ -1526,47 +1528,48 @@ fn selection_section( let is_current_root = target == app.crumbs; let highlight = palette::highlight(theme); - let identity = div() - .flex() - .flex_col() - .gap(space::XS) - .min_w_0() - .child( - div() - .flex() - .flex_row() - .items_center() - .gap(space::SM) - .min_w_0() - .child( - div() - .flex_shrink_0() - .w(space::XS) - .h(text::HEADING) - .bg(palette::category_accent(theme, node.category)), - ) - .child( - div() - .text_size(text::HEADING) - .font_weight(FontWeight::BOLD) - .text_color(theme.bright) - .whitespace_nowrap() - .overflow_hidden() - .text_ellipsis() - .child(node.name.to_string()), - ), - ) - .child( - div() - .text_size(text::CAPTION) - .text_color(theme.secondary) - .whitespace_nowrap() - .overflow_hidden() - .text_ellipsis() - .child(path.as_deref().map_or_else(String::new, |path| { - crate::marks::display_path(path, app.home.as_deref()) - })), - ); + let identity = + div() + .flex() + .flex_col() + .gap(space::XS) + .min_w_0() + .child( + div() + .flex() + .flex_row() + .items_center() + .gap(space::SM) + .min_w_0() + .child( + div() + .flex_shrink_0() + .w(space::XS) + .h(text::HEADING) + .bg(palette::category_accent(theme, node.category)), + ) + .child( + div() + .text_size(text::HEADING) + .font_weight(FontWeight::BOLD) + .text_color(theme.bright) + .whitespace_nowrap() + .overflow_hidden() + .text_ellipsis() + .child(node.name.to_string()), + ), + ) + .child( + div() + .text_size(text::CAPTION) + .text_color(theme.secondary) + .whitespace_nowrap() + .overflow_hidden() + .text_ellipsis() + .child(path.as_deref().map_or_else(String::new, |path| { + app.display_path(path) + })), + ); let (number, unit) = match app.options.metric { Metric::Bytes => widgets::split_size(&human_bytes(node.bytes)), @@ -1696,7 +1699,7 @@ fn selection_section( let crumbs = target; let label = match &ancestor { Some(ancestor) if !marked => { - format!("Unmark {}", short_name(ancestor)) + format!("Unmark {}", short_name(ancestor, app.path_style())) } _ if marked => "Unmark".to_string(), _ => "Mark for removal".to_string(), @@ -1945,10 +1948,7 @@ fn marked_section( .whitespace_nowrap() .overflow_hidden() .text_ellipsis() - .child(crate::marks::display_path( - &item.path, - app.home.as_deref(), - )), + .child(app.display_path(&item.path)), ) .child( div() @@ -2557,18 +2557,11 @@ fn indexing_progress( || "Directory not reported yet".into(), |path| path.display().to_string(), ); - let mut parts = path - .map(|path| { - path.components() - .rev() - .take(3) - .map(|part| part.as_os_str().to_string_lossy().into_owned()) - .collect::>() - }) - .unwrap_or_default(); - parts.reverse(); - let short_path = if path.is_some_and(|path| path.components().count() > 3) { - format!("…/{}", parts.join("/")) + // Split by the scanned machine's rules: a Unix host's `a\b` is one name. + let names = path.map(|path| app.path_style().names(path)); + let names = names.as_deref().unwrap_or_default(); + let short_path = if names.len() > 3 { + format!("…/{}", names[names.len() - 3..].join("/")) } else { full_path.clone() }; @@ -2724,8 +2717,8 @@ fn find_field(app: &Disktree, theme: &Theme) -> Div { ) } -fn short_name(path: &std::path::Path) -> String { - path.file_name().map_or_else( +fn short_name(path: &std::path::Path, style: PathStyle) -> String { + style.file_name(path).map_or_else( || path.display().to_string(), |name| name.to_string_lossy().into_owned(), ) @@ -2740,8 +2733,8 @@ fn marks_ancestor( app.marks .items() .iter() - .filter(|item| item.path != path && path.starts_with(&item.path)) - .map(|item| short_name(&item.path)) + .filter(|item| app.path_style().is_below(path, &item.path)) + .map(|item| short_name(&item.path, app.path_style())) .next() } @@ -2851,7 +2844,7 @@ fn mark_row( cx: &Context<'_, Disktree>, ) -> Div { let root_value = app.tree().map_or(0, |tree| tree.bytes); - let path_text = crate::marks::display_path(&item.path, app.home.as_deref()); + let path_text = app.display_path(&item.path); let color = if blocked.is_some() || covered { theme.secondary } else { @@ -2887,7 +2880,7 @@ fn mark_row( div() .min_w_0() .text_color(color) - .child(short_name(&item.path)), + .child(short_name(&item.path, app.path_style())), ) .child( div() @@ -3069,7 +3062,7 @@ fn review_summary( .text_color(theme.warning) .child(format!( "{}: {}", - short_name(&item.path), + short_name(&item.path, app.path_style()), item.reason )), ); @@ -3305,7 +3298,7 @@ fn running(app: &Disktree, cx: &gpui_kit::Context<'_, Disktree>) -> Div { } else { theme.danger }) - .child(short_name(path)), + .child(short_name(path, app.path_style())), ) .children(outcome.as_ref().err().map(|error| { div() @@ -3397,7 +3390,11 @@ fn done(app: &Disktree, cx: &gpui_kit::Context<'_, Disktree>) -> Div { .flex_row() .gap(space::SM) .text_size(text::CAPTION) - .child(div().text_color(theme.danger).child(short_name(path))) + .child( + div() + .text_color(theme.danger) + .child(short_name(path, app.path_style())), + ) .child(div().text_color(theme.secondary).child( outcome.as_ref().err().cloned().unwrap_or_default(), )), @@ -3654,82 +3651,83 @@ fn node_card( let hidden = node.name.starts_with('.'); let covered = marks_ancestor(app, path.as_deref()); - let mut tip = div() - .flex() - .flex_col() - .gap(space::XS) - .child( - div() - .flex() - .flex_row() - .items_center() - .gap(space::XS) - .child( - gpui_omarchy::icon(if node.is_dir() { - gpui_omarchy::IconName::FolderOpen - } else { - gpui_omarchy::IconName::File - }) - .size(icon::SM) - .text_color(theme.secondary), - ) - .child( - div() - .min_w_0() - .font_weight(FontWeight::SEMIBOLD) - .text_color(theme.bright) - .child(node.name.to_string()), - ), - ) - .child( - div() - .text_size(text::CAPTION) - .text_color(theme.secondary) - .child(path.as_deref().map_or_else(String::new, |path| { - crate::marks::display_path(path, app.home.as_deref()) - })), - ) - .child( - div() - .flex() - .flex_row() - .items_center() - .gap(space::SM) - .child( - div() - .text_size(text::TITLE) - .font_weight(FontWeight::BOLD) - .text_color(theme.bright) - .child(human_bytes(node.bytes)), - ) - .child(widgets::glyph_bar( - node.bytes, - parent_value.max(1), - 10, - theme.secondary, - cx, - )) - .child( - div() - .text_size(text::CAPTION) - .text_color(theme.secondary) - .child(widgets::percent(node.bytes, parent_value)), - ), - ) - .child( - div() - .text_size(text::CAPTION) - .text_color(theme.secondary) - .child(format!( - "{} files · {} dirs · {} direct", - widgets::human_count(node.files), - widgets::human_count( - u64::from(node.dirs) - .saturating_sub(u64::from(node.is_dir())) + let mut tip = + div() + .flex() + .flex_col() + .gap(space::XS) + .child( + div() + .flex() + .flex_row() + .items_center() + .gap(space::XS) + .child( + gpui_omarchy::icon(if node.is_dir() { + gpui_omarchy::IconName::FolderOpen + } else { + gpui_omarchy::IconName::File + }) + .size(icon::SM) + .text_color(theme.secondary), + ) + .child( + div() + .min_w_0() + .font_weight(FontWeight::SEMIBOLD) + .text_color(theme.bright) + .child(node.name.to_string()), ), - human_bytes(node.own_bytes()) - )), - ); + ) + .child( + div() + .text_size(text::CAPTION) + .text_color(theme.secondary) + .child(path.as_deref().map_or_else(String::new, |path| { + app.display_path(path) + })), + ) + .child( + div() + .flex() + .flex_row() + .items_center() + .gap(space::SM) + .child( + div() + .text_size(text::TITLE) + .font_weight(FontWeight::BOLD) + .text_color(theme.bright) + .child(human_bytes(node.bytes)), + ) + .child(widgets::glyph_bar( + node.bytes, + parent_value.max(1), + 10, + theme.secondary, + cx, + )) + .child( + div() + .text_size(text::CAPTION) + .text_color(theme.secondary) + .child(widgets::percent(node.bytes, parent_value)), + ), + ) + .child( + div() + .text_size(text::CAPTION) + .text_color(theme.secondary) + .child(format!( + "{} files · {} dirs · {} direct", + widgets::human_count(node.files), + widgets::human_count( + u64::from(node.dirs) + .saturating_sub(u64::from(node.is_dir())) + ), + human_bytes(node.own_bytes()) + )), + ); let mut badges = div().flex().flex_row().gap(space::XS).flex_wrap(); if hidden { diff --git a/crates/disktree-core/src/lib.rs b/crates/disktree-core/src/lib.rs index 0f58c7ba..aa9e47ce 100644 --- a/crates/disktree-core/src/lib.rs +++ b/crates/disktree-core/src/lib.rs @@ -16,6 +16,7 @@ pub mod insights; mod macos; #[cfg(windows)] mod mft; +pub mod path_style; pub mod payload; pub mod remote; pub mod remote_progress; diff --git a/crates/disktree-core/src/path_style.rs b/crates/disktree-core/src/path_style.rs new file mode 100644 index 00000000..ed8b364e --- /dev/null +++ b/crates/disktree-core/src/path_style.rs @@ -0,0 +1,278 @@ +//! How paths on the scanned machine are joined, split and compared. +//! +//! A local scan follows this machine's rules, which is `std::path`. An SSH +//! host is always Unix: the remote command, the embedded scanners and the +//! delete script are POSIX shell and Linux tools. So remote paths follow +//! POSIX rules on every client. On a Windows client `std::path` would join +//! them with `\`, naming a different file on the host, and split a Unix name +//! such as `a\b` or `c:d` into parts the host never had. +//! +//! Remote paths only ever arrive as JSON strings, so the POSIX side works on +//! `str` and treats a path that is not UTF-8 as matching nothing. + +use std::borrow::Cow; +use std::ffi::OsStr; +use std::path::{Path, PathBuf}; + +/// The path rules of the machine a tree was scanned on. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub enum PathStyle { + /// This machine's own rules. + #[default] + Native, + /// `/`-separated, as on the Unix host an SSH session scans. + Posix, +} + +impl PathStyle { + /// The style for a local scan, or for a scan of an SSH host. + #[must_use] + pub const fn of(remote: bool) -> Self { + if remote { Self::Posix } else { Self::Native } + } + + /// The separator shown between names. + #[must_use] + pub const fn separator(self) -> char { + match self { + Self::Native => std::path::MAIN_SEPARATOR, + Self::Posix => '/', + } + } + + /// Whether `path` names a directory from the machine's root. + #[must_use] + pub fn is_absolute(self, path: &Path) -> bool { + match self { + Self::Native => path.is_absolute(), + Self::Posix => path.to_str().is_some_and(|s| s.starts_with('/')), + } + } + + /// `base` with one more name below it. + #[must_use] + pub fn join(self, base: &Path, name: &str) -> PathBuf { + let mut path = base.to_path_buf(); + self.push(&mut path, name); + path + } + + /// Append one name to `path`. + pub fn push(self, path: &mut PathBuf, name: &str) { + match self { + Self::Native => path.push(name), + Self::Posix => { + let mut joined = path.to_string_lossy().into_owned(); + if !joined.is_empty() && !joined.ends_with('/') { + joined.push('/'); + } + joined.push_str(name); + *path = PathBuf::from(joined); + } + } + } + + /// The last name in `path`, or `None` for the root. + #[must_use] + pub fn file_name(self, path: &Path) -> Option<&OsStr> { + match self { + Self::Native => path.file_name(), + Self::Posix => { + let trimmed = path.to_str()?.trim_end_matches('/'); + let name = trimmed.rsplit('/').next()?; + (!name.is_empty() && name != "..").then(|| OsStr::new(name)) + } + } + } + + /// `path` without its last name, or `None` for the root. + #[must_use] + pub fn parent(self, path: &Path) -> Option<&Path> { + match self { + Self::Native => path.parent(), + Self::Posix => { + let trimmed = path.to_str()?.trim_end_matches('/'); + if trimmed.is_empty() { + return None; + } + Some(Path::new(match trimmed.rfind('/') { + None => "", + Some(at) => match trimmed[..at].trim_end_matches('/') { + "" => "/", + head => head, + }, + })) + } + } + } + + /// `path`, its parent, its parent's parent, and so on to the root. + pub fn ancestors(self, path: &Path) -> impl Iterator { + std::iter::successors(Some(path), move |path| self.parent(path)) + } + + /// The names from `base` down to `path`, or `None` when `path` is not + /// `base` or below it. Names are compared whole, so a name containing a + /// separator of some other platform cannot be mistaken for two. + #[must_use] + pub fn relative<'a>( + self, + base: &Path, + path: &'a Path, + ) -> Option>> { + match self { + Self::Native => { + let rest = path.strip_prefix(base).ok()?; + Some( + rest.components() + .map(|part| part.as_os_str().to_string_lossy()) + .collect(), + ) + } + Self::Posix => posix_relative(base.to_str()?, path.to_str()?) + .map(|names| names.map(Cow::Borrowed).collect()), + } + } + + /// Every name in `path` below its root, in order. + #[must_use] + pub fn names(self, path: &Path) -> Vec> { + match self { + Self::Native => path + .components() + .filter(|part| matches!(part, std::path::Component::Normal(_))) + .map(|part| part.as_os_str().to_string_lossy()) + .collect(), + Self::Posix => path.to_str().map_or_else(Vec::new, |path| { + path.split('/') + .filter(|name| !name.is_empty() && *name != ".") + .map(Cow::Borrowed) + .collect() + }), + } + } + + /// Whether `path` is strictly below `base`. + #[must_use] + pub fn is_below(self, path: &Path, base: &Path) -> bool { + match self { + Self::Native => path != base && path.starts_with(base), + Self::Posix => match (base.to_str(), path.to_str()) { + (Some(base), Some(path)) => posix_relative(base, path) + .is_some_and(|mut names| names.next().is_some()), + _ => false, + }, + } + } +} + +fn posix_relative<'a>( + base: &str, + path: &'a str, +) -> Option> { + let rest = path.strip_prefix(base.trim_end_matches('/'))?; + // `/srv` must not claim `/srvx`; an empty base is a relative start. + if !base.is_empty() && !rest.is_empty() && !rest.starts_with('/') { + return None; + } + Some( + rest.split('/') + .filter(|name| !name.is_empty() && *name != "."), + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn names(style: PathStyle, base: &str, path: &str) -> Option> { + style + .relative(Path::new(base), Path::new(path)) + .map(|names| names.into_iter().map(Cow::into_owned).collect()) + } + + /// The host's paths are joined with `/` whatever this machine uses. + #[test] + fn posix_paths_join_with_a_slash_on_every_client() { + let posix = PathStyle::Posix; + let joined = posix.join(Path::new("/home/x"), "child"); + assert_eq!(joined.to_str(), Some("/home/x/child")); + let joined = posix.join(Path::new("/"), "srv"); + assert_eq!(joined.to_str(), Some("/srv")); + let joined = posix.join(&joined, r"back\slash"); + assert_eq!(joined.to_str(), Some(r"/srv/back\slash")); + assert_eq!(posix.join(Path::new(""), "a").to_str(), Some("a")); + } + + /// Names a Windows client would split on stay whole on a Unix host. + #[test] + fn posix_names_keep_backslashes_and_colons() { + let posix = PathStyle::Posix; + let path = Path::new(r"/srv/a\b/c:d"); + assert_eq!(posix.file_name(path), Some(OsStr::new("c:d"))); + assert_eq!(posix.parent(path), Some(Path::new(r"/srv/a\b"))); + assert_eq!( + names(posix, "/srv", r"/srv/a\b/c:d"), + Some(vec![r"a\b".into(), "c:d".into()]) + ); + assert!(posix.is_below(path, Path::new(r"/srv/a\b"))); + assert_eq!(posix.names(path), ["srv", r"a\b", "c:d"]); + assert!(!posix.is_below(path, Path::new("/srv/a"))); + } + + #[test] + fn posix_parents_end_at_the_root() { + let posix = PathStyle::Posix; + let chain: Vec<_> = posix + .ancestors(Path::new("/home/x/")) + .map(|path| path.to_str().unwrap()) + .collect(); + assert_eq!(chain, ["/home/x/", "/home", "/"]); + assert_eq!(posix.parent(Path::new("/")), None); + assert_eq!(posix.file_name(Path::new("/")), None); + assert_eq!(posix.parent(Path::new("name")), Some(Path::new(""))); + } + + #[test] + fn posix_relative_names_respect_name_boundaries() { + let posix = PathStyle::Posix; + assert_eq!(names(posix, "/srv", "/srv"), Some(vec![])); + assert_eq!( + names(posix, "/", "/srv/x"), + Some(vec!["srv".into(), "x".into()]) + ); + assert_eq!(names(posix, "/srv/", "/srv/x"), Some(vec!["x".into()])); + assert_eq!(names(posix, "/srv", "/srvx"), None); + assert_eq!(names(posix, "/srv", "/var"), None); + assert!(!posix.is_below(Path::new("/srv"), Path::new("/srv"))); + assert!(posix.is_below(Path::new("/srv"), Path::new("/"))); + } + + /// Only `/`-rooted paths are absolute on a Unix host; `C:\x` and `\x` + /// name nothing there. + #[test] + fn posix_absolute_means_slash_rooted() { + let posix = PathStyle::Posix; + assert!(posix.is_absolute(Path::new("/"))); + assert!(posix.is_absolute(Path::new("/home/someone"))); + assert!(!posix.is_absolute(Path::new("home/someone"))); + assert!(!posix.is_absolute(Path::new(""))); + assert!(!posix.is_absolute(Path::new(r"\home\someone"))); + assert!(!posix.is_absolute(Path::new(r"C:\Users"))); + assert_eq!(posix.separator(), '/'); + } + + /// Native is `std::path`, so local scans behave exactly as before. + #[test] + fn native_follows_this_machine() { + let native = PathStyle::Native; + let base = std::env::temp_dir(); + let joined = native.join(&base, "child"); + assert_eq!(joined, base.join("child")); + assert_eq!(native.parent(&joined), Some(base.as_path())); + assert_eq!(native.file_name(&joined), Some(OsStr::new("child"))); + assert!(native.is_below(&joined, &base)); + assert!(!native.is_below(&base, &base)); + assert_eq!(native.separator(), std::path::MAIN_SEPARATOR); + } +} diff --git a/crates/disktree-core/src/remote.rs b/crates/disktree-core/src/remote.rs index 27da007f..874d7d18 100644 --- a/crates/disktree-core/src/remote.rs +++ b/crates/disktree-core/src/remote.rs @@ -5,12 +5,13 @@ //! and stream without making a second serialized copy of the whole tree. use std::io::{self, BufRead, BufReader, Read as _, Write}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::{Child, Command, Stdio}; use std::sync::{Arc, Mutex, mpsc}; use serde::{Deserialize, Serialize}; +use crate::path_style::PathStyle; use crate::remote_progress::{INTERVAL, Progress, Shared, Stage, stage}; use crate::scan::ScanOptions; #[cfg(not(unix))] @@ -23,6 +24,25 @@ const VERSION: u32 = 1; const MAX_LINE: u64 = 1 << 20; const MAX_DIAGNOSTICS: usize = 16 << 10; +/// Whether `path` names a directory from the remote host's root. Hosts are +/// Unix, so this is POSIX on every client; see `path_style`. +#[must_use] +pub fn is_remote_absolute(path: &Path) -> bool { + PathStyle::Posix.is_absolute(path) +} + +/// The remote command, the scanners and the delete script are POSIX shell +/// and Linux tools, and remote paths are joined as POSIX paths. A Windows +/// host's `C:\` paths would be joined wrongly, so refuse it by name. +fn check_platform(platform: &str) -> io::Result<()> { + if platform == "windows" { + return Err(io::Error::other( + "SSH hosts must run Unix; Windows hosts are not supported", + )); + } + Ok(()) +} + // Non-interactive shells often omit ~/.local/bin. Do not require users to // edit the server's startup files after the documented user-local install. const REMOTE_COMMAND: &str = r#"if command -v disktree-scan >/dev/null 2>&1; then @@ -80,12 +100,16 @@ pub struct Roots { pub fn read_roots(input: &mut impl BufRead) -> io::Result { let roots: Roots = receive(input)?; + check_platform(&roots.platform)?; if roots.version != VERSION || roots .volumes .iter() - .any(|volume| !volume.point.is_absolute()) - || roots.home.as_ref().is_some_and(|home| !home.is_absolute()) + .any(|volume| !is_remote_absolute(&volume.point)) + || roots + .home + .as_ref() + .is_some_and(|home| !is_remote_absolute(home)) { return Err(io::Error::other("incompatible remote root discovery")); } @@ -312,7 +336,14 @@ fn read_snapshot_with_progress( shared: &Shared, ) -> io::Result { let header: Header = receive(input)?; - if header.version != VERSION || !header.root.is_absolute() { + check_platform(&header.platform)?; + if header.version != VERSION + || !is_remote_absolute(&header.root) + || [&header.home, &header.disk_root] + .into_iter() + .flatten() + .any(|path| !is_remote_absolute(path)) + { return Err(io::Error::other("incompatible remote helper protocol")); } let mut progress = Progress { @@ -344,8 +375,7 @@ fn read_snapshot_with_progress( && (name.is_empty() || name == "." || name == ".." - || name.contains(['/', '\0']) - || (cfg!(windows) && name.contains(['\\', ':']))) + || name.contains(['/', '\0'])) { return Err(io::Error::other("invalid preview directory")); } @@ -383,11 +413,12 @@ fn read_snapshot_with_progress( let Record(name, kind, bytes, files, modified, read_error, count) = record; // Names must remain single components when joined to marked paths. + // Remote paths are joined as POSIX paths on every client, so `\` and + // `:` are ordinary characters in a name, as they are on the host. if name.is_empty() || name == "." || name == ".." || name.contains(['/', '\0']) - || (cfg!(windows) && name.contains(['\\', ':'])) { return Err(io::Error::other(format!( "remote name cannot be represented safely: {name:?}" @@ -577,7 +608,8 @@ impl RemoteScan { Self::spawn_with_archive(command, host, request, archive) } - #[cfg(test)] + // Only the Unix tests run a fake ssh, so Windows has no caller. + #[cfg(all(test, unix))] fn spawn_command( command: Command, host: &str, @@ -744,6 +776,7 @@ mod tests { } } + #[cfg(unix)] fn wait(scan: &RemoteJob) -> io::Result { wait_for(scan, std::time::Duration::from_secs(10)) } @@ -808,6 +841,8 @@ mod tests { } } + // The helper runs on this machine, which must be a Unix host. + #[cfg(unix)] #[test] fn discovery_is_not_a_legacy_scan_request() { let session = SessionRequest { @@ -831,6 +866,8 @@ mod tests { assert!(roots.identity.is_some()); } + // The helper runs on this machine, which must be a Unix host. + #[cfg(unix)] #[test] fn discovery_never_resolves_a_scan_path_and_rejects_unknown_versions() { let mut output = Vec::new(); @@ -1086,6 +1123,61 @@ mod tests { ); } + fn unix_stream(platform: &str) -> Vec { + let mut data = Vec::new(); + let header = Header { + version: VERSION, + root: PathBuf::from("/srv"), + home: Some(PathBuf::from("/home/someone")), + disk_root: Some(PathBuf::from("/")), + space: None, + platform: platform.into(), + identity: None, + }; + send(&mut data, &header).unwrap(); + for name in [r"a\b", "c:d"] { + send(&mut data, &Record(name.into(), 1, 5, 1, 0, false, 0)) + .unwrap(); + } + send(&mut data, &Record("root".into(), 0, 0, 0, 0, false, 2)).unwrap(); + send(&mut data, &"done").unwrap(); + data + } + + /// Every client reads a Unix host's names as the host wrote them: `\` + /// and `:` are ordinary characters there, and the joined path uses `/`. + #[test] + fn unix_names_with_backslashes_and_colons_reach_every_client() { + let snapshot = read_snapshot(&mut unix_stream("linux").as_slice()) + .expect("a Unix host's names are accepted"); + assert_eq!(snapshot.tree.files, 2); + let style = PathStyle::Posix; + for name in [r"a\b", "c:d"] { + assert!(snapshot.tree.child_named(name).is_some()); + let path = style.join(&snapshot.header.root, name); + assert_eq!(path.to_str().unwrap(), format!("/srv/{name}")); + } + } + + /// Remote paths are joined as POSIX paths, so a Windows host is refused + /// by name rather than half-read with the wrong separator. + #[test] + fn windows_hosts_and_non_posix_roots_are_refused() { + let error = read_snapshot(&mut unix_stream("windows").as_slice()) + .unwrap_err() + .to_string(); + assert!(error.contains("Windows hosts"), "{error}"); + for value in [ + serde_json::json!({"version":1,"home":null,"platform":"windows","identity":null,"volumes":[]}), + serde_json::json!({"version":1,"home":"C:\\Users\\x","platform":"linux","identity":null,"volumes":[]}), + serde_json::json!({"version":1,"home":null,"platform":"linux","identity":null,"volumes":[{"point":"C:\\","device":null,"space":null}]}), + ] { + let mut data = serde_json::to_vec(&value).unwrap(); + data.push(b'\n'); + assert!(read_roots(&mut data.as_slice()).is_err(), "{value}"); + } + } + #[test] fn rejects_ssh_options_and_shell_syntax() { for host in ["-oProxyCommand=oops", "dev; touch x", "dev\nother", ""] { @@ -1174,6 +1266,9 @@ mod tests { assert_eq!(shared.lock().unwrap().stage, Stage::Finalizing); } + // The live preview this checks comes from the Unix helper's streaming + // walk; on other systems `serve` sends a finished tree with no preview. + #[cfg(unix)] #[test] fn desktop_dedup_counts_links_once_without_requiring_external_names() { let dir = tempfile::tempdir().unwrap(); @@ -1224,6 +1319,8 @@ mod tests { } } + // The helper runs on this machine, which must be a Unix host. + #[cfg(unix)] #[test] fn desktop_dedup_accepts_more_than_the_old_pending_link_limit() { let mut data = Vec::new(); @@ -1271,6 +1368,8 @@ mod tests { assert_eq!(snapshot.tree.bytes, count); } + // The helper runs on this machine, which must be a Unix host. + #[cfg(unix)] #[test] fn protocol_roundtrip_and_truncation() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/disktree-core/src/tree.rs b/crates/disktree-core/src/tree.rs index 4bbeda44..eabb221b 100644 --- a/crates/disktree-core/src/tree.rs +++ b/crates/disktree-core/src/tree.rs @@ -9,6 +9,7 @@ use rayon::prelude::*; use rustc_hash::FxHashSet; use crate::classify::{Category, Reclaim}; +use crate::path_style::PathStyle; /// What a node represents on disk. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -412,14 +413,20 @@ fn aggregate_at( }); } -/// Absolute path of the node at `crumbs` beneath a scanned root. -pub fn path_of(root_path: &Path, root: &Node, crumbs: &[usize]) -> PathBuf { +/// Absolute path of the node at `crumbs` beneath a scanned root, joined by +/// the rules of the machine that was scanned. +pub fn path_of( + root_path: &Path, + root: &Node, + crumbs: &[usize], + style: PathStyle, +) -> PathBuf { let mut path = root_path.to_path_buf(); let mut node = root; for &index in crumbs { match node.children.get(index) { Some(child) => { - path.push(&*child.name); + style.push(&mut path, &child.name); node = child; } None => break, @@ -552,8 +559,13 @@ mod tests { nested.children.push(leaf("deep", 1)); root.children.push(nested); - let path = path_of(Path::new("/home/tobi"), &root, &[0, 0]); + let path = + path_of(Path::new("/home/tobi"), &root, &[0, 0], PathStyle::Native); assert_eq!(path, PathBuf::from("/home/tobi/child/deep")); + // A Unix host's path is joined with `/` on every client. + let path = + path_of(Path::new("/home/tobi"), &root, &[0, 0], PathStyle::Posix); + assert_eq!(path.to_str(), Some("/home/tobi/child/deep")); } #[test] diff --git a/crates/disktree-scan/tests/stdio.rs b/crates/disktree-scan/tests/stdio.rs index b29f4c7c..005a9965 100644 --- a/crates/disktree-scan/tests/stdio.rs +++ b/crates/disktree-scan/tests/stdio.rs @@ -2,9 +2,13 @@ use std::io::{BufRead as _, BufReader, Write as _}; use std::process::{Command, Stdio}; +#[cfg(unix)] use disktree_core::remote::{Request, read_snapshot}; use disktree_core::scan::ScanOptions; +// The helper runs here, so this machine is the host, and clients only +// accept Unix hosts. +#[cfg(unix)] #[test] fn executable_returns_a_read_only_snapshot() { let dir = tempfile::tempdir().unwrap(); @@ -101,6 +105,9 @@ fn disconnect_stops_a_helper_even_when_its_output_is_blocked() { wait_bounded(&mut helper.0); } +// The helper runs here, so this machine is the host, and clients only +// accept Unix hosts. +#[cfg(unix)] #[test] fn a_live_supervised_client_receives_a_complete_snapshot() { let dir = tempfile::tempdir().unwrap(); @@ -239,6 +246,9 @@ fn assert_bounded_stream_memory(root: &std::path::Path) { drop(input); } +// The helper runs here, so this machine is the host, and clients only +// accept Unix hosts. +#[cfg(unix)] #[test] fn executable_discovers_roots_without_scanning_a_path() { let mut helper = supervised_helper();