From 28530a476a26a16b20140632c38cfd9532be418f Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 7 Sep 2026 22:55:01 +0800 Subject: [PATCH] fix(git): key a repository by one spelling of its root (#796) --- crates/tty7-core/src/core/git/diff.rs | 2 +- crates/tty7-core/src/core/git/mod.rs | 130 ++++++- crates/tty7-core/src/core/git/status.rs | 6 +- crates/tty7-core/src/core/mod.rs | 1 + crates/tty7-core/src/core/path_spelling.rs | 409 +++++++++++++++++++++ crates/tty7-core/src/core/worktree.rs | 12 +- crates/tty7-core/src/host/local.rs | 15 +- src/terminal/git_status.rs | 240 +++++++++++- src/ui/path_display.rs | 57 +-- src/ui/scm/panel.rs | 8 +- 10 files changed, 801 insertions(+), 79 deletions(-) create mode 100644 crates/tty7-core/src/core/path_spelling.rs diff --git a/crates/tty7-core/src/core/git/diff.rs b/crates/tty7-core/src/core/git/diff.rs index cb27a3a3..552cfa90 100644 --- a/crates/tty7-core/src/core/git/diff.rs +++ b/crates/tty7-core/src/core/git/diff.rs @@ -428,7 +428,7 @@ pub fn probe_diff(host: &dyn Host, root: &Path, req: &DiffRequest<'_>) -> Option return None; } let toplevel = git::git(host, root, &["rev-parse", "--show-toplevel"])?; - let toplevel = PathBuf::from(toplevel.trim_end_matches(['\n', '\r'])); + let toplevel = git::git_path(host, toplevel.trim_end_matches(['\n', '\r'])); let branch = git::branch_name(host, root)?; let argv = req.args(); diff --git a/crates/tty7-core/src/core/git/mod.rs b/crates/tty7-core/src/core/git/mod.rs index 73536ea1..07314e7e 100644 --- a/crates/tty7-core/src/core/git/mod.rs +++ b/crates/tty7-core/src/core/git/mod.rs @@ -48,8 +48,8 @@ pub fn probe(host: &dyn Host, cwd: &Path) -> Option { ], )?; let mut lines = paths.lines().map(|l| l.trim_end_matches(['\n', '\r'])); - let root = PathBuf::from(lines.next()?); - let home = repo_home(&root, lines.next(), lines.next()); + let root = git_path(host, lines.next()?); + let home = repo_home(host, &root, lines.next(), lines.next()); let branch = branch_name(host, cwd)?; Some(RepoSnapshot { home, @@ -59,18 +59,43 @@ pub fn probe(host: &dyn Host, cwd: &Path) -> Option { }) } -pub(crate) fn repo_home(root: &Path, git_dir: Option<&str>, common_dir: Option<&str>) -> PathBuf { +/// A path `git` just printed, in the spelling the rest of tty7 keys by. +/// +/// Git for Windows is MSYS2 and answers `rev-parse` with `C:/Users/…` whatever +/// shell asked it. `Path` forgives that much on its own, but the same root also +/// has to compare equal to one that came past `fs::canonicalize` — which spells +/// it `\\?\C:\Users\…`, a different prefix component and so a different key. +/// One spelling at the boundary, rather than a normalisation remembered at each +/// of the places these roots are later compared. See +/// [`crate::core::path_spelling`]. +/// +/// Asked of `host`, not of `cfg!(windows)`: the same probes run against a +/// remote box, whose `/home/u/src` is native over there and goes straight +/// back over the wire as the cwd of the next `git`. A Windows client +/// re-spelling it would ask a Linux server about `\home\u\src`. +pub(crate) fn git_path(host: &dyn Host, printed: &str) -> PathBuf { + crate::core::path_spelling::spelling_on_buf(host.id(), printed) +} + +pub(crate) fn repo_home( + host: &dyn Host, + root: &Path, + git_dir: Option<&str>, + common_dir: Option<&str>, +) -> PathBuf { let (Some(git_dir), Some(common)) = (git_dir, common_dir) else { return root.to_path_buf(); }; if git_dir == common { return root.to_path_buf(); } - let common = Path::new(common); - match (common.file_name(), common.parent()) { - (Some(name), Some(parent)) if name == ".git" => parent.to_path_buf(), - _ => common.to_path_buf(), + let common = git_path(host, common); + if common.file_name().is_some_and(|name| name == ".git") + && let Some(parent) = common.parent() + { + return parent.to_path_buf(); } + common } pub fn branch_name(host: &dyn Host, cwd: &Path) -> Option { @@ -659,24 +684,105 @@ mod tests { } #[test] fn repo_home_resolves_worktree_layouts() { + let host = h(); + let host = &*host; let root = Path::new("/repo/.wt/feat"); assert_eq!( - repo_home(Path::new("/repo"), Some("/repo/.git"), Some("/repo/.git")), + repo_home( + host, + Path::new("/repo"), + Some("/repo/.git"), + Some("/repo/.git") + ), PathBuf::from("/repo") ); assert_eq!( - repo_home(root, Some("/repo/.git/worktrees/feat"), Some("/repo/.git")), + repo_home( + host, + root, + Some("/repo/.git/worktrees/feat"), + Some("/repo/.git") + ), PathBuf::from("/repo") ); assert_eq!( - repo_home(root, Some("/bare.git/worktrees/feat"), Some("/bare.git")), + repo_home( + host, + root, + Some("/bare.git/worktrees/feat"), + Some("/bare.git") + ), PathBuf::from("/bare.git") ); assert_eq!( - repo_home(root, Some("/repo/.git"), None), + repo_home(host, root, Some("/repo/.git"), None), root.to_path_buf() ); - assert_eq!(repo_home(root, None, None), root.to_path_buf()); + assert_eq!(repo_home(host, root, None, None), root.to_path_buf()); + } + + /// The root every git probe answers with is the directory the *OS* names, + /// compared as a plain `PathBuf` — which is how every consumer compares + /// it. + /// + /// Not gated to any platform, deliberately. Git for Windows is MSYS2 and + /// prints `C:/Users/…`; `Host::canonicalize` used to answer `\\?\C:\Users\ + /// …`; those are different `Prefix` components, so the two never matched + /// and every SCM cache keyed by one missed the other. Nothing compared + /// them on Windows, which is exactly why nobody noticed — a `#[cfg(unix)]` + /// on this would put it straight back. + #[test] + fn a_probed_root_is_the_same_path_the_host_canonicalizes_to() { + let host = h(); + let dir = std::env::temp_dir().join(format!("tty7-root-spelling-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + + let made = git(&*host, &dir, &["init", "--quiet"]).is_some(); + if !made { + let _ = std::fs::remove_dir_all(&dir); + return; // no git on this machine + } + assert!(super::test_support::pin_repo_config(&dir)); + std::fs::write(dir.join("a.txt"), "one\n").unwrap(); + let mut commit = super::test_support::PINS.to_vec(); + commit.extend_from_slice(&["commit", "--quiet", "-m", "base"]); + assert!(git(&*host, &dir, &["add", "-A"]).is_some()); + assert!(git(&*host, &dir, &commit).is_some()); + + // The one directory, under the two names this process can learn it by: + // what the OS handed back, and what resolving it answers. + let canonical = host.canonicalize(&dir).expect("the scratch dir resolves"); + + let snap = probe(&*host, &dir).expect("a repository was just created here"); + assert_eq!( + snap.root, canonical, + "the probed root and the resolved directory are one key" + ); + assert_eq!(snap.home, canonical, "and so is a non-worktree's home"); + + // Asking from the resolved spelling has to reach the same answer, or + // a pane whose cwd arrived that way lands in a second repository. + let from_canonical = + probe(&*host, &canonical).expect("the same repository, asked from its other name"); + assert_eq!(from_canonical.root, snap.root); + + // The other two probes answer the same question and must not disagree + // with it: `probe_status` keys `ScmData`, `probe_diff` keys the diff + // overlay, and a disagreement between any two of them is a re-probe + // that never settles. + let status = match super::status::probe_status(&*host, &dir) { + super::status::StatusProbe::Status(status) => *status, + other => panic!("expected a repository, got {other:?}"), + }; + assert_eq!(status.root, snap.root, "probe_status agrees with probe"); + assert_eq!(status.home, snap.home); + + let diff = super::diff::probe_diff(&*host, &dir, &Default::default()) + .expect("an empty repository still has a diff"); + assert_eq!(diff.root, snap.root, "probe_diff agrees with probe"); + + let _ = std::fs::remove_dir_all(&dir); } } diff --git a/crates/tty7-core/src/core/git/status.rs b/crates/tty7-core/src/core/git/status.rs index 20a1b4c4..7ab8a5b3 100644 --- a/crates/tty7-core/src/core/git/status.rs +++ b/crates/tty7-core/src/core/git/status.rs @@ -845,12 +845,12 @@ pub fn probe_status(host: &dyn Host, cwd: &Path) -> StatusProbe { } let paths = String::from_utf8_lossy(&out.stdout).into_owned(); let mut lines = paths.lines().map(|l| l.trim_end_matches(['\n', '\r'])); - let Some(root) = lines.next().map(PathBuf::from) else { + let Some(root) = lines.next().map(|l| super::git_path(host, l)) else { return StatusProbe::Unreachable; }; let git_dir = lines.next(); - let home = super::repo_home(&root, git_dir, lines.next()); - let Some(git_dir) = git_dir.map(PathBuf::from) else { + let home = super::repo_home(host, &root, git_dir, lines.next()); + let Some(git_dir) = git_dir.map(|l| super::git_path(host, l)) else { return StatusProbe::Unreachable; }; diff --git a/crates/tty7-core/src/core/mod.rs b/crates/tty7-core/src/core/mod.rs index eb5ba282..51853442 100644 --- a/crates/tty7-core/src/core/mod.rs +++ b/crates/tty7-core/src/core/mod.rs @@ -12,6 +12,7 @@ pub mod kitty_graphics; pub mod logfile; pub mod machine; pub mod osc; +pub mod path_spelling; pub mod proc; pub mod session; pub mod shells; diff --git a/crates/tty7-core/src/core/path_spelling.rs b/crates/tty7-core/src/core/path_spelling.rs new file mode 100644 index 00000000..5df26d44 --- /dev/null +++ b/crates/tty7-core/src/core/path_spelling.rs @@ -0,0 +1,409 @@ +//! The one spelling tty7 stores a path on **this** machine in. +//! +//! The same directory reaches this process under three names on Windows and +//! only two of them compare equal: +//! +//! - `C:\Users\x\repo` — what the OS, a shell and a pane's cwd all say; +//! - `C:/Users/x/repo` — what Git for Windows says, whatever shell asked it: +//! `rev-parse --show-toplevel` and `--git-common-dir` are MSYS2 paths and +//! always come back with forward slashes; +//! - `\\?\C:\Users\x\repo` — what [`std::fs::canonicalize`] says, because Rust +//! asks the OS for the extended-length form. +//! +//! `Path` on Windows already forgives the first two of each other: it compares, +//! hashes and prefix-matches by *component*, and both `/` and `\` end a +//! component, so a drive letter's case is folded on the way past too. What it +//! does not forgive is the third. `\\?\C:` parses as [`Prefix::VerbatimDisk`] +//! and `C:` as [`Prefix::Disk`], those are different components, and so +//! `\\?\C:\Users\x\repo != C:/Users/x/repo` — by equality, by hash, and by +//! `starts_with`. Every cache in the SCM layer is keyed by exactly that +//! comparison, so a root that came in past `canonicalize` and a root that came +//! out of `git` name the same repository and share nothing. +//! +//! [`local_spelling`] is where that is settled, once, at the boundary a path +//! is *created* at rather than at each of the places it is later compared. +//! The spelling it lands on is the plain one — native separators, no +//! extended-length prefix — because that is the one every other consumer +//! wants: the Win32 shell's `ParseDisplayName` rejects both a mixed-separator +//! path and a `\\?\` one, `git` takes either on its command line, and it is +//! the only one of the three a person would recognise in a tooltip. +//! +//! Off Windows all three collapse: `/` is the separator, there is no +//! extended-length form, and both functions here are the identity. +//! +//! [`Prefix::VerbatimDisk`]: std::path::Prefix::VerbatimDisk +//! [`Prefix::Disk`]: std::path::Prefix::Disk + +use std::borrow::Cow; +use std::path::Path; + +/// Re-spells a path on **this** machine with the separators this OS expects. +/// +/// On Windows the shell's `IShellFolder::ParseDisplayName` bails out with +/// `E_INVALIDARG` on a mixed-separator path — a forward-slash prefix joined +/// with backslash entries. The forward slashes get in from two routes: the +/// shell's PWD (OSC 7 from Git Bash / MSYS bash reports `/`, and that string +/// survives `Path::ancestors()` when the file tree walks up to find `.git`), +/// and `git rev-parse --show-toplevel` from Git for Windows (MSYS2), which +/// always prints `/` regardless of the calling shell. +/// +/// **Only for paths on the machine this window runs on.** A remote host's +/// `/home/u/src` is already native over there; re-spelling it would put a +/// path on the clipboard that names nothing on either machine. +/// +/// The rewrite runs on the path's own UTF-16 code units, not on a +/// `to_string_lossy` copy of them. A Windows filename may hold unpaired +/// surrogates, which `to_string_lossy` turns into `U+FFFD` — the returned +/// path would then silently name a *different* file. `/` and `\` are ASCII, +/// so a code unit equal to one of them is that character and never half of a +/// surrogate pair, which is what makes the swap safe to do one unit at a time. +#[cfg(windows)] +pub fn native_separators(path: &Path) -> Cow<'_, Path> { + use std::ffi::OsString; + use std::os::windows::ffi::{OsStrExt, OsStringExt}; + use std::path::PathBuf; + + let os = path.as_os_str(); + // Nothing to fix — including every UNC (`\\wsl$\…`, `\\?\…`) and + // already-native path — hands the caller's own path straight back. + if !os.encode_wide().any(|unit| unit == SLASH) { + return Cow::Borrowed(path); + } + let wide: Vec = os + .encode_wide() + .map(|unit| if unit == SLASH { BACKSLASH } else { unit }) + .collect(); + Cow::Owned(PathBuf::from(OsString::from_wide(&wide))) +} + +/// Off Windows the OS separator is already `/`, and a backslash in a path is +/// an ordinary filename character — there is nothing to re-spell. +#[cfg(not(windows))] +pub fn native_separators(path: &Path) -> Cow<'_, Path> { + Cow::Borrowed(path) +} + +#[cfg(windows)] +const SLASH: u16 = b'/' as u16; +#[cfg(windows)] +const BACKSLASH: u16 = b'\\' as u16; + +/// The spelling tty7 stores a local path in, so that two of them naming one +/// directory are one key. +/// +/// Native separators (see [`native_separators`]) *and* no extended-length +/// prefix: `\\?\C:\x` becomes `C:\x` and `\\?\UNC\srv\share` becomes +/// `\\srv\share`, which is the same path as far as every Win32 API is +/// concerned and the only spelling `Path` will compare equal to the one a +/// shell, a pane cwd or `git` reports. +/// +/// What it deliberately does **not** do: +/// +/// - **fold case.** `Path` already folds the drive letter, which is where +/// Windows case instability actually lives; folding the rest would make two +/// genuinely different names on a case-sensitive volume — or on a remote +/// host, whose paths also pass through here unchanged off Windows — collide. +/// - **trim a trailing separator.** `Path` already ignores one: `C:\x\` and +/// `C:\x` are equal, hash alike and prefix-match each other. +/// - **touch the disk.** This is a re-spelling, not a resolution: a junction, +/// a `subst` drive or an 8.3 short name is left exactly as it arrived. +/// `git` has already resolved its own answer, and a caller that wants the +/// real path calls `Host::canonicalize`, which now lands here on its way +/// out. +/// - **rewrite anything but a drive or UNC verbatim path.** `\\?\pipe\…` and +/// the other device namespaces have no plain form to fall back to. +#[cfg(windows)] +pub fn local_spelling(path: &Path) -> Cow<'_, Path> { + use std::ffi::OsString; + use std::os::windows::ffi::{OsStrExt, OsStringExt}; + use std::path::PathBuf; + + let native = native_separators(path); + let wide: Vec = native.as_os_str().encode_wide().collect(); + let Some(bare) = strip_extended_length(&wide) else { + return native; + }; + Cow::Owned(PathBuf::from(OsString::from_wide(&bare))) +} + +/// Off Windows there is no second spelling to fold into the first. +#[cfg(not(windows))] +pub fn local_spelling(path: &Path) -> Cow<'_, Path> { + Cow::Borrowed(path) +} + +/// [`local_spelling`], for a caller that is building the path anyway and has +/// nothing to hand back borrowed. +pub fn local_spelling_buf(path: impl AsRef) -> std::path::PathBuf { + local_spelling(path.as_ref()).into_owned() +} + +/// The spelling a path that lives on `host` is stored in. +/// +/// [`local_spelling`] answers for the machine this process runs on, and every +/// caller that keys a repository by its root has to ask this one instead: the +/// same caches, the same `git` probes and the same SCM panel serve a pane on +/// another machine, and `/home/u/src` from a Linux box is already native over +/// there. Re-spelling it here would send `\home\u\src` back over the +/// wire — `Host::git` puts the path on the far side's command line verbatim — +/// and name nothing on either machine. +/// +/// A remote host is left exactly as it arrived, which is what this tree did +/// everywhere before the local rule existed. Path syntax is a property of the +/// machine the path is *on*, not of the one asking. +pub fn spelling_on(host: crate::host::HostId, path: &Path) -> Cow<'_, Path> { + match host.is_local() { + true => local_spelling(path), + false => Cow::Borrowed(path), + } +} + +/// [`spelling_on`], for a caller with nothing to hand back borrowed. +pub fn spelling_on_buf(host: crate::host::HostId, path: impl AsRef) -> std::path::PathBuf { + spelling_on(host, path.as_ref()).into_owned() +} + +/// The plain form of an extended-length path, or `None` when there is not one. +/// +/// Split out so the rule is testable on literal UTF-16, which is the only way +/// to write the surrogate case down and the only way a non-Windows developer +/// ever sees either shape. +#[cfg(windows)] +fn strip_extended_length(wide: &[u16]) -> Option> { + const VERBATIM: [u16; 4] = [BACKSLASH, BACKSLASH, b'?' as u16, BACKSLASH]; + const UNC: [u16; 4] = [b'U' as u16, b'N' as u16, b'C' as u16, BACKSLASH]; + + let rest = wide.strip_prefix(&VERBATIM)?; + // `\\?\UNC\srv\share` → `\\srv\share`. The `\` that follows `UNC` is kept + // and one more put in front of it, which is the pair a plain UNC path + // opens with. Windows spells the segment `UNC` but accepts any case, so + // match it the way the OS would. + let head: Vec = rest + .iter() + .take(4) + .map(|u| u16::from(u8::try_from(*u).unwrap_or(0).to_ascii_uppercase())) + .collect(); + if head == UNC { + let mut plain = vec![BACKSLASH]; + plain.extend_from_slice(&rest[3..]); + return Some(plain); + } + // `\\?\C:\…` → `C:\…`, and `\\?\C:` on its own too. Anything else behind + // the prefix is a device namespace with no plain form — leave it whole. + let (drive, colon) = (*rest.first()?, *rest.get(1)?); + let drive_letter = u8::try_from(drive).is_ok_and(|b| b.is_ascii_alphabetic()); + if drive_letter && colon == b':' as u16 && rest.get(2).is_none_or(|u| *u == BACKSLASH) { + return Some(rest.to_vec()); + } + None +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_path_with_nothing_to_fix_is_handed_back_borrowed() { + // No allocation for the overwhelmingly common case: a path already + // spelled the way this machine spells one. That is every path in the + // app once the boundaries below have done their work, so the cost of + // asking again at a lookup is a scan and nothing else. + let native: &[&str] = match cfg!(windows) { + true => &["README.md", r"C:\code\repo", r"\\server\share\proj"], + false => &["README.md", "/code/repo"], + }; + for p in native { + let got = local_spelling(Path::new(p)); + assert_eq!(got.as_ref(), Path::new(p), "{p:?}"); + assert!(matches!(got, Cow::Borrowed(_)), "{p:?} should not allocate"); + assert!(matches!(native_separators(Path::new(p)), Cow::Borrowed(_))); + } + } + + /// The whole point: the three spellings of one directory become one key. + #[test] + fn every_spelling_of_one_directory_lands_on_the_same_key() { + let want = local_spelling_buf(Path::new(if cfg!(windows) { + r"C:\Users\x\repo" + } else { + "/home/x/repo" + })); + let spellings: &[&str] = if cfg!(windows) { + &[ + r"C:\Users\x\repo", + "C:/Users/x/repo", + r"\\?\C:\Users\x\repo", + // Mixed, which is what `root.join(rel)` produces once a + // forward-slash root has had a native component added to it. + r"C:/Users/x\repo", + ] + } else { + &["/home/x/repo"] + }; + for spelling in spellings { + assert_eq!( + local_spelling(Path::new(spelling)).as_ref(), + want.as_path(), + "{spelling:?}" + ); + } + } + + /// The local rule is asked of the machine the path is *on*. + /// + /// A pane, a git probe and the SCM panel all serve a remote workspace + /// with the same code, and the root they settle on goes back over the + /// wire as the cwd of the next `git` — `RemoteHost` sends + /// `to_string_lossy` of it, verbatim. A Windows client folding a Linux + /// box's `/home/u/src` would ask that box about `\home\u\src`. + /// + /// Ungated: on unix both arms are the identity anyway, and Windows is the + /// only client where getting this wrong is visible. + #[test] + fn a_path_on_another_machine_is_left_in_that_machines_spelling() { + use crate::host::HostId; + + let remote = HostId::from_connection_key("ssh-direct:me@box:22"); + for posix in ["/home/u/src", "/home/u/a b/c", "/"] { + let got = spelling_on(remote, Path::new(posix)); + assert_eq!(got.as_ref(), Path::new(posix), "{posix:?}"); + assert!(matches!(got, Cow::Borrowed(_)), "{posix:?}"); + assert_eq!(spelling_on_buf(remote, posix).to_string_lossy(), posix); + } + // A remote *Windows* box is left alone too: its spelling is its own + // business, and this client may not even have a notion of a drive. + let win = r"C:/Users/x/repo"; + assert_eq!(spelling_on_buf(remote, win).to_string_lossy(), win); + // This machine's own paths still go through the rule, which on + // Windows is what makes the two arms different answers at all. + if cfg!(windows) { + assert_eq!( + spelling_on_buf(HostId::LOCAL, win), + std::path::PathBuf::from(r"C:\Users\x\repo") + ); + } + } + + /// What `Path` already does for us, pinned so a later "improvement" here + /// cannot quietly start folding things it must not. These are the cases + /// the #791 gate blamed for the SCM divergence; they were never the cause. + #[test] + fn path_equality_already_forgives_case_slashes_and_a_trailing_separator() { + if !cfg!(windows) { + return; + } + let root = Path::new(r"C:\Users\x\repo"); + for same in [ + "C:/Users/x/repo", + r"c:\Users\x\repo", + r"C:\Users\x\repo\", + "C:/Users/x/repo/", + ] { + assert_eq!(Path::new(same), root, "{same:?}"); + assert_eq!(local_spelling(Path::new(same)).as_ref(), root, "{same:?}"); + } + // …and the one it does not, which is why this module exists. + assert_ne!(Path::new(r"\\?\C:\Users\x\repo"), root); + } + + #[cfg(windows)] + #[test] + fn a_unc_verbatim_path_falls_back_to_its_plain_form() { + assert_eq!( + local_spelling(Path::new(r"\\?\UNC\server\share\proj")).as_ref(), + Path::new(r"\\server\share\proj") + ); + // Lowercase `unc` is the same namespace to Windows. + assert_eq!( + local_spelling(Path::new(r"\\?\unc\server\share")).as_ref(), + Path::new(r"\\server\share") + ); + // A plain UNC path is already plain, and must keep its leading `\\`. + for p in [r"\\server\share\proj", r"\\wsl$\Ubuntu\home"] { + let got = local_spelling(Path::new(p)); + assert_eq!(got.as_ref(), Path::new(p), "{p:?}"); + assert!(matches!(got, Cow::Borrowed(_)), "{p:?} should not allocate"); + } + } + + #[cfg(windows)] + #[test] + fn a_device_namespace_has_no_plain_form_and_is_left_whole() { + // `\\?\pipe\…` and `\\.\…` are not filesystem paths with a drive to + // fall back to; rewriting either would name nothing. + for p in [r"\\?\pipe\tty7", r"\\.\PhysicalDrive0", r"\\?\Volume{0}\x"] { + assert_eq!(local_spelling(Path::new(p)).as_ref(), Path::new(p), "{p:?}"); + } + } + + #[cfg(windows)] + #[test] + fn a_bare_verbatim_drive_keeps_its_root() { + assert_eq!( + local_spelling(Path::new(r"\\?\C:\")).as_ref(), + Path::new(r"C:\") + ); + assert_eq!( + local_spelling(Path::new(r"\\?\C:")).as_ref(), + Path::new("C:") + ); + } + + #[cfg(windows)] + #[test] + fn a_non_ascii_component_survives_both_rewrites() { + assert_eq!( + local_spelling(Path::new(r"\\?\C:\Users\x\中文名\проект")).as_ref(), + Path::new(r"C:\Users\x\中文名\проект") + ); + assert_eq!( + local_spelling(Path::new("C:/Users/x/中文名/проект")).as_ref(), + Path::new(r"C:\Users\x\中文名\проект") + ); + } + + #[cfg(windows)] + #[test] + fn a_name_a_string_cannot_hold_is_kept() { + use std::ffi::OsString; + use std::os::windows::ffi::{OsStrExt, OsStringExt}; + use std::path::PathBuf; + + // `0xD800` is a lone high surrogate — legal in an NTFS name, and not + // representable in a Rust `str`. Going through `to_string_lossy` + // would swap it for `U+FFFD` and hand back a path naming a + // *different* file. Working on the UTF-16 units keeps the name. + let raw: Vec = r"\\?\C:\a" + .encode_utf16() + .chain([0xD800]) + .chain("/b".encode_utf16()) + .collect(); + let path = PathBuf::from(OsString::from_wide(&raw)); + let want: Vec = r"C:\a" + .encode_utf16() + .chain([0xD800]) + .chain(r"\b".encode_utf16()) + .collect(); + assert_eq!( + local_spelling(&path) + .as_os_str() + .encode_wide() + .collect::>(), + want + ); + // The round-trip this avoids really does destroy it. + assert!(path.to_string_lossy().contains('\u{FFFD}')); + } + + #[cfg(not(windows))] + #[test] + fn off_windows_both_are_the_identity() { + // A backslash in a Unix path is an ordinary filename character, and a + // remote host's paths pass through this same code on a Windows client. + for p in ["/home/u/tty7", r"C:\Users\dev", r"mixed/path\here"] { + let got = local_spelling(Path::new(p)); + assert_eq!(got.as_ref(), Path::new(p), "{p:?}"); + assert!(matches!(got, Cow::Borrowed(_))); + } + } +} diff --git a/crates/tty7-core/src/core/worktree.rs b/crates/tty7-core/src/core/worktree.rs index bc0fceed..55ec586c 100644 --- a/crates/tty7-core/src/core/worktree.rs +++ b/crates/tty7-core/src/core/worktree.rs @@ -1,6 +1,7 @@ use std::path::{Path, PathBuf}; use crate::core::codename::Names; +use crate::core::git::git_path; use crate::host::Host; #[derive(Debug)] @@ -63,14 +64,17 @@ pub fn managed(host: &dyn Host, cwd: &Path) -> Option { if !cwd.ancestors().any(|a| a.ends_with(&suffix)) { return None; } - let path = PathBuf::from(git(host, &cwd, &["rev-parse", "--show-toplevel"]).ok()?); + let path = git_path( + host, + &git(host, &cwd, &["rev-parse", "--show-toplevel"]).ok()?, + ); let main_root = git( host, &path, &["rev-parse", "--path-format=absolute", "--git-common-dir"], ) .ok() - .map(PathBuf::from)? + .map(|d| git_path(host, &d))? .parent()? .to_path_buf(); if !path.starts_with(managed_root(host, &main_root)) { @@ -111,14 +115,14 @@ pub fn remove(host: &dyn Host, wt: &ManagedWorktree, force: bool) -> Result<(), fn repo_dir(host: &dyn Host, cwd: &Path) -> Result<(PathBuf, PathBuf), String> { let repo_root = git(host, cwd, &["rev-parse", "--show-toplevel"]) .map_err(|_| "not inside a git repository".to_string())?; - let repo_root = PathBuf::from(repo_root); + let repo_root = git_path(host, &repo_root); let main_root = git( host, cwd, &["rev-parse", "--path-format=absolute", "--git-common-dir"], ) .ok() - .map(PathBuf::from) + .map(|d| git_path(host, &d)) .and_then(|d| d.parent().map(Path::to_path_buf)) .unwrap_or_else(|| repo_root.clone()); let dir = managed_root(host, &main_root); diff --git a/crates/tty7-core/src/host/local.rs b/crates/tty7-core/src/host/local.rs index 6288037e..627c8f9f 100644 --- a/crates/tty7-core/src/host/local.rs +++ b/crates/tty7-core/src/host/local.rs @@ -177,9 +177,22 @@ impl Host for LocalHost { fs::read(p) } + /// The real path behind `p`, in the spelling the rest of tty7 keys by. + /// + /// `fs::canonicalize` answers with the extended-length form on Windows — + /// `\\?\C:\Users\x\repo` — which is a different `Prefix` component from + /// the `C:\Users\x\repo` a shell, a pane cwd and `git` all report, and so + /// compares unequal, hashes differently and fails `starts_with` against + /// every one of them. `\\?\` is a Win32 API escape hatch rather than part + /// of the path's identity, so it comes off here, at the one boundary that + /// produces it. What the call is actually *for* — resolving a junction, a + /// `subst` drive, an 8.3 short name or a symlink — is untouched. See + /// [`crate::core::path_spelling`]. fn canonicalize(&self, p: &Path) -> io::Result { guard_off_ui(); - fs::canonicalize(p) + Ok(crate::core::path_spelling::local_spelling_buf( + fs::canonicalize(p)?, + )) } fn search( diff --git a/src/terminal/git_status.rs b/src/terminal/git_status.rs index 715249b3..a8fa0250 100644 --- a/src/terminal/git_status.rs +++ b/src/terminal/git_status.rs @@ -1,9 +1,30 @@ +use std::borrow::Cow; use std::path::{Path, PathBuf}; use std::time::{Duration, Instant}; pub use crate::core::git::{GitStatus, RepoSnapshot, probe}; use crate::ui::host_ops::{ByHost, HostId, InFlight}; +/// The spelling a directory is keyed by in here. +/// +/// This cache is where a repository gets its identity: `roots` maps a cwd to a +/// root, and `homes`, `status` and `last_probe` are then all keyed by that +/// root, which is in turn the key `ScmData` and the diff overlay use. The two +/// halves of a lookup arrive from different places — a cwd from the pane, a +/// root from `git`, and either one possibly past `fs::canonicalize` — so this +/// is the one place that has to insist they agree. Off Windows, and for every +/// path that already spells itself the OS's way, it is a borrow and nothing +/// else. See [`tty7_core::core::path_spelling`]. +/// +/// Keyed by the *pane's* host, not by this process. Every method here serves a +/// remote workspace too, whose `/home/u/src` is native over there and is handed +/// straight back to `Host::git` and to `ScmData`'s watcher — folding it +/// with Windows rules on a Windows client would ask a Linux box about +/// `\home\u\src`. A path from another machine is left exactly as it arrived. +fn key(host: HostId, path: &Path) -> Cow<'_, Path> { + tty7_core::core::path_spelling::spelling_on(host, path) +} + #[derive(Default)] pub struct GitStatusCache { roots: ByHost>, @@ -17,12 +38,12 @@ impl gpui::Global for GitStatusCache {} impl GitStatusCache { pub fn status_for(&self, host: HostId, cwd: &Path) -> Option { - let root = self.roots.get(host, cwd)?.as_ref()?; - self.status.get(host, root).cloned() + let root = self.roots.get(host, &*key(host, cwd))?.as_ref()?; + self.status.get(host, root.as_path()).cloned() } pub fn known_repo_for(&self, host: HostId, cwd: &Path) -> Option> { - let root = self.roots.get(host, cwd)?; + let root = self.roots.get(host, &*key(host, cwd))?; Some(root.as_ref().map(|root| { self.homes .get(host, root) @@ -38,7 +59,7 @@ impl GitStatusCache { /// a "which project is this" question wants. This answers with the root, /// which is the key everything git-shaped is stored under. pub fn repo_root_for(&self, host: HostId, cwd: &Path) -> Option<&Path> { - self.roots.get(host, cwd)?.as_deref() + self.roots.get(host, &*key(host, cwd))?.as_deref() } /// Forget a machine we have stopped talking to, so a reconnect starts from @@ -51,7 +72,7 @@ impl GitStatusCache { } pub fn begin_probe(&mut self, host: HostId, cwd: &Path) -> bool { - let key = (host, cwd.to_path_buf()); + let key = (host, key(host, cwd).into_owned()); if self.probes.begin(key.clone()) { true } else { @@ -66,22 +87,25 @@ impl GitStatusCache { cwd: &Path, min_interval: Duration, ) -> bool { + let cwd = key(host, cwd); if self.probes.is_pending(&(host, cwd.to_path_buf())) { return false; } - let key = self.throttle_key(host, cwd).to_path_buf(); + let throttle = self.throttle_key(host, &cwd).to_path_buf(); if self .last_probe - .get(host, key.as_path()) + .get(host, throttle.as_path()) .is_some_and(|at| at.elapsed() < min_interval) { return false; } - self.last_probe.insert(host, key, Instant::now()); - self.probes.begin((host, cwd.to_path_buf())); + self.last_probe.insert(host, throttle, Instant::now()); + self.probes.begin((host, cwd.into_owned())); true } + /// `cwd` is already in the cache's own spelling — every caller of this one + /// has been past [`key`]. fn throttle_key<'a>(&'a self, host: HostId, cwd: &'a Path) -> &'a Path { match self.roots.get(host, cwd) { Some(Some(root)) => root, @@ -122,7 +146,8 @@ impl GitStatusCache { branch: &str, counts: Option<(u32, u32)>, ) -> bool { - let Some(status) = self.status.get(host, root) else { + let root = key(host, root); + let Some(status) = self.status.get(host, &*root) else { return false; }; let (added, removed) = counts.unwrap_or((status.added, status.removed)); @@ -131,7 +156,7 @@ impl GitStatusCache { } self.status.insert( host, - root.to_path_buf(), + root.into_owned(), GitStatus { branch: branch.to_string(), added, @@ -147,12 +172,21 @@ impl GitStatusCache { cwd: &Path, snapshot: Option, ) -> bool { + // A snapshot arrives spelled by `git`, the cwd by whoever asked for + // the probe. Both land in the cache's own spelling or the root a + // status is filed under is not the root the next lookup asks for. + let cwd = key(host, cwd); + let snapshot = snapshot.map(|snap| RepoSnapshot { + root: key(host, &snap.root).into_owned(), + home: key(host, &snap.home).into_owned(), + ..snap + }); let rerun = !self.probes.finish(&(host, cwd.to_path_buf())); - let key = match &snapshot { + let throttle = match &snapshot { Some(snap) => snap.root.clone(), - None => self.throttle_key(host, cwd).to_path_buf(), + None => self.throttle_key(host, &cwd).to_path_buf(), }; - self.last_probe.insert(host, key, Instant::now()); + self.last_probe.insert(host, throttle, Instant::now()); match snapshot { Some(snap) => { let (added, removed) = snap.counts.unwrap_or_else(|| { @@ -465,6 +499,184 @@ mod tests { assert!(cache.begin_probe(L, cwd)); } + /// Every way one directory can be spelled on the way into this cache is + /// one key. + /// + /// Ungated on purpose. The spellings below are the ones Windows actually + /// produces — a pane says `C:\repo`, `git rev-parse` says `C:/repo`, + /// `fs::canonicalize` says `\\?\C:\repo` — and on Unix they collapse to + /// one, so this costs nothing there and is the whole test here. Gating it + /// to unix is what let the divergence live: the *only* platform that has + /// three spellings was the only one not running the comparison. + #[test] + fn one_directory_spelled_three_ways_is_one_repository() { + let mut cache = GitStatusCache::default(); + let (a, b, c) = match cfg!(windows) { + true => (r"C:\code\repo", "C:/code/repo", r"\\?\C:\code\repo"), + false => ("/code/repo", "/code/repo", "/code/repo"), + }; + let (a, b, c) = (Path::new(a), Path::new(b), Path::new(c)); + + // Probed under the resolved spelling, which is what a caller that went + // through `Host::canonicalize` has. + cache.finish_probe(L, c, Some(snap(c.to_str().unwrap(), "main", Some((9, 9))))); + + for spelling in [a, b, c] { + assert_eq!( + cache.repo_root_for(L, spelling), + Some(a), + "{spelling:?} names the repository the others do" + ); + assert_eq!( + cache.known_repo_for(L, spelling), + Some(Some(a.to_path_buf())), + "{spelling:?}" + ); + assert_eq!( + cache.status_for(L, spelling).unwrap().branch, + "main", + "{spelling:?}" + ); + } + } + + /// A repository on another machine keeps that machine's spelling. + /// + /// The rule above is a *local* one, and this cache serves a remote + /// workspace with the same four methods. The root it hands back is what + /// `Host::git` puts on the far side's command line — `wire_path` is + /// `to_string_lossy`, verbatim — and what `ScmData` opens the `.git` + /// watch on. Folding `/home/u/src` with this client's rules would ask a + /// Linux box about `\home\u\src`, which names nothing there. + /// + /// Ungated, like the one above and for the same reason: the assertion is + /// only ever interesting on Windows, so gating it away from Windows is + /// how it would stop holding. + #[test] + fn a_repository_on_another_machine_keeps_that_machines_spelling() { + let mut cache = GitStatusCache::default(); + let remote = HostId::from_connection_key("ssh-direct:me@box:22"); + let (cwd, root) = (Path::new("/home/u/src/crates/app"), "/home/u/src"); + + cache.finish_probe(remote, cwd, Some(snap(root, "main", Some((2, 1))))); + + assert_eq!( + cache.repo_root_for(remote, cwd).map(Path::to_string_lossy), + Some(root.into()), + "the far side is handed this string back unchanged" + ); + assert_eq!( + cache.known_repo_for(remote, cwd), + Some(Some(PathBuf::from(root))) + ); + assert_eq!(cache.status_for(remote, cwd).unwrap().branch, "main"); + // And a diff read filed under git's own answer still reaches it. + assert!(cache.note_diff_read(remote, Path::new(root), "moved-on", Some((0, 0)))); + assert_eq!(cache.status_for(remote, cwd).unwrap().branch, "moved-on"); + } + + /// The diff overlay's spin, in the cache underneath it. + /// + /// `install_diff_snapshot` hands the branch it just read back with the + /// root `git rev-parse` printed, while the status it is correcting was + /// filed under the root whoever probed had. When those two spellings miss + /// each other the correction is dropped, the overlay's next + /// `maybe_refresh` finds the same disagreement it just tried to settle, + /// and it re-reads the diff — `load=ready loading=true`, two `git` + /// processes a lap, for as long as the overlay is open. + #[test] + fn a_diff_read_settles_a_branch_it_learned_the_root_of_from_git() { + let mut cache = GitStatusCache::default(); + let (probed, from_git) = match cfg!(windows) { + true => (r"\\?\C:\code\repo", "C:/code/repo"), + false => ("/code/repo", "/code/repo"), + }; + cache.finish_probe( + L, + Path::new(probed), + Some(snap(probed, "a-branch-this-repo-has-left", Some((99, 99)))), + ); + + assert!( + cache.note_diff_read(L, Path::new(from_git), "main", Some((1, 0))), + "the correction has to land, or the overlay reprobes forever" + ); + let got = cache.status_for(L, Path::new(probed)).unwrap(); + assert_eq!(got.branch, "main"); + assert_eq!((got.added, got.removed), (1, 0)); + assert!( + !cache.note_diff_read(L, Path::new(from_git), "main", Some((1, 0))), + "and the second lap has nothing left to say — this is what ends it" + ); + } + + /// The same loop, end to end against a repository `git` actually created, + /// because the literals above only prove the rule and not that this is the + /// rule the real answers need. + /// + /// This is the shape the diff overlay runs every frame: a status filed + /// under the cwd a probe was asked about, then a diff read filed under the + /// root `git rev-parse` printed. No window and no gpui, so it runs + /// everywhere the test binary does. + #[test] + fn a_real_repository_files_its_probe_and_its_diff_under_one_root() { + use tty7_core::core::git::diff::{DiffRequest, probe_diff}; + + let host = tty7_core::host::local::LocalHost::new(); + let dir = std::env::temp_dir().join(format!("tty7-one-root-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let ok = host + .git(&dir, &["init", "--quiet"]) + .is_ok_and(|o| o.success()); + if !ok { + let _ = std::fs::remove_dir_all(&dir); + return; // no git on this machine + } + for cfg in [ + ["config", "user.email", "t@x"].as_slice(), + ["config", "user.name", "t"].as_slice(), + ] { + assert!(host.git(&dir, cfg).is_ok_and(|o| o.success())); + } + std::fs::write(dir.join("a.rs"), "fn main() {}\n").unwrap(); + assert!(host.git(&dir, &["add", "-A"]).is_ok_and(|o| o.success())); + assert!( + host.git(&dir, &["commit", "--quiet", "-m", "one"]) + .is_ok_and(|o| o.success()) + ); + std::fs::write(dir.join("a.rs"), "fn main() { /* edited */ }\n").unwrap(); + + // The cwd a pane reports can have been past `Host::canonicalize`; the + // root a diff carries never has been. + let cwd = host.canonicalize(&dir).expect("the scratch dir resolves"); + let mut cache = GitStatusCache::default(); + let snapshot = crate::core::git::probe(&*host, &cwd).expect("a repository is here"); + cache.finish_probe(L, &cwd, Some(snapshot)); + assert_eq!( + cache.repo_root_for(L, &cwd), + Some(cwd.as_path()), + "the probed root is the directory the cache was asked about" + ); + + let diff = probe_diff(&*host, &cwd, &DiffRequest::default()).expect("a diff is readable"); + assert_eq!(diff.root, cwd, "and the diff names that same directory"); + // A branch switched outside tty7 is what makes this correction the + // thing that ends the overlay's loop rather than a no-op: the read has + // to land the first time and have nothing to say the second. + assert!( + cache.note_diff_read(L, &diff.root, "moved-on", Some((0, 0))), + "a diff read filed under git's root must reach the probe's status" + ); + assert_eq!(cache.status_for(L, &cwd).unwrap().branch, "moved-on"); + assert!( + !cache.note_diff_read(L, &diff.root, "moved-on", Some((0, 0))), + "and the second lap says nothing — this is what ends the loop" + ); + + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn throttle_collapses_subdirectories_of_one_repo() { let mut cache = GitStatusCache::default(); diff --git a/src/ui/path_display.rs b/src/ui/path_display.rs index 51a319c8..5a76cf6b 100644 --- a/src/ui/path_display.rs +++ b/src/ui/path_display.rs @@ -69,56 +69,27 @@ fn normalized(s: &str) -> String { .to_ascii_lowercase() } -/// Re-spells a path on **this** machine with the separators this OS expects. +/// Re-spells a path on **this** machine with the separators this OS expects, +/// so the Win32 shell will take it. /// -/// On Windows the shell's `IShellFolder::ParseDisplayName` bails out with -/// `E_INVALIDARG` on a mixed-separator path — a forward-slash prefix joined -/// with backslash entries. The forward slashes get in from two routes: the -/// shell's PWD (OSC 7 from Git Bash / MSYS bash reports `/`, and that string -/// survives `Path::ancestors()` when the file tree walks up to find `.git`), -/// and `git rev-parse --show-toplevel` from Git for Windows (MSYS2), which -/// always prints `/` regardless of the calling shell. `reveal_path` swallows -/// that failure (it only logs), so handing it native separators is what makes -/// "open folder" actually open. +/// `IShellFolder::ParseDisplayName` bails out with `E_INVALIDARG` on a +/// mixed-separator path — a forward-slash prefix joined with backslash +/// entries — and `reveal_path` swallows that failure (it only logs), so +/// handing it native separators is what makes "open folder" actually open. +/// +/// The rule itself lives in [`tty7_core::core::path_spelling`], next to the +/// prefix rule the SCM caches need, because a path spelled two ways is one +/// problem and it must not have two answers in two crates. This is the +/// separators half on its own: a `\\?\` path is already something +/// `ParseDisplayName` will not take, and re-spelling one here would be a +/// silent change of subject rather than a fix. /// /// **Only for paths on the machine this window runs on.** A remote host's /// `/home/u/src` is already native over there; re-spelling it would put a /// path on the clipboard that names nothing on either machine. Every caller /// sits behind a locality check for that reason. -/// -/// The rewrite runs on the path's own UTF-16 code units, not on a -/// `to_string_lossy` copy of them. A Windows filename may hold unpaired -/// surrogates, which `to_string_lossy` turns into `U+FFFD` — the returned -/// path would then silently name a *different* file, and reveal would open -/// nothing without reporting why. `/` and `\` are ASCII, so a code unit -/// equal to one of them is that character and never half of a surrogate -/// pair, which is what makes the swap safe to do one unit at a time. -#[cfg(windows)] pub(crate) fn native_separators(path: &Path) -> Cow<'_, Path> { - use std::ffi::OsString; - use std::os::windows::ffi::{OsStrExt, OsStringExt}; - - const SLASH: u16 = b'/' as u16; - const BACKSLASH: u16 = b'\\' as u16; - - let os = path.as_os_str(); - // Nothing to fix — including every UNC (`\\wsl$\…`, `\\?\…`) and - // already-native path — hands the caller's own path straight back. - if !os.encode_wide().any(|unit| unit == SLASH) { - return Cow::Borrowed(path); - } - let wide: Vec = os - .encode_wide() - .map(|unit| if unit == SLASH { BACKSLASH } else { unit }) - .collect(); - Cow::Owned(PathBuf::from(OsString::from_wide(&wide))) -} - -/// Off Windows the OS separator is already `/`, and a backslash in a path is -/// an ordinary filename character — there is nothing to re-spell. -#[cfg(not(windows))] -pub(crate) fn native_separators(path: &Path) -> Cow<'_, Path> { - Cow::Borrowed(path) + tty7_core::core::path_spelling::native_separators(path) } /// Shortens `path` to start from `~` when it is (inside) `home` — the home diff --git a/src/ui/scm/panel.rs b/src/ui/scm/panel.rs index bf3dc850..fc591cdf 100644 --- a/src/ui/scm/panel.rs +++ b/src/ui/scm/panel.rs @@ -1255,12 +1255,18 @@ impl Tty7App { }, move |this, out, cx| { this.scm.root_lookups.remove(&key); + // In the spelling everything else keys by: Git for Windows + // answers `C:/Users/…` and this root is what the panel, the + // commit detail and `ScmData` all compare against a path the + // OS spelled. Only for a repository on this machine — a remote + // root is native over there and every write below runs `git` + // from it on that box. See `tty7_core::core::path_spelling`. let root = out .as_deref() .and_then(|s| s.lines().next()) .map(str::trim) .filter(|l| !l.is_empty()) - .map(PathBuf::from); + .map(|l| tty7_core::core::path_spelling::spelling_on_buf(id, l)); this.scm.roots.insert(key, (Instant::now(), root)); cx.notify(); },