From b47976864599a17a942ec218502e79585564f055 Mon Sep 17 00:00:00 2001 From: Tom Date: Wed, 11 Feb 2026 17:51:08 +0100 Subject: [PATCH] surf: fix patchspec handling for directory names using `[]` Fixes requesting a directory containing another directory with [brackets] in its name disable_pathspec_match(true) sets the GIT_DIFF_DISABLE_PATHSPEC_MATCH flag on libgit2's diff options. Normally, pathspecs are matched using fnmatch/glob pattern matching, where [ starts a character class, * and ? are wildcards, and \ is the escape character. With this flag, pathspecs are treated as literal strings with prefix matching for directories. So src/[something] matches exactly src/[something] and any files under it, rather than [something] being interpreted as a glob character class matching a single character from the set s, o, m, e, etc. Removing escaped_name is OK because the old escaped_name doubled backslashes(\ -> \\) specifically because pathspecs used glob/fnmatch interpretation, where \ is the escape character. The doubling made pathspec where \ is the escape character. The doubling made pathspec treat \\ as "match a literal \". With disable_pathspec_match(true), there's no glob interpretation at all, so \ is already treated literally. The doubling is no longer needed and would actually break things (pathspec would look for \\ instead of \). Regarding Windows, escaped_name has no impact on Windows behavior. On Windows, \ is the path separator and cannot appear in filenames, so escaped_name was effectively a no-op on Windows anyway. The existing test for backslash filenames (special/faux\path) is already gated with `#[cfg(not(windows))]` since such filenames only exist on Unix. So removing escaped_name has no impact on Windows behavior. --- radicle-surf/CHANGELOG.md | 8 ++++ radicle-surf/src/fs.rs | 12 ++---- radicle-surf/src/repo.rs | 1 + radicle-surf/t/src/file_system.rs | 64 ++++++++++++++++++++++++++++++- radicle-surf/t/src/last_commit.rs | 2 +- 5 files changed, 76 insertions(+), 11 deletions(-) diff --git a/radicle-surf/CHANGELOG.md b/radicle-surf/CHANGELOG.md index 835e6d5..c309a37 100644 --- a/radicle-surf/CHANGELOG.md +++ b/radicle-surf/CHANGELOG.md @@ -1,5 +1,13 @@ # CHANGELOG +## [Unreleased] + +### Fixed + +- `radicle-surf` has learned how to handle directories that use `[]` in their + name. This was a limitation in how the `git2` library was used for searching + "pathspecs". + ## Version 0.25.0 * Update to `radicle-git-ext-0.10.0` [6422fd5](https://app.radicle.xyz/nodes/seed.radicle.xyz/rad:z6cFWeWpnZNHh9rUW8phgA3b5yGt/commits/6422fd580b1c9c96ba40620197e29d7b9fbe2824) diff --git a/radicle-surf/src/fs.rs b/radicle-surf/src/fs.rs index ce03062..9ceec21 100644 --- a/radicle-surf/src/fs.rs +++ b/radicle-surf/src/fs.rs @@ -114,7 +114,7 @@ impl File { /// /// The path is relative to the git repository root. pub fn path(&self) -> PathBuf { - self.prefix.join(escaped_name(&self.name)) + self.prefix.join(&self.name) } /// Return the [`Path`] where this `File` is located, relative to the @@ -345,7 +345,7 @@ impl Directory { /// /// The path is relative to the git repository root. pub fn path(&self) -> PathBuf { - self.prefix.join(escaped_name(&self.name)) + self.prefix.join(&self.name) } /// Return the [`Path`] where this `Directory` is located, relative to the @@ -587,7 +587,7 @@ impl Submodule { /// /// The path is relative to the git repository root. pub fn path(&self) -> PathBuf { - self.prefix.join(escaped_name(&self.name)) + self.prefix.join(&self.name) } /// The object identifier of this `Submodule`. @@ -603,9 +603,3 @@ impl Submodule { &self.url } } - -/// When we need to escape "\" (represented as `\\`) for `PathBuf` -/// so that it can be processed correctly. -fn escaped_name(name: &str) -> String { - name.replace('\\', r"\\") -} diff --git a/radicle-surf/src/repo.rs b/radicle-surf/src/repo.rs index 97c8a57..b71a7d0 100644 --- a/radicle-surf/src/repo.rs +++ b/radicle-surf/src/repo.rs @@ -510,6 +510,7 @@ impl Repository { let mut opts = git2::DiffOptions::new(); if let Some(path) = path { opts.pathspec(path.to_string_lossy().to_string()); + opts.disable_pathspec_match(true); opts.skip_binary_check(false); } diff --git a/radicle-surf/t/src/file_system.rs b/radicle-surf/t/src/file_system.rs index 75a5096..bf61505 100644 --- a/radicle-surf/t/src/file_system.rs +++ b/radicle-surf/t/src/file_system.rs @@ -4,7 +4,7 @@ mod directory { use radicle_git_ext::ref_format::refname; use radicle_surf::{ fs::{self, Entry}, - Branch, Repository, + Branch, Oid, Repository, }; use std::path::Path; @@ -137,4 +137,66 @@ mod directory { "a0dd9122d33dff2a35f564d564db127152c88e02" ); } + + /// Test that directories and files with glob metacharacters in their names + /// can be browsed and have their history retrieved correctly. + /// + /// This is a regression test for a bug where paths containing `[` were + /// interpreted as glob patterns by git's pathspec, causing errors. + #[test] + fn directory_with_bracket_in_name() { + let repo = test_helpers::tempdir::WithTmpDir::new(|path| { + git2::Repository::init(path).map_err(std::io::Error::other) + }) + .unwrap(); + + // Initialize the repo and create test structure: + // src/ + // [special-dir]/ + // file.txt + // normal-file.txt + let commit = { + let mut tb = repo.treebuilder(None).unwrap(); + let hello = repo.blob(b"hello world").unwrap(); + tb.insert("file.txt", hello, git2::FileMode::Blob.into()) + .unwrap(); + let inner = tb.write().unwrap(); + let mut tb = repo.treebuilder(None).unwrap(); + let normal = repo.blob(b"normal content").unwrap(); + tb.insert("normal-file.txt", normal, git2::FileMode::Blob.into()) + .unwrap(); + tb.insert("[special-dir]", inner, git2::FileMode::Tree.into()) + .unwrap(); + let id = tb.write().unwrap(); + let mut tb = repo.treebuilder(None).unwrap(); + tb.insert("src", id, git2::FileMode::Tree.into()).unwrap(); + let tree = tb.write().unwrap(); + let tree = repo.find_tree(tree).unwrap(); + let sig = git2::Signature::now("Test", "test@test.com").unwrap(); + Oid::from( + repo.commit(Some("HEAD"), &sig, &sig, "Initial commit", &tree, &[]) + .unwrap(), + ) + }; + + let repo = Repository::open(repo.path()).unwrap(); + let branch = Branch::local(refname!("master")); + let root = repo.root_dir(&branch).unwrap(); + + let src = root.find_directory(&"src", &repo).unwrap(); + let src_entries: Vec = src.entries(&repo).unwrap().collect(); + assert_eq!(src_entries.len(), 2); + + let special_dir = src.find_directory(&"[special-dir]", &repo).unwrap(); + assert_eq!(special_dir.name(), "[special-dir]"); + + let dir_path = special_dir.path(); + let dir_last_commit = repo.last_commit(&dir_path, &branch).unwrap(); + assert_eq!(dir_last_commit.map(|c| c.id), Some(commit)); + + let file = special_dir.find_file(&"file.txt", &repo).unwrap(); + let file_path = file.path(); + let file_last_commit = repo.last_commit(&file_path, &branch).unwrap(); + assert_eq!(file_last_commit.map(|c| c.id), Some(commit)); + } } diff --git a/radicle-surf/t/src/last_commit.rs b/radicle-surf/t/src/last_commit.rs index 3bdf4ac..009893e 100644 --- a/radicle-surf/t/src/last_commit.rs +++ b/radicle-surf/t/src/last_commit.rs @@ -78,7 +78,7 @@ fn can_get_last_commit_for_special_filenames() { let expected_commit_id = Oid::from_str("a0dd9122d33dff2a35f564d564db127152c88e02").unwrap(); let backslash_commit_id = repo - .last_commit(&r"special/faux\\path", oid) + .last_commit(&"special/faux\\path", oid) .expect("Failed to get last commit") .map(|commit| commit.id); assert_eq!(backslash_commit_id, Some(expected_commit_id));