From 828e9035a40796f79650cf5e3becb8d8e5e29883 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 21 May 2025 09:55:59 +0200 Subject: [PATCH 1/4] feat!: Pattern parser in is now stateful to allow options for how to parse ignore patterns. That way it can support settings and other state that affect parsing. This affects various crates which are all marked as breaking now. --- gix-attributes/src/search/attributes.rs | 8 ++-- gix-glob/src/search/mod.rs | 12 ++++- gix-glob/src/search/pattern.rs | 10 ++-- gix-glob/tests/search/pattern.rs | 14 +++--- gix-ignore/src/lib.rs | 8 +++- gix-ignore/src/parse.rs | 11 ++++- gix-ignore/src/search.rs | 48 ++++++++++++++----- .../tests/{ignore.rs => ignore/main.rs} | 0 .../tests/{parse/mod.rs => ignore/parse.rs} | 28 +++++++---- .../tests/{search/mod.rs => ignore/search.rs} | 30 ++++++++---- gix-worktree/src/stack/state/ignore.rs | 18 +++++-- gix-worktree/src/stack/state/mod.rs | 2 + gix-worktree/tests/worktree/stack/ignore.rs | 9 ++-- tests/tools/src/lib.rs | 11 ++++- 14 files changed, 153 insertions(+), 56 deletions(-) rename gix-ignore/tests/{ignore.rs => ignore/main.rs} (100%) rename gix-ignore/tests/{parse/mod.rs => ignore/parse.rs} (79%) rename gix-ignore/tests/{search/mod.rs => ignore/search.rs} (87%) diff --git a/gix-attributes/src/search/attributes.rs b/gix-attributes/src/search/attributes.rs index 600c7d9765d..765aff06828 100644 --- a/gix-attributes/src/search/attributes.rs +++ b/gix-attributes/src/search/attributes.rs @@ -58,7 +58,8 @@ impl Search { ) -> std::io::Result { // TODO: should `Pattern` trait use an instance as first argument to carry this information // (so no `retain` later, it's slower than skipping) - let was_added = gix_glob::search::add_patterns_file(&mut self.patterns, source, follow_symlinks, root, buf)?; + let was_added = + gix_glob::search::add_patterns_file(&mut self.patterns, source, follow_symlinks, root, buf, Attributes)?; if was_added { let last = self.patterns.last_mut().expect("just added"); if !allow_macros { @@ -80,7 +81,8 @@ impl Search { collection: &mut MetadataCollection, allow_macros: bool, ) { - self.patterns.push(pattern::List::from_bytes(bytes, source, root)); + self.patterns + .push(pattern::List::from_bytes(bytes, source, root, Attributes)); let last = self.patterns.last_mut().expect("just added"); if !allow_macros { last.patterns @@ -124,7 +126,7 @@ impl Search { impl Pattern for Attributes { type Value = Value; - fn bytes_to_patterns(bytes: &[u8], _source: &std::path::Path) -> Vec> { + fn bytes_to_patterns(&self, bytes: &[u8], _source: &std::path::Path) -> Vec> { fn into_owned_assignments<'a>( attrs: impl Iterator, crate::name::Error>>, ) -> Option { diff --git a/gix-glob/src/search/mod.rs b/gix-glob/src/search/mod.rs index b6fb2a49025..1810c6cacc7 100644 --- a/gix-glob/src/search/mod.rs +++ b/gix-glob/src/search/mod.rs @@ -15,11 +15,12 @@ pub trait Pattern: Clone + PartialEq + Eq + std::fmt::Debug + std::hash::Hash + type Value: PartialEq + Eq + std::fmt::Debug + std::hash::Hash + Ord + PartialOrd + Clone; /// Parse all patterns in `bytes` line by line, ignoring lines with errors, and collect them. - fn bytes_to_patterns(bytes: &[u8], source: &Path) -> Vec>; + fn bytes_to_patterns(&self, bytes: &[u8], source: &Path) -> Vec>; } /// Add the given file at `source` if it exists, otherwise do nothing. /// If a `root` is provided, it's not considered a global file anymore. +/// `parse` is a way to parse bytes to pattern. /// Returns `true` if the file was added, or `false` if it didn't exist. pub fn add_patterns_file( patterns: &mut Vec>, @@ -27,8 +28,15 @@ pub fn add_patterns_file( follow_symlinks: bool, root: Option<&Path>, buf: &mut Vec, + parse: T, ) -> std::io::Result { let previous_len = patterns.len(); - patterns.extend(pattern::List::::from_file(source, root, follow_symlinks, buf)?); + patterns.extend(pattern::List::::from_file( + source, + root, + follow_symlinks, + buf, + parse, + )?); Ok(patterns.len() != previous_len) } diff --git a/gix-glob/src/search/pattern.rs b/gix-glob/src/search/pattern.rs index 54981651ecb..460de263e29 100644 --- a/gix-glob/src/search/pattern.rs +++ b/gix-glob/src/search/pattern.rs @@ -79,8 +79,9 @@ where /// `source_file` is the location of the `bytes` which represents a list of patterns, one pattern per line. /// If `root` is `Some(…)` it's used to see `source_file` as relative to itself, if `source_file` is absolute. /// If source is relative and should be treated as base, set `root` to `Some("")`. - pub fn from_bytes(bytes: &[u8], source_file: PathBuf, root: Option<&Path>) -> Self { - let patterns = T::bytes_to_patterns(bytes, source_file.as_path()); + /// `parse` is a way to parse bytes to pattern. + pub fn from_bytes(bytes: &[u8], source_file: PathBuf, root: Option<&Path>, parse: T) -> Self { + let patterns = parse.bytes_to_patterns(bytes, source_file.as_path()); let base = root .and_then(|root| source_file.parent().expect("file").strip_prefix(root).ok()) .and_then(|base| { @@ -101,14 +102,17 @@ where /// Create a pattern list from the `source` file, which may be located underneath `root`, while optionally /// following symlinks with `follow_symlinks`, providing `buf` to temporarily store the data contained in the file. + /// `parse` is a way to parse bytes to pattern. pub fn from_file( source: impl Into, root: Option<&Path>, follow_symlinks: bool, buf: &mut Vec, + parse: T, ) -> std::io::Result> { let source = source.into(); - Ok(read_in_full_ignore_missing(&source, follow_symlinks, buf)?.then(|| Self::from_bytes(buf, source, root))) + Ok(read_in_full_ignore_missing(&source, follow_symlinks, buf)? + .then(|| Self::from_bytes(buf, source, root, parse))) } } diff --git a/gix-glob/tests/search/pattern.rs b/gix-glob/tests/search/pattern.rs index cfdb410a67f..807277b31cd 100644 --- a/gix-glob/tests/search/pattern.rs +++ b/gix-glob/tests/search/pattern.rs @@ -15,7 +15,7 @@ mod list { impl Pattern for Dummy { type Value = (); - fn bytes_to_patterns(_bytes: &[u8], _source: &Path) -> Vec> { + fn bytes_to_patterns(&self, _bytes: &[u8], _source: &Path) -> Vec> { vec![] } } @@ -23,7 +23,7 @@ mod list { #[test] fn from_bytes_base() { { - let list = List::::from_bytes(&[], "a/b/source".into(), None); + let list = List::from_bytes(&[], "a/b/source".into(), None, Dummy); assert_eq!(list.base, None, "no root always means no-base, i.e. globals lists"); assert_eq!( list.source.as_deref(), @@ -34,7 +34,7 @@ mod list { { let cwd = std::env::current_dir().expect("cwd available"); - let list = List::::from_bytes(&[], cwd.join("a/b/source"), Some(cwd.as_path())); + let list = List::from_bytes(&[], cwd.join("a/b/source"), Some(cwd.as_path()), Dummy); assert_eq!( list.base.as_ref().expect("set"), "a/b/", @@ -48,7 +48,7 @@ mod list { } { - let list = List::::from_bytes(&[], "a/b/source".into(), Some(Path::new("c/"))); + let list = List::from_bytes(&[], "a/b/source".into(), Some(Path::new("c/")), Dummy); assert_eq!( list.base, None, "if root doesn't contain source, it silently skips it as base" @@ -63,7 +63,7 @@ mod list { #[test] fn strip_base_handle_recompute_basename_pos() { - let list = List::::from_bytes(&[], "a/b/source".into(), Some(Path::new(""))); + let list = List::from_bytes(&[], "a/b/source".into(), Some(Path::new("")), Dummy); assert_eq!( list.base.as_ref().expect("set"), "a/b/", @@ -91,7 +91,7 @@ mod list { Path::new(".").join("non-existing-dir").join("pattern-file"), Path::new("file").to_owned(), ] { - let list = List::::from_file(path, None, false, &mut buf).expect("no io error"); + let list = List::from_file(path, None, false, &mut buf, Dummy).expect("no io error"); assert!(list.is_none(), "the file does not exist"); } } @@ -102,7 +102,7 @@ mod list { let dir_path = tmp.path().join(".gitignore"); std::fs::create_dir(&dir_path)?; let mut buf = Vec::new(); - let list = List::::from_file(dir_path, None, false, &mut buf).expect("no io error"); + let list = List::from_file(dir_path, None, false, &mut buf, Dummy).expect("no io error"); assert!(list.is_none(), "directories are ignored just like Git does it"); Ok(()) diff --git a/gix-ignore/src/lib.rs b/gix-ignore/src/lib.rs index a9ba2351e3c..17501b0adc3 100644 --- a/gix-ignore/src/lib.rs +++ b/gix-ignore/src/lib.rs @@ -48,6 +48,10 @@ pub enum Kind { pub mod parse; /// Parse git ignore patterns, line by line, from `bytes`. -pub fn parse(bytes: &[u8]) -> parse::Lines<'_> { - parse::Lines::new(bytes) +/// +/// If `support_precious` is `true`, we will parse `$` prefixed entries as precious. +/// This is backward-incompatible as files that actually start with `$` like `$houdini` +/// will then not be ignored anymore, instead it ignores `houdini`. +pub fn parse(bytes: &[u8], support_precious: bool) -> parse::Lines<'_> { + parse::Lines::new(bytes, support_precious) } diff --git a/gix-ignore/src/parse.rs b/gix-ignore/src/parse.rs index cb53c456d94..a3057206b57 100644 --- a/gix-ignore/src/parse.rs +++ b/gix-ignore/src/parse.rs @@ -4,15 +4,22 @@ use bstr::ByteSlice; pub struct Lines<'a> { lines: bstr::Lines<'a>, line_no: usize, + /// Only if `true` we will be able to parse precious files. + support_precious: bool, } impl<'a> Lines<'a> { /// Create a new instance from `buf` to parse ignore patterns from. - pub fn new(buf: &'a [u8]) -> Self { + /// + /// If `support_precious` is `true`, we will parse `$` prefixed entries as precious. + /// This is backward-incompatible as files that actually start with `$` like `$houdini` + /// will then not be ignored anymore, instead it ignores `houdini`. + pub fn new(buf: &'a [u8], support_precious: bool) -> Self { let bom = unicode_bom::Bom::from(buf); Lines { lines: buf[bom.len()..].lines(), line_no: 0, + support_precious, } } } @@ -27,7 +34,7 @@ impl Iterator for Lines<'_> { Some(b'#') | None => continue, Some(c) => c, }; - let (kind, can_negate) = if first == b'$' { + let (kind, can_negate) = if self.support_precious && first == b'$' { line = &line[1..]; (crate::Kind::Precious, false) } else { diff --git a/gix-ignore/src/search.rs b/gix-ignore/src/search.rs index 9b5a2766b61..2b62b5797bd 100644 --- a/gix-ignore/src/search.rs +++ b/gix-ignore/src/search.rs @@ -21,15 +21,20 @@ pub struct Match<'a> { pub sequence_number: usize, } -/// An implementation of the [`Pattern`] trait for ignore patterns. -#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Default)] -pub struct Ignore; +/// An implementation of the [`Pattern`] trait for ignore-patterns. +#[derive(Default, PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)] +pub struct Ignore { + /// If `support_precious` is `true`, we will parse `$` prefixed entries as precious. + /// This is backward-incompatible as files that actually start with `$` like `$houdini` + /// will then not be ignored anymore, instead it ignores `houdini`. + pub support_precious: bool, +} impl Pattern for Ignore { type Value = crate::Kind; - fn bytes_to_patterns(bytes: &[u8], _source: &std::path::Path) -> Vec> { - crate::parse(bytes) + fn bytes_to_patterns(&self, bytes: &[u8], _source: &std::path::Path) -> Vec> { + crate::parse(bytes, self.support_precious) .map(|(pattern, line_number, kind)| pattern::Mapping { pattern, value: kind, @@ -44,14 +49,22 @@ impl Search { /// Given `git_dir`, a `.git` repository, load static ignore patterns from `info/exclude` /// and from `excludes_file` if it is provided. /// Note that it's not considered an error if the provided `excludes_file` does not exist. - pub fn from_git_dir(git_dir: &Path, excludes_file: Option, buf: &mut Vec) -> std::io::Result { + /// `parse` is a way to parse bytes to ignore patterns. + pub fn from_git_dir( + git_dir: &Path, + excludes_file: Option, + buf: &mut Vec, + parse: Ignore, + ) -> std::io::Result { let mut group = Self::default(); let follow_symlinks = true; // order matters! More important ones first. group.patterns.extend( excludes_file - .and_then(|file| pattern::List::::from_file(file, None, follow_symlinks, buf).transpose()) + .and_then(|file| { + pattern::List::::from_file(file, None, follow_symlinks, buf, parse).transpose() + }) .transpose()?, ); group.patterns.extend(pattern::List::::from_file( @@ -59,23 +72,25 @@ impl Search { None, follow_symlinks, buf, + parse, )?); Ok(group) } /// Parse a list of ignore patterns, using slashes as path separators. - pub fn from_overrides(patterns: impl IntoIterator>) -> Self { - Self::from_overrides_inner(&mut patterns.into_iter().map(Into::into)) + /// `parse` is a way to parse bytes to ignore patterns. + pub fn from_overrides(patterns: impl IntoIterator>, parse: Ignore) -> Self { + Self::from_overrides_inner(&mut patterns.into_iter().map(Into::into), parse) } - fn from_overrides_inner(patterns: &mut dyn Iterator) -> Self { + fn from_overrides_inner(patterns: &mut dyn Iterator, parse: Ignore) -> Self { Search { patterns: vec![pattern::List { patterns: patterns .enumerate() .filter_map(|(seq_id, pattern)| { let pattern = gix_path::try_into_bstr(PathBuf::from(pattern)).ok()?; - crate::parse(pattern.as_ref()) + crate::parse(pattern.as_ref(), parse.support_precious) .next() .map(|(p, _seq_id, kind)| pattern::Mapping { pattern: p, @@ -95,9 +110,16 @@ impl Search { impl Search { /// Add patterns as parsed from `bytes`, providing their `source` path and possibly their `root` path, the path they /// are relative to. This also means that `source` is contained within `root` if `root` is provided. - pub fn add_patterns_buffer(&mut self, bytes: &[u8], source: impl Into, root: Option<&Path>) { + /// Use `parse` to control how ignore patterns are parsed. + pub fn add_patterns_buffer( + &mut self, + bytes: &[u8], + source: impl Into, + root: Option<&Path>, + parse: Ignore, + ) { self.patterns - .push(pattern::List::from_bytes(bytes, source.into(), root)); + .push(pattern::List::from_bytes(bytes, source.into(), root, parse)); } } diff --git a/gix-ignore/tests/ignore.rs b/gix-ignore/tests/ignore/main.rs similarity index 100% rename from gix-ignore/tests/ignore.rs rename to gix-ignore/tests/ignore/main.rs diff --git a/gix-ignore/tests/parse/mod.rs b/gix-ignore/tests/ignore/parse.rs similarity index 79% rename from gix-ignore/tests/parse/mod.rs rename to gix-ignore/tests/ignore/parse.rs index 612a977a251..e3ee21ce4fd 100644 --- a/gix-ignore/tests/parse/mod.rs +++ b/gix-ignore/tests/ignore/parse.rs @@ -5,7 +5,7 @@ use gix_testtools::fixture_bytes; #[test] fn precious() { let input = fixture_bytes("ignore/precious.txt"); - let actual: Vec<_> = gix_ignore::parse(&input).map(flat_map).collect(); + let actual: Vec<_> = gix_ignore::parse(&input, true).map(flat_map).collect(); assert_eq!( actual, vec![ @@ -16,12 +16,24 @@ fn precious() { pat_precious("!/*", Mode::empty(), 12), ] ); + + let actual: Vec<_> = gix_ignore::parse(&input, false).map(flat_map).collect(); + assert_eq!( + actual, + vec![ + pat("$.config", Mode::NO_SUB_DIR, 1), + pat("$starts-with-dollar", Mode::NO_SUB_DIR, 2), + pat("$*.html", Mode::NO_SUB_DIR, 4), + pat("foo.html", Mode::NO_SUB_DIR | Mode::NEGATIVE, 6), + pat("$!/*", Mode::empty(), 12), + ] + ); } #[test] fn byte_order_marks_are_no_patterns() { assert_eq!( - flatten(gix_ignore::parse("\u{feff}hello".as_bytes()).next()), + flatten(gix_ignore::parse("\u{feff}hello".as_bytes(), false).next()), Some(pat(r"hello", Mode::NO_SUB_DIR, 1)) ); } @@ -29,7 +41,7 @@ fn byte_order_marks_are_no_patterns() { #[test] fn line_numbers_are_counted_correctly() { let input = fixture_bytes("ignore/various.txt"); - let actual: Vec<_> = gix_ignore::parse(&input).map(flat_map).collect(); + let actual: Vec<_> = gix_ignore::parse(&input, false).map(flat_map).collect(); assert_eq!( actual, vec![ @@ -47,7 +59,7 @@ fn line_numbers_are_counted_correctly() { #[test] fn line_endings_can_be_windows_or_unix() { assert_eq!( - gix_ignore::parse(b"unix\nwindows\r\nlast") + gix_ignore::parse(b"unix\nwindows\r\nlast", false) .map(flat_map) .collect::>(), vec![ @@ -60,14 +72,14 @@ fn line_endings_can_be_windows_or_unix() { #[test] fn comments_are_ignored_as_well_as_empty_ones() { - assert!(gix_ignore::parse(b"# hello world").next().is_none()); - assert!(gix_ignore::parse(b"\n\r\n\t\t \n").next().is_none()); + assert!(gix_ignore::parse(b"# hello world", false).next().is_none()); + assert!(gix_ignore::parse(b"\n\r\n\t\t \n", false).next().is_none()); } #[test] fn backslashes_before_hashes_are_no_comments() { assert_eq!( - flatten(gix_ignore::parse(br"\#hello").next()), + flatten(gix_ignore::parse(br"\#hello", false).next()), Some(pat(r"#hello", Mode::NO_SUB_DIR, 1)) ); } @@ -75,7 +87,7 @@ fn backslashes_before_hashes_are_no_comments() { #[test] fn trailing_spaces_can_be_escaped_to_be_literal() { fn parse_one(input: &str) -> (BString, Mode, usize, gix_ignore::Kind) { - let actual: Vec<_> = gix_ignore::parse(input.as_bytes()).map(flat_map).collect(); + let actual: Vec<_> = gix_ignore::parse(input.as_bytes(), false).map(flat_map).collect(); assert_eq!(actual.len(), 1, "{input:?} should match"); actual.into_iter().next().expect("present") } diff --git a/gix-ignore/tests/search/mod.rs b/gix-ignore/tests/ignore/search.rs similarity index 87% rename from gix-ignore/tests/search/mod.rs rename to gix-ignore/tests/ignore/search.rs index ee47c44803e..24b338ab72b 100644 --- a/gix-ignore/tests/search/mod.rs +++ b/gix-ignore/tests/ignore/search.rs @@ -46,11 +46,22 @@ fn baseline_from_git_dir() -> crate::Result { let baseline = std::fs::read(git_dir.parent().unwrap().join("git-check-ignore.baseline"))?; let mut buf = Vec::new(); let user_exclude = dir.join("user.exclude"); - let mut group = - gix_ignore::Search::from_git_dir(&git_dir, user_exclude.is_file().then_some(user_exclude), &mut buf)?; + let mut group = gix_ignore::Search::from_git_dir( + &git_dir, + user_exclude.is_file().then_some(user_exclude), + &mut buf, + Default::default(), + )?; assert!( - !gix_glob::search::add_patterns_file(&mut group.patterns, "not-a-file".into(), false, None, &mut buf)?, + !gix_glob::search::add_patterns_file( + &mut group.patterns, + "not-a-file".into(), + false, + None, + &mut buf, + Default::default() + )?, "missing files are no problem and cause a negative response" ); let mut ignore_file = repo_dir.join(".gitignore"); @@ -64,7 +75,8 @@ fn baseline_from_git_dir() -> crate::Result { ignore_file, true, repo_dir.as_path().into(), - &mut buf + &mut buf, + Default::default() )?, "existing files return true" ); @@ -72,7 +84,7 @@ fn baseline_from_git_dir() -> crate::Result { let ignore_file = repo_dir.join("dir-with-ignore").join(".gitignore"); if ignore_file.is_file() { let buf = std::fs::read(&ignore_file)?; - group.add_patterns_buffer(&buf, ignore_file, repo_dir.as_path().into()); + group.add_patterns_buffer(&buf, ignore_file, repo_dir.as_path().into(), Default::default()); } for (path, source_and_line) in (Expectations { @@ -116,7 +128,7 @@ fn baseline_from_git_dir() -> crate::Result { #[test] fn from_overrides_with_precious() { let input = ["$s?mple", "pattern/"]; - let group = gix_ignore::Search::from_overrides(input.iter()); + let group = gix_ignore::Search::from_overrides(input.iter(), gix_ignore::search::Ignore { support_precious: true }); assert_eq!( group.pattern_matching_relative_path("Simple".into(), None, gix_glob::pattern::Case::Fold), @@ -131,7 +143,7 @@ fn from_overrides_with_precious() { #[test] fn from_overrides_with_excludes() { - let group = gix_ignore::Search::from_overrides(["$simple", "!simple", "pattern/"]); + let group = gix_ignore::Search::from_overrides(["$simple", "!simple", "pattern/"], Default::default()); assert_eq!( group.pattern_matching_relative_path("Simple".into(), None, gix_glob::pattern::Case::Fold), Some(pattern_to_match( @@ -145,7 +157,7 @@ fn from_overrides_with_excludes() { #[test] fn from_overrides() { - let group = gix_ignore::Search::from_overrides(["simple", "pattern/"]); + let group = gix_ignore::Search::from_overrides(["simple", "pattern/"], Default::default()); assert_eq!( group.pattern_matching_relative_path("Simple".into(), None, gix_glob::pattern::Case::Fold), Some(pattern_to_match( @@ -164,7 +176,7 @@ fn from_overrides() { ); assert_eq!(group.patterns.len(), 1); assert_eq!( - gix_ignore::Search::from_overrides(["simple", "pattern/"]).patterns[0], + gix_ignore::Search::from_overrides(["simple", "pattern/"], Default::default()).patterns[0], group.patterns.into_iter().next().unwrap() ); } diff --git a/gix-worktree/src/stack/state/ignore.rs b/gix-worktree/src/stack/state/ignore.rs index 05c0f48d9f9..23a0b3a315c 100644 --- a/gix-worktree/src/stack/state/ignore.rs +++ b/gix-worktree/src/stack/state/ignore.rs @@ -9,6 +9,9 @@ use crate::{ PathIdMapping, }; +/// Specify how to parse ignore patterns. +pub use gix_ignore::search::Ignore as ParseIgnore; + /// Decide where to read `.gitignore` files from. #[derive(Default, Debug, Clone, Copy)] pub enum Source { @@ -55,11 +58,14 @@ impl Ignore { /// /// The `exclude_file_name_for_directories` is an optional override for the filename to use when checking per-directory /// ignore files within the repository, defaults to`.gitignore`. + /// + /// `parse` controls how to parse ignore files. pub fn new( overrides: IgnoreMatchGroup, globals: IgnoreMatchGroup, exclude_file_name_for_directories: Option<&BStr>, source: Source, + parse: gix_ignore::search::Ignore, ) -> Self { Ignore { overrides, @@ -69,6 +75,7 @@ impl Ignore { exclude_file_name_for_directories: exclude_file_name_for_directories .map_or_else(|| ".gitignore".into(), ToOwned::to_owned), source, + parse, } } } @@ -183,7 +190,7 @@ impl Ignore { .map_err(std::io::Error::other)?; let ignore_path = gix_path::from_bstring(ignore_path_relative.into_owned()); self.stack - .add_patterns_buffer(ignore_blob.data, ignore_path, Some(Path::new(""))); + .add_patterns_buffer(ignore_blob.data, ignore_path, Some(Path::new("")), self.parse); stats.patterns_buffers += 1; } Err(_) => { @@ -200,6 +207,7 @@ impl Ignore { follow_symlinks, Some(root), buf, + self.parse, )?; stats.pattern_files += usize::from(added); stats.tried_pattern_files += 1; @@ -210,8 +218,12 @@ impl Ignore { .find_blob(&id_mappings[idx].1, buf) .map_err(std::io::Error::other)?; let ignore_path = gix_path::from_bstring(ignore_path_relative.into_owned()); - self.stack - .add_patterns_buffer(ignore_blob.data, ignore_path, Some(Path::new(""))); + self.stack.add_patterns_buffer( + ignore_blob.data, + ignore_path, + Some(Path::new("")), + self.parse, + ); stats.patterns_buffers += 1; } Err(_) => { diff --git a/gix-worktree/src/stack/state/mod.rs b/gix-worktree/src/stack/state/mod.rs index be423623562..9b635723abb 100644 --- a/gix-worktree/src/stack/state/mod.rs +++ b/gix-worktree/src/stack/state/mod.rs @@ -47,6 +47,8 @@ pub struct Ignore { pub(crate) exclude_file_name_for_directories: BString, /// Where to read ignore files from source: ignore::Source, + /// Control how to parse ignore files. + parse: gix_ignore::search::Ignore, } /// diff --git a/gix-worktree/tests/worktree/stack/ignore.rs b/gix-worktree/tests/worktree/stack/ignore.rs index 03feec6c4ac..06d73402318 100644 --- a/gix-worktree/tests/worktree/stack/ignore.rs +++ b/gix-worktree/tests/worktree/stack/ignore.rs @@ -43,9 +43,10 @@ fn exclude_by_dir_is_handled_just_like_git() { Default::default(), gix_worktree::stack::state::Ignore::new( Default::default(), - gix_ignore::Search::from_git_dir(&git_dir, None, &mut buf).unwrap(), + gix_ignore::Search::from_git_dir(&git_dir, None, &mut buf, Default::default()).unwrap(), None, Source::WorktreeThenIdMappingIfNotSkipped, + Default::default(), ), ); let mut cache = Stack::new(&dir, state, case, buf, Default::default()); @@ -113,13 +114,15 @@ fn check_against_baseline() -> crate::Result { let case = probe_case()?; let mut index = gix_index::File::at(git_dir.join("index"), gix_hash::Kind::Sha1, false, Default::default())?; let odb = gix_odb::at(git_dir.join("objects"))?; + let parse_ignore = gix_ignore::search::Ignore::default(); let state = gix_worktree::stack::State::for_add( Default::default(), gix_worktree::stack::state::Ignore::new( - gix_ignore::Search::from_overrides(["!force-include"]), - gix_ignore::Search::from_git_dir(&git_dir, Some(user_exclude_path), &mut buf)?, + gix_ignore::Search::from_overrides(["!force-include"], parse_ignore), + gix_ignore::Search::from_git_dir(&git_dir, Some(user_exclude_path), &mut buf, parse_ignore)?, None, Source::WorktreeThenIdMappingIfNotSkipped, + parse_ignore, ), ); let paths_storage = index.take_path_backing(); diff --git a/tests/tools/src/lib.rs b/tests/tools/src/lib.rs index 8f04dc35cc3..caf882af5c8 100644 --- a/tests/tools/src/lib.rs +++ b/tests/tools/src/lib.rs @@ -74,9 +74,18 @@ static EXCLUDE_LUT: Lazy>> = Lazy::new(|| { }; let state = gix_worktree::stack::State::IgnoreStack(gix_worktree::stack::state::Ignore::new( Default::default(), - gix_worktree::ignore::Search::from_git_dir(&gix_dir, None, &mut buf).ok()?, + gix_worktree::ignore::Search::from_git_dir( + &gix_dir, + None, + &mut buf, + gix_worktree::stack::state::ignore::ParseIgnore { + support_precious: false, + }, + ) + .ok()?, None, gix_worktree::stack::state::ignore::Source::WorktreeThenIdMappingIfNotSkipped, + Default::default(), )); Some(gix_worktree::Stack::new( work_tree, From 85a24b3a07f08bc83a3ef34c3f07ed00cdbd9fe2 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 21 May 2025 10:14:09 +0200 Subject: [PATCH 2/4] feat: add `gitoxide.parsePrecious` configuration key to opt-in to precious file parsing. --- gix/src/config/cache/access.rs | 11 ++++++++++- gix/src/config/cache/init.rs | 14 ++++++++++---- gix/src/config/mod.rs | 3 +++ gix/src/config/tree/sections/gitoxide.rs | 5 ++++- gix/tests/gix-init.rs | 2 ++ 5 files changed, 29 insertions(+), 6 deletions(-) diff --git a/gix/src/config/cache/access.rs b/gix/src/config/cache/access.rs index aa5b59f6f1a..c81abbbada5 100644 --- a/gix/src/config/cache/access.rs +++ b/gix/src/config/cache/access.rs @@ -396,11 +396,20 @@ impl Cache { Some(user_path) => Some(user_path), None => self.xdg_config_path("ignore")?, }; + let parse_ignore = gix_ignore::search::Ignore { + support_precious: boolean( + self, + "gitoxide.parsePrecious", + &config::tree::Gitoxide::PARSE_PRECIOUS, + false, + )?, + }; Ok(gix_worktree::stack::state::Ignore::new( overrides.unwrap_or_default(), - gix_ignore::Search::from_git_dir(git_dir, excludes_file, buf)?, + gix_ignore::Search::from_git_dir(git_dir, excludes_file, buf, parse_ignore)?, None, source, + parse_ignore, )) } // TODO: at least one test, maybe related to core.attributesFile configuration. diff --git a/gix/src/config/cache/init.rs b/gix/src/config/cache/init.rs index eca58a5cacf..0901f2d445a 100644 --- a/gix/src/config/cache/init.rs +++ b/gix/src/config/cache/init.rs @@ -357,10 +357,16 @@ fn apply_environment_overrides( "gitoxide", None, git_prefix, - &[{ - let key = &Gitoxide::TRACE_PACKET; - (env(key), key.name) - }], + &[ + { + let key = &Gitoxide::TRACE_PACKET; + (env(key), key.name) + }, + { + let key = &Gitoxide::PARSE_PRECIOUS; + (env(key), key.name) + }, + ], ), ( "gitoxide", diff --git a/gix/src/config/mod.rs b/gix/src/config/mod.rs index 41a00c56a48..ffcd31edf7b 100644 --- a/gix/src/config/mod.rs +++ b/gix/src/config/mod.rs @@ -239,6 +239,7 @@ pub mod command_context { /// pub mod exclude_stack { + use crate::config; use std::path::PathBuf; /// The error produced when setting up a stack to query `gitignore` information. @@ -251,6 +252,8 @@ pub mod exclude_stack { EnvironmentPermission(#[from] gix_sec::permission::Error), #[error("The value for `core.excludesFile` could not be read from configuration")] ExcludesFilePathInterpolation(#[from] gix_config::path::interpolate::Error), + #[error(transparent)] + ParsePreciousEnabled(#[from] config::boolean::Error), } } diff --git a/gix/src/config/tree/sections/gitoxide.rs b/gix/src/config/tree/sections/gitoxide.rs index af24cfac66a..ec97e2ffb22 100644 --- a/gix/src/config/tree/sections/gitoxide.rs +++ b/gix/src/config/tree/sections/gitoxide.rs @@ -36,6 +36,9 @@ impl Gitoxide { /// The `gitoxide.tracePacket` Key. pub const TRACE_PACKET: keys::Boolean = keys::Boolean::new_boolean("tracePacket", &config::Tree::GITOXIDE) .with_environment_override("GIT_TRACE_PACKET"); + /// The `gitoxide.parsePrecious` Key. + pub const PARSE_PRECIOUS: keys::Boolean = keys::Boolean::new_boolean("parsePrecious", &config::Tree::GITOXIDE) + .with_environment_override("GIX_PARSE_PRECIOUS"); } impl Section for Gitoxide { @@ -44,7 +47,7 @@ impl Section for Gitoxide { } fn keys(&self) -> &[&dyn Key] { - &[&Self::USER_AGENT, &Self::TRACE_PACKET] + &[&Self::USER_AGENT, &Self::TRACE_PACKET, &Self::PARSE_PRECIOUS] } fn sub_sections(&self) -> &[&dyn Section] { diff --git a/gix/tests/gix-init.rs b/gix/tests/gix-init.rs index a0eb77045db..6a63c719c55 100644 --- a/gix/tests/gix-init.rs +++ b/gix/tests/gix-init.rs @@ -52,6 +52,7 @@ mod with_overrides { .set("GIT_AUTHOR_DATE", default_date) .set("EMAIL", "user email") .set("GIX_PACK_CACHE_MEMORY", "0") + .set("GIX_PARSE_PRECIOUS", "1") .set("GIX_OBJECT_CACHE_MEMORY", "5m") .set("GIX_CREDENTIALS_HELPER_STDERR", "creds-stderr") .set("GIX_EXTERNAL_COMMAND_STDERR", "filter-stderr") @@ -244,6 +245,7 @@ mod with_overrides { ("gitoxide.commit.authorDate", default_date), ("gitoxide.commit.committerDate", default_date), ("gitoxide.user.emailFallback", "user email"), + ("gitoxide.parsePrecious", "1"), ("core.deltaBaseCacheLimit", "0"), ("gitoxide.objects.cacheLimit", "5m"), ("gitoxide.pathspec.icase", "pathspecs-icase"), From 4ef7806e62954d069861bddb06cb8c0baf47bb69 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 21 May 2025 09:57:01 +0200 Subject: [PATCH 3/4] adapt to changes in `gix-ignore` and `gix-glob`, and more. --- gitoxide-core/src/repository/exclude.rs | 5 ++++- gix-dir/tests/walk_utils/mod.rs | 1 + .../tests/{driver/mod.rs => filter/driver.rs} | 0 .../tests/{ => filter}/eol/convert_to_git.rs | 0 .../{ => filter}/eol/convert_to_worktree.rs | 0 gix-filter/tests/{ => filter}/eol/mod.rs | 0 .../tests/{ident/mod.rs => filter/ident.rs} | 0 .../tests/{filter.rs => filter/main.rs} | 0 .../{ => filter}/pipeline/convert_to_git.rs | 0 .../pipeline/convert_to_worktree.rs | 0 gix-filter/tests/{ => filter}/pipeline/mod.rs | 1 + .../{worktree/mod.rs => filter/worktree.rs} | 0 gix/src/config/cache/access.rs | 21 ++++++++++++------- gix/src/repository/config/mod.rs | 9 ++++++++ 14 files changed, 28 insertions(+), 9 deletions(-) rename gix-filter/tests/{driver/mod.rs => filter/driver.rs} (100%) rename gix-filter/tests/{ => filter}/eol/convert_to_git.rs (100%) rename gix-filter/tests/{ => filter}/eol/convert_to_worktree.rs (100%) rename gix-filter/tests/{ => filter}/eol/mod.rs (100%) rename gix-filter/tests/{ident/mod.rs => filter/ident.rs} (100%) rename gix-filter/tests/{filter.rs => filter/main.rs} (100%) rename gix-filter/tests/{ => filter}/pipeline/convert_to_git.rs (100%) rename gix-filter/tests/{ => filter}/pipeline/convert_to_worktree.rs (100%) rename gix-filter/tests/{ => filter}/pipeline/mod.rs (98%) rename gix-filter/tests/{worktree/mod.rs => filter/worktree.rs} (100%) diff --git a/gitoxide-core/src/repository/exclude.rs b/gitoxide-core/src/repository/exclude.rs index 608d81bd949..eba046cccc6 100644 --- a/gitoxide-core/src/repository/exclude.rs +++ b/gitoxide-core/src/repository/exclude.rs @@ -37,7 +37,10 @@ pub fn query( let index = repo.index()?; let mut cache = repo.excludes( &index, - Some(gix::ignore::Search::from_overrides(overrides.into_iter())), + Some(gix::ignore::Search::from_overrides( + overrides.into_iter(), + repo.ignore_pattern_parser()?, + )), Default::default(), )?; diff --git a/gix-dir/tests/walk_utils/mod.rs b/gix-dir/tests/walk_utils/mod.rs index cbf61634249..878819c433e 100644 --- a/gix-dir/tests/walk_utils/mod.rs +++ b/gix-dir/tests/walk_utils/mod.rs @@ -330,6 +330,7 @@ pub fn try_collect_filtered_opts( Default::default(), None, gix_worktree::stack::state::ignore::Source::WorktreeThenIdMappingIfNotSkipped, + gix_ignore::search::Ignore { support_precious: true }, )), &index, index.path_backing(), diff --git a/gix-filter/tests/driver/mod.rs b/gix-filter/tests/filter/driver.rs similarity index 100% rename from gix-filter/tests/driver/mod.rs rename to gix-filter/tests/filter/driver.rs diff --git a/gix-filter/tests/eol/convert_to_git.rs b/gix-filter/tests/filter/eol/convert_to_git.rs similarity index 100% rename from gix-filter/tests/eol/convert_to_git.rs rename to gix-filter/tests/filter/eol/convert_to_git.rs diff --git a/gix-filter/tests/eol/convert_to_worktree.rs b/gix-filter/tests/filter/eol/convert_to_worktree.rs similarity index 100% rename from gix-filter/tests/eol/convert_to_worktree.rs rename to gix-filter/tests/filter/eol/convert_to_worktree.rs diff --git a/gix-filter/tests/eol/mod.rs b/gix-filter/tests/filter/eol/mod.rs similarity index 100% rename from gix-filter/tests/eol/mod.rs rename to gix-filter/tests/filter/eol/mod.rs diff --git a/gix-filter/tests/ident/mod.rs b/gix-filter/tests/filter/ident.rs similarity index 100% rename from gix-filter/tests/ident/mod.rs rename to gix-filter/tests/filter/ident.rs diff --git a/gix-filter/tests/filter.rs b/gix-filter/tests/filter/main.rs similarity index 100% rename from gix-filter/tests/filter.rs rename to gix-filter/tests/filter/main.rs diff --git a/gix-filter/tests/pipeline/convert_to_git.rs b/gix-filter/tests/filter/pipeline/convert_to_git.rs similarity index 100% rename from gix-filter/tests/pipeline/convert_to_git.rs rename to gix-filter/tests/filter/pipeline/convert_to_git.rs diff --git a/gix-filter/tests/pipeline/convert_to_worktree.rs b/gix-filter/tests/filter/pipeline/convert_to_worktree.rs similarity index 100% rename from gix-filter/tests/pipeline/convert_to_worktree.rs rename to gix-filter/tests/filter/pipeline/convert_to_worktree.rs diff --git a/gix-filter/tests/pipeline/mod.rs b/gix-filter/tests/filter/pipeline/mod.rs similarity index 98% rename from gix-filter/tests/pipeline/mod.rs rename to gix-filter/tests/filter/pipeline/mod.rs index 8c9d8fffcb1..fe42785a52d 100644 --- a/gix-filter/tests/pipeline/mod.rs +++ b/gix-filter/tests/filter/pipeline/mod.rs @@ -38,6 +38,7 @@ fn attribute_cache(name: &str) -> gix_testtools::Result { Default::default(), None, gix_worktree::stack::state::ignore::Source::WorktreeThenIdMappingIfNotSkipped, + Default::default(), ), ), Case::Sensitive, diff --git a/gix-filter/tests/worktree/mod.rs b/gix-filter/tests/filter/worktree.rs similarity index 100% rename from gix-filter/tests/worktree/mod.rs rename to gix-filter/tests/filter/worktree.rs diff --git a/gix/src/config/cache/access.rs b/gix/src/config/cache/access.rs index c81abbbada5..433efa26e9a 100644 --- a/gix/src/config/cache/access.rs +++ b/gix/src/config/cache/access.rs @@ -384,6 +384,18 @@ impl Cache { }) } + #[cfg(feature = "excludes")] + pub(crate) fn ignore_pattern_parser(&self) -> Result { + Ok(gix_ignore::search::Ignore { + support_precious: boolean( + self, + "gitoxide.parsePrecious", + &config::tree::Gitoxide::PARSE_PRECIOUS, + false, + )?, + }) + } + #[cfg(feature = "excludes")] pub(crate) fn assemble_exclude_globals( &self, @@ -396,14 +408,7 @@ impl Cache { Some(user_path) => Some(user_path), None => self.xdg_config_path("ignore")?, }; - let parse_ignore = gix_ignore::search::Ignore { - support_precious: boolean( - self, - "gitoxide.parsePrecious", - &config::tree::Gitoxide::PARSE_PRECIOUS, - false, - )?, - }; + let parse_ignore = self.ignore_pattern_parser()?; Ok(gix_worktree::stack::state::Ignore::new( overrides.unwrap_or_default(), gix_ignore::Search::from_git_dir(git_dir, excludes_file, buf, parse_ignore)?, diff --git a/gix/src/repository/config/mod.rs b/gix/src/repository/config/mod.rs index 1658a59a125..93e91b7ae8a 100644 --- a/gix/src/repository/config/mod.rs +++ b/gix/src/repository/config/mod.rs @@ -48,6 +48,15 @@ impl crate::Repository { self.config.big_file_threshold() } + /// Create a low-level parser for ignore patterns, for instance for use in [`excludes()`](crate::Repository::excludes()). + /// + /// Depending on the configuration, precious-file parsing in `.gitignore-files` is supported. + /// This means that `$` prefixed files will be interpreted as precious, which is a backwards-incompatible change. + #[cfg(feature = "excludes")] + pub fn ignore_pattern_parser(&self) -> Result { + self.config.ignore_pattern_parser() + } + /// Obtain options for use when connecting via `ssh`. #[cfg(feature = "blocking-network-client")] pub fn ssh_connect_options( From 1df1ebb34dd3e2101d8a112dda66f6bac5261ea7 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 21 May 2025 11:02:02 +0200 Subject: [PATCH 4/4] feat: Enable precious file parsing in `gix` CLI by default, allow overrides. That's pretty neat as one can now set `GIX_PARSE_PRECIOUS=0` in the environment to disable precious file parsing, good to see what difference it makes. It's also possible to do this wiht `gix -c gitoxide.parsePrecious=0`. --- src/plumbing/main.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/plumbing/main.rs b/src/plumbing/main.rs index 894b4e2031a..fe111b065cf 100644 --- a/src/plumbing/main.rs +++ b/src/plumbing/main.rs @@ -118,6 +118,16 @@ pub fn main() -> Result<()> { .append_config(config.iter(), gix::config::Source::Cli) .context("Unable to parse command-line configuration")?; } + { + let mut config_mut = repo.config_snapshot_mut(); + // Enable precious file parsing unless the user made a choice. + if config_mut + .boolean(gix::config::tree::Gitoxide::PARSE_PRECIOUS) + .is_none() + { + config_mut.set_raw_value(&gix::config::tree::Gitoxide::PARSE_PRECIOUS, "true")?; + } + } Ok(repo) } };