mirror of
https://seed.radicle.dev/rad:z6cFWeWpnZNHh9rUW8phgA3b5yGt.git
synced 2026-11-03 02:10:32 +08:00
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.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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"\\")
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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<Entry> = 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));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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));
|
||||
|
||||
Reference in New Issue
Block a user