From d79f896f48449af90efcc84da58418194cb31bff Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 2 Nov 2025 15:06:54 +0100 Subject: [PATCH 1/6] Allow complex glob patterns in one-sided refspecs - Modified validated() function to skip strict ref name validation for one-sided refspecs with globs - Allow multiple asterisks in one-sided refspecs (no destination) - Two-sided refspecs still require balanced patterns and reject multiple asterisks - Added tests for complex glob pattern parsing and simple glob matching - Updated existing tests to reflect the new behavior Co-authored-by: Byron <63622+Byron@users.noreply.github.com> --- gix-refspec/src/parse.rs | 31 ++++--- gix-refspec/tests/refspec/match_group.rs | 99 ++++++++++++++++++++++ gix-refspec/tests/refspec/parse/fetch.rs | 41 +++++++++ gix-refspec/tests/refspec/parse/invalid.rs | 20 ++++- gix-refspec/tests/refspec/parse/mod.rs | 9 ++ 5 files changed, 186 insertions(+), 14 deletions(-) diff --git a/gix-refspec/src/parse.rs b/gix-refspec/src/parse.rs index 398d93294ad..772c2aa975b 100644 --- a/gix-refspec/src/parse.rs +++ b/gix-refspec/src/parse.rs @@ -116,9 +116,11 @@ pub(crate) mod function { *spec = "HEAD".into(); } } - let (src, src_had_pattern) = validated(src, operation == Operation::Push && dst.is_some())?; - let (dst, dst_had_pattern) = validated(dst, false)?; - if mode != Mode::Negative && src_had_pattern != dst_had_pattern { + let is_one_sided = dst.is_none(); + let (src, src_had_pattern) = validated(src, operation == Operation::Push && dst.is_some(), is_one_sided)?; + let (dst, dst_had_pattern) = validated(dst, false, false)?; + // For one-sided refspecs, we don't need to check for pattern balance + if !is_one_sided && mode != Mode::Negative && src_had_pattern != dst_had_pattern { return Err(Error::PatternUnbalanced); } @@ -149,20 +151,27 @@ pub(crate) mod function { spec.len() >= gix_hash::Kind::shortest().len_in_hex() && spec.iter().all(u8::is_ascii_hexdigit) } - fn validated(spec: Option<&BStr>, allow_revspecs: bool) -> Result<(Option<&BStr>, bool), Error> { + fn validated(spec: Option<&BStr>, allow_revspecs: bool, is_one_sided: bool) -> Result<(Option<&BStr>, bool), Error> { match spec { Some(spec) => { let glob_count = spec.iter().filter(|b| **b == b'*').take(2).count(); if glob_count > 1 { - return Err(Error::PatternUnsupported { pattern: spec.into() }); + // For one-sided refspecs, allow any number of globs without validation + if !is_one_sided { + return Err(Error::PatternUnsupported { pattern: spec.into() }); + } } - let has_globs = glob_count == 1; + // Check if there are any globs (one or more asterisks) + let has_globs = glob_count > 0; if has_globs { - let mut buf = smallvec::SmallVec::<[u8; 256]>::with_capacity(spec.len()); - buf.extend_from_slice(spec); - let glob_pos = buf.find_byte(b'*').expect("glob present"); - buf[glob_pos] = b'a'; - gix_validate::reference::name_partial(buf.as_bstr())?; + // For one-sided refspecs, skip validation of glob patterns + if !is_one_sided { + let mut buf = smallvec::SmallVec::<[u8; 256]>::with_capacity(spec.len()); + buf.extend_from_slice(spec); + let glob_pos = buf.find_byte(b'*').expect("glob present"); + buf[glob_pos] = b'a'; + gix_validate::reference::name_partial(buf.as_bstr())?; + } } else { gix_validate::reference::name_partial(spec) .map_err(Error::from) diff --git a/gix-refspec/tests/refspec/match_group.rs b/gix-refspec/tests/refspec/match_group.rs index 42cece6799f..6ed285ece4c 100644 --- a/gix-refspec/tests/refspec/match_group.rs +++ b/gix-refspec/tests/refspec/match_group.rs @@ -184,3 +184,102 @@ mod multiple { ); } } + +mod complex_globs { + use gix_refspec::{parse::Operation, MatchGroup}; + use gix_hash::ObjectId; + use bstr::BString; + use std::borrow::Cow; + + #[test] + fn one_sided_complex_glob_patterns_can_be_parsed() { + // The key change is that complex glob patterns with multiple asterisks + // can now be parsed for one-sided refspecs + let spec1 = gix_refspec::parse("refs/*/foo/*".into(), Operation::Fetch); + assert!(spec1.is_ok(), "Should parse complex glob pattern for one-sided refspec"); + + let spec2 = gix_refspec::parse("refs/*/*/bar".into(), Operation::Fetch); + assert!(spec2.is_ok(), "Should parse complex glob pattern with multiple asterisks"); + + let spec3 = gix_refspec::parse("refs/heads/*/release/*".into(), Operation::Fetch); + assert!(spec3.is_ok(), "Should parse complex glob pattern"); + + // Two-sided refspecs with multiple asterisks should still fail + let spec4 = gix_refspec::parse("refs/*/foo/*:refs/remotes/*".into(), Operation::Fetch); + assert!(spec4.is_err(), "Two-sided refspecs with multiple asterisks should fail"); + } + + #[test] + fn one_sided_simple_glob_patterns_match() { + // Test that simple glob patterns (one asterisk) work correctly with matching + let refs = vec![ + create_ref("refs/heads/feature/foo", "1111111111111111111111111111111111111111"), + create_ref("refs/heads/bugfix/bar", "2222222222222222222222222222222222222222"), + create_ref("refs/tags/v1.0", "3333333333333333333333333333333333333333"), + create_ref("refs/pull/123", "4444444444444444444444444444444444444444"), + ]; + let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); + + // Test: refs/heads/* should match all refs under refs/heads/ + let spec = gix_refspec::parse("refs/heads/*".into(), Operation::Fetch).unwrap(); + let group = MatchGroup::from_fetch_specs([spec]); + let outcome = group.match_lhs(items.iter().copied()); + let mappings = outcome.mappings; + + assert_eq!(mappings.len(), 2, "Should match two refs under refs/heads/"); + + // Test: refs/tags/* should match all refs under refs/tags/ + let items2: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); + let spec2 = gix_refspec::parse("refs/tags/*".into(), Operation::Fetch).unwrap(); + let group2 = MatchGroup::from_fetch_specs([spec2]); + let outcome2 = group2.match_lhs(items2.iter().copied()); + let mappings2 = outcome2.mappings; + + assert_eq!(mappings2.len(), 1, "Should match one ref under refs/tags/"); + } + + #[test] + fn one_sided_glob_with_suffix_matches() { + // Test that glob patterns with suffix work correctly + let refs = vec![ + create_ref("refs/heads/feature", "1111111111111111111111111111111111111111"), + create_ref("refs/heads/feat", "2222222222222222222222222222222222222222"), + create_ref("refs/heads/main", "3333333333333333333333333333333333333333"), + ]; + let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); + + // Test: refs/heads/feat* should match refs/heads/feature and refs/heads/feat + let spec = gix_refspec::parse("refs/heads/feat*".into(), Operation::Fetch).unwrap(); + let group = MatchGroup::from_fetch_specs([spec]); + let outcome = group.match_lhs(items.iter().copied()); + let mappings = outcome.mappings; + + assert_eq!(mappings.len(), 2, "Should match two refs starting with feat"); + } + + // Helper function to create a ref + fn create_ref(name: &str, id_hex: &str) -> Ref { + Ref { + name: name.into(), + target: ObjectId::from_hex(id_hex.as_bytes()).unwrap(), + object: None, + } + } + + #[derive(Debug, Clone)] + struct Ref { + name: BString, + target: ObjectId, + object: Option, + } + + impl Ref { + fn to_item(&self) -> gix_refspec::match_group::Item<'_> { + gix_refspec::match_group::Item { + full_ref_name: self.name.as_ref(), + target: &self.target, + object: self.object.as_deref(), + } + } + } +} diff --git a/gix-refspec/tests/refspec/parse/fetch.rs b/gix-refspec/tests/refspec/parse/fetch.rs index c10a47801b2..7df32570392 100644 --- a/gix-refspec/tests/refspec/parse/fetch.rs +++ b/gix-refspec/tests/refspec/parse/fetch.rs @@ -174,3 +174,44 @@ fn ampersand_on_left_hand_side_is_head() { fn empty_refspec_is_enough_for_fetching_head_into_fetchhead() { assert_parse("", Instruction::Fetch(Fetch::Only { src: b("HEAD") })); } + +#[test] +fn complex_glob_patterns_are_allowed_in_one_sided_refspecs() { + // Complex patterns with multiple asterisks should work for one-sided refspecs + assert_parse( + "refs/*/foo/*", + Instruction::Fetch(Fetch::Only { + src: b("refs/*/foo/*"), + }), + ); + + assert_parse( + "+refs/heads/*/release/*", + Instruction::Fetch(Fetch::Only { + src: b("refs/heads/*/release/*"), + }), + ); + + // Even more complex patterns + assert_parse( + "refs/*/*/branch", + Instruction::Fetch(Fetch::Only { + src: b("refs/*/*/branch"), + }), + ); +} + +#[test] +fn complex_glob_patterns_still_fail_for_two_sided_refspecs() { + // Two-sided refspecs with complex patterns (multiple asterisks) should still fail + for spec in [ + "refs/*/foo/*:refs/remotes/origin/*", + "refs/*/*:refs/remotes/*", + "a/*/c/*:b/*", + ] { + assert!(matches!( + try_parse(spec, Operation::Fetch).unwrap_err(), + Error::PatternUnsupported { .. } + )); + } +} diff --git a/gix-refspec/tests/refspec/parse/invalid.rs b/gix-refspec/tests/refspec/parse/invalid.rs index db591d2e0b3..6bb165498df 100644 --- a/gix-refspec/tests/refspec/parse/invalid.rs +++ b/gix-refspec/tests/refspec/parse/invalid.rs @@ -27,24 +27,33 @@ fn whitespace() { #[test] fn complex_patterns_with_more_than_one_asterisk() { + // For one-sided refspecs, complex patterns are now allowed for op in [Operation::Fetch, Operation::Push] { - for spec in ["a/*/c/*", "a**:**b", "+:**/"] { + assert!(try_parse("a/*/c/*", op).is_ok()); + } + + // For two-sided refspecs, complex patterns should still fail + for op in [Operation::Fetch, Operation::Push] { + for spec in ["a/*/c/*:x/*/y/*", "a**:**b", "+:**/"] { assert!(matches!( try_parse(spec, op).unwrap_err(), Error::PatternUnsupported { .. } )); } } + + // Negative specs with multiple patterns still fail assert!(matches!( try_parse("^*/*", Operation::Fetch).unwrap_err(), - Error::PatternUnsupported { .. } + Error::NegativeGlobPattern )); } #[test] fn both_sides_need_pattern_if_one_uses_it() { + // For two-sided refspecs, both sides still need patterns if one uses it for op in [Operation::Fetch, Operation::Push] { - for spec in ["refs/*/a", ":a/*", "+:a/*", "a*:b/c", "a:b/*"] { + for spec in [":a/*", "+:a/*", "a*:b/c", "a:b/*"] { assert!( matches!(try_parse(spec, op).unwrap_err(), Error::PatternUnbalanced), "{}", @@ -52,6 +61,11 @@ fn both_sides_need_pattern_if_one_uses_it() { ); } } + + // One-sided refspecs with patterns are now allowed + for op in [Operation::Fetch, Operation::Push] { + assert!(try_parse("refs/*/a", op).is_ok()); + } } #[test] diff --git a/gix-refspec/tests/refspec/parse/mod.rs b/gix-refspec/tests/refspec/parse/mod.rs index 0e0cf77d56f..7662f04bd29 100644 --- a/gix-refspec/tests/refspec/parse/mod.rs +++ b/gix-refspec/tests/refspec/parse/mod.rs @@ -45,6 +45,10 @@ fn baseline() { ), true, ) => {} // we prefer failing fast, git let's it pass + // We now allow complex glob patterns in one-sided refspecs + (None, false) if is_one_sided_glob_pattern(spec, op) => { + // This is an intentional behavior change: we allow complex globs in one-sided refspecs + } _ => { eprintln!("{err_code} {res:?} {} {:?}", kind.as_bstr(), spec.as_bstr()); mismatch += 1; @@ -66,6 +70,11 @@ fn baseline() { panics ); } + + fn is_one_sided_glob_pattern(spec: &[u8], op: Operation) -> bool { + use bstr::ByteSlice; + matches!(op, Operation::Fetch) && spec.to_str().map(|s| s.contains('*') && !s.contains(':')).unwrap_or(false) + } } #[test] From bb84248d3515565b936f2cfb71a4bc79e4563a62 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 2 Nov 2025 15:07:32 +0100 Subject: [PATCH 2/6] cargo fmt --- gix-refspec/src/parse.rs | 6 +++- gix-refspec/tests/refspec/match_group.rs | 37 ++++++++++++---------- gix-refspec/tests/refspec/parse/fetch.rs | 8 ++--- gix-refspec/tests/refspec/parse/invalid.rs | 6 ++-- gix-refspec/tests/refspec/parse/mod.rs | 8 +++-- 5 files changed, 37 insertions(+), 28 deletions(-) diff --git a/gix-refspec/src/parse.rs b/gix-refspec/src/parse.rs index 772c2aa975b..6844d2f7ecb 100644 --- a/gix-refspec/src/parse.rs +++ b/gix-refspec/src/parse.rs @@ -151,7 +151,11 @@ pub(crate) mod function { spec.len() >= gix_hash::Kind::shortest().len_in_hex() && spec.iter().all(u8::is_ascii_hexdigit) } - fn validated(spec: Option<&BStr>, allow_revspecs: bool, is_one_sided: bool) -> Result<(Option<&BStr>, bool), Error> { + fn validated( + spec: Option<&BStr>, + allow_revspecs: bool, + is_one_sided: bool, + ) -> Result<(Option<&BStr>, bool), Error> { match spec { Some(spec) => { let glob_count = spec.iter().filter(|b| **b == b'*').take(2).count(); diff --git a/gix-refspec/tests/refspec/match_group.rs b/gix-refspec/tests/refspec/match_group.rs index 6ed285ece4c..d6a46ec3aac 100644 --- a/gix-refspec/tests/refspec/match_group.rs +++ b/gix-refspec/tests/refspec/match_group.rs @@ -186,9 +186,9 @@ mod multiple { } mod complex_globs { - use gix_refspec::{parse::Operation, MatchGroup}; - use gix_hash::ObjectId; use bstr::BString; + use gix_hash::ObjectId; + use gix_refspec::{parse::Operation, MatchGroup}; use std::borrow::Cow; #[test] @@ -197,18 +197,21 @@ mod complex_globs { // can now be parsed for one-sided refspecs let spec1 = gix_refspec::parse("refs/*/foo/*".into(), Operation::Fetch); assert!(spec1.is_ok(), "Should parse complex glob pattern for one-sided refspec"); - + let spec2 = gix_refspec::parse("refs/*/*/bar".into(), Operation::Fetch); - assert!(spec2.is_ok(), "Should parse complex glob pattern with multiple asterisks"); - + assert!( + spec2.is_ok(), + "Should parse complex glob pattern with multiple asterisks" + ); + let spec3 = gix_refspec::parse("refs/heads/*/release/*".into(), Operation::Fetch); assert!(spec3.is_ok(), "Should parse complex glob pattern"); - + // Two-sided refspecs with multiple asterisks should still fail let spec4 = gix_refspec::parse("refs/*/foo/*:refs/remotes/*".into(), Operation::Fetch); assert!(spec4.is_err(), "Two-sided refspecs with multiple asterisks should fail"); } - + #[test] fn one_sided_simple_glob_patterns_match() { // Test that simple glob patterns (one asterisk) work correctly with matching @@ -219,25 +222,25 @@ mod complex_globs { create_ref("refs/pull/123", "4444444444444444444444444444444444444444"), ]; let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); - + // Test: refs/heads/* should match all refs under refs/heads/ let spec = gix_refspec::parse("refs/heads/*".into(), Operation::Fetch).unwrap(); let group = MatchGroup::from_fetch_specs([spec]); let outcome = group.match_lhs(items.iter().copied()); let mappings = outcome.mappings; - + assert_eq!(mappings.len(), 2, "Should match two refs under refs/heads/"); - + // Test: refs/tags/* should match all refs under refs/tags/ let items2: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); let spec2 = gix_refspec::parse("refs/tags/*".into(), Operation::Fetch).unwrap(); let group2 = MatchGroup::from_fetch_specs([spec2]); let outcome2 = group2.match_lhs(items2.iter().copied()); let mappings2 = outcome2.mappings; - + assert_eq!(mappings2.len(), 1, "Should match one ref under refs/tags/"); } - + #[test] fn one_sided_glob_with_suffix_matches() { // Test that glob patterns with suffix work correctly @@ -247,16 +250,16 @@ mod complex_globs { create_ref("refs/heads/main", "3333333333333333333333333333333333333333"), ]; let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); - + // Test: refs/heads/feat* should match refs/heads/feature and refs/heads/feat let spec = gix_refspec::parse("refs/heads/feat*".into(), Operation::Fetch).unwrap(); let group = MatchGroup::from_fetch_specs([spec]); let outcome = group.match_lhs(items.iter().copied()); let mappings = outcome.mappings; - + assert_eq!(mappings.len(), 2, "Should match two refs starting with feat"); } - + // Helper function to create a ref fn create_ref(name: &str, id_hex: &str) -> Ref { Ref { @@ -265,14 +268,14 @@ mod complex_globs { object: None, } } - + #[derive(Debug, Clone)] struct Ref { name: BString, target: ObjectId, object: Option, } - + impl Ref { fn to_item(&self) -> gix_refspec::match_group::Item<'_> { gix_refspec::match_group::Item { diff --git a/gix-refspec/tests/refspec/parse/fetch.rs b/gix-refspec/tests/refspec/parse/fetch.rs index 7df32570392..c3e500d780a 100644 --- a/gix-refspec/tests/refspec/parse/fetch.rs +++ b/gix-refspec/tests/refspec/parse/fetch.rs @@ -180,18 +180,16 @@ fn complex_glob_patterns_are_allowed_in_one_sided_refspecs() { // Complex patterns with multiple asterisks should work for one-sided refspecs assert_parse( "refs/*/foo/*", - Instruction::Fetch(Fetch::Only { - src: b("refs/*/foo/*"), - }), + Instruction::Fetch(Fetch::Only { src: b("refs/*/foo/*") }), ); - + assert_parse( "+refs/heads/*/release/*", Instruction::Fetch(Fetch::Only { src: b("refs/heads/*/release/*"), }), ); - + // Even more complex patterns assert_parse( "refs/*/*/branch", diff --git a/gix-refspec/tests/refspec/parse/invalid.rs b/gix-refspec/tests/refspec/parse/invalid.rs index 6bb165498df..54902422378 100644 --- a/gix-refspec/tests/refspec/parse/invalid.rs +++ b/gix-refspec/tests/refspec/parse/invalid.rs @@ -31,7 +31,7 @@ fn complex_patterns_with_more_than_one_asterisk() { for op in [Operation::Fetch, Operation::Push] { assert!(try_parse("a/*/c/*", op).is_ok()); } - + // For two-sided refspecs, complex patterns should still fail for op in [Operation::Fetch, Operation::Push] { for spec in ["a/*/c/*:x/*/y/*", "a**:**b", "+:**/"] { @@ -41,7 +41,7 @@ fn complex_patterns_with_more_than_one_asterisk() { )); } } - + // Negative specs with multiple patterns still fail assert!(matches!( try_parse("^*/*", Operation::Fetch).unwrap_err(), @@ -61,7 +61,7 @@ fn both_sides_need_pattern_if_one_uses_it() { ); } } - + // One-sided refspecs with patterns are now allowed for op in [Operation::Fetch, Operation::Push] { assert!(try_parse("refs/*/a", op).is_ok()); diff --git a/gix-refspec/tests/refspec/parse/mod.rs b/gix-refspec/tests/refspec/parse/mod.rs index 7662f04bd29..61a4a2803a4 100644 --- a/gix-refspec/tests/refspec/parse/mod.rs +++ b/gix-refspec/tests/refspec/parse/mod.rs @@ -70,10 +70,14 @@ fn baseline() { panics ); } - + fn is_one_sided_glob_pattern(spec: &[u8], op: Operation) -> bool { use bstr::ByteSlice; - matches!(op, Operation::Fetch) && spec.to_str().map(|s| s.contains('*') && !s.contains(':')).unwrap_or(false) + matches!(op, Operation::Fetch) + && spec + .to_str() + .map(|s| s.contains('*') && !s.contains(':')) + .unwrap_or(false) } } From 0d5372d30bf58583419b8467ed5f3e2c5a6f16c7 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 2 Nov 2025 15:12:50 +0100 Subject: [PATCH 3/6] refactor --- Cargo.lock | 1 + gix-refspec/Cargo.toml | 1 + gix-refspec/tests/refspec/match_group.rs | 109 +++++++++++++++++------ 3 files changed, 84 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 82468ee5370..c96a4ad14a7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2256,6 +2256,7 @@ dependencies = [ "gix-revision", "gix-testtools", "gix-validate", + "insta", "smallvec", "thiserror 2.0.17", ] diff --git a/gix-refspec/Cargo.toml b/gix-refspec/Cargo.toml index 2c60ce1bd93..7e187c57477 100644 --- a/gix-refspec/Cargo.toml +++ b/gix-refspec/Cargo.toml @@ -25,3 +25,4 @@ smallvec = "1.15.1" [dev-dependencies] gix-testtools = { path = "../tests/tools" } +insta = "1.43.2" \ No newline at end of file diff --git a/gix-refspec/tests/refspec/match_group.rs b/gix-refspec/tests/refspec/match_group.rs index d6a46ec3aac..981130ffbb3 100644 --- a/gix-refspec/tests/refspec/match_group.rs +++ b/gix-refspec/tests/refspec/match_group.rs @@ -189,37 +189,36 @@ mod complex_globs { use bstr::BString; use gix_hash::ObjectId; use gix_refspec::{parse::Operation, MatchGroup}; - use std::borrow::Cow; #[test] fn one_sided_complex_glob_patterns_can_be_parsed() { // The key change is that complex glob patterns with multiple asterisks // can now be parsed for one-sided refspecs - let spec1 = gix_refspec::parse("refs/*/foo/*".into(), Operation::Fetch); - assert!(spec1.is_ok(), "Should parse complex glob pattern for one-sided refspec"); + let spec = gix_refspec::parse("refs/*/foo/*".into(), Operation::Fetch); + assert!(spec.is_ok(), "Should parse complex glob pattern for one-sided refspec"); - let spec2 = gix_refspec::parse("refs/*/*/bar".into(), Operation::Fetch); + let spec = gix_refspec::parse("refs/*/*/bar".into(), Operation::Fetch); assert!( - spec2.is_ok(), + spec.is_ok(), "Should parse complex glob pattern with multiple asterisks" ); - let spec3 = gix_refspec::parse("refs/heads/*/release/*".into(), Operation::Fetch); - assert!(spec3.is_ok(), "Should parse complex glob pattern"); + let spec = gix_refspec::parse("refs/heads/[a-z.]/release/*".into(), Operation::Fetch); + assert!(spec.is_ok(), "Should parse complex glob pattern"); // Two-sided refspecs with multiple asterisks should still fail - let spec4 = gix_refspec::parse("refs/*/foo/*:refs/remotes/*".into(), Operation::Fetch); - assert!(spec4.is_err(), "Two-sided refspecs with multiple asterisks should fail"); + let spec = gix_refspec::parse("refs/*/foo/*:refs/remotes/*".into(), Operation::Fetch); + assert!(spec.is_err(), "Two-sided refspecs with multiple asterisks should fail"); } #[test] fn one_sided_simple_glob_patterns_match() { // Test that simple glob patterns (one asterisk) work correctly with matching let refs = vec![ - create_ref("refs/heads/feature/foo", "1111111111111111111111111111111111111111"), - create_ref("refs/heads/bugfix/bar", "2222222222222222222222222222222222222222"), - create_ref("refs/tags/v1.0", "3333333333333333333333333333333333333333"), - create_ref("refs/pull/123", "4444444444444444444444444444444444444444"), + new_ref("refs/heads/feature/foo", "1111111111111111111111111111111111111111"), + new_ref("refs/heads/bugfix/bar", "2222222222222222222222222222222222222222"), + new_ref("refs/tags/v1.0", "3333333333333333333333333333333333333333"), + new_ref("refs/pull/123", "4444444444444444444444444444444444444444"), ]; let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); @@ -227,27 +226,61 @@ mod complex_globs { let spec = gix_refspec::parse("refs/heads/*".into(), Operation::Fetch).unwrap(); let group = MatchGroup::from_fetch_specs([spec]); let outcome = group.match_lhs(items.iter().copied()); - let mappings = outcome.mappings; - assert_eq!(mappings.len(), 2, "Should match two refs under refs/heads/"); + insta::assert_debug_snapshot!(outcome.mappings, @r#" + [ + Mapping { + item_index: Some( + 0, + ), + lhs: FullName( + "refs/heads/feature/foo", + ), + rhs: None, + spec_index: 0, + }, + Mapping { + item_index: Some( + 1, + ), + lhs: FullName( + "refs/heads/bugfix/bar", + ), + rhs: None, + spec_index: 0, + }, + ] + "#); // Test: refs/tags/* should match all refs under refs/tags/ - let items2: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); - let spec2 = gix_refspec::parse("refs/tags/*".into(), Operation::Fetch).unwrap(); - let group2 = MatchGroup::from_fetch_specs([spec2]); - let outcome2 = group2.match_lhs(items2.iter().copied()); - let mappings2 = outcome2.mappings; + let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); + let spec = gix_refspec::parse("refs/tags/v[0.9]*".into(), Operation::Fetch).unwrap(); + let group = MatchGroup::from_fetch_specs([spec]); + let outcome = group.match_lhs(items.iter().copied()); - assert_eq!(mappings2.len(), 1, "Should match one ref under refs/tags/"); + insta::assert_debug_snapshot!(outcome.mappings, @r#" + [ + Mapping { + item_index: Some( + 2, + ), + lhs: FullName( + "refs/tags/v1.0", + ), + rhs: None, + spec_index: 0, + }, + ] + "#); } #[test] fn one_sided_glob_with_suffix_matches() { // Test that glob patterns with suffix work correctly let refs = vec![ - create_ref("refs/heads/feature", "1111111111111111111111111111111111111111"), - create_ref("refs/heads/feat", "2222222222222222222222222222222222222222"), - create_ref("refs/heads/main", "3333333333333333333333333333333333333333"), + new_ref("refs/heads/feature", "1111111111111111111111111111111111111111"), + new_ref("refs/heads/feat", "2222222222222222222222222222222222222222"), + new_ref("refs/heads/main", "3333333333333333333333333333333333333333"), ]; let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); @@ -257,11 +290,33 @@ mod complex_globs { let outcome = group.match_lhs(items.iter().copied()); let mappings = outcome.mappings; - assert_eq!(mappings.len(), 2, "Should match two refs starting with feat"); + insta::assert_debug_snapshot!(mappings, @r#" + [ + Mapping { + item_index: Some( + 0, + ), + lhs: FullName( + "refs/heads/feature", + ), + rhs: None, + spec_index: 0, + }, + Mapping { + item_index: Some( + 1, + ), + lhs: FullName( + "refs/heads/feat", + ), + rhs: None, + spec_index: 0, + }, + ] + "#); } - // Helper function to create a ref - fn create_ref(name: &str, id_hex: &str) -> Ref { + fn new_ref(name: &str, id_hex: &str) -> Ref { Ref { name: name.into(), target: ObjectId::from_hex(id_hex.as_bytes()).unwrap(), From 1bccc54111f9cd888c4f8d42ba6b9b5e3a5e9994 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 2 Nov 2025 15:41:47 +0100 Subject: [PATCH 4/6] Add a way to match refspecs with full globs support --- Cargo.lock | 1 + gix-refspec/Cargo.toml | 1 + gix-refspec/src/match_group/util.rs | 10 +++++++++- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index c96a4ad14a7..b565828fd95 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2252,6 +2252,7 @@ name = "gix-refspec" version = "0.32.0" dependencies = [ "bstr", + "gix-glob", "gix-hash", "gix-revision", "gix-testtools", diff --git a/gix-refspec/Cargo.toml b/gix-refspec/Cargo.toml index 7e187c57477..c7513cc2c50 100644 --- a/gix-refspec/Cargo.toml +++ b/gix-refspec/Cargo.toml @@ -18,6 +18,7 @@ doctest = false gix-revision = { version = "^0.36.0", path = "../gix-revision", default-features = false } gix-validate = { version = "^0.10.1", path = "../gix-validate" } gix-hash = { version = "^0.20.0", path = "../gix-hash" } +gix-glob = { version = "^0.22.1", path = "../gix-glob" } bstr = { version = "1.12.0", default-features = false, features = ["std"] } thiserror = "2.0.17" diff --git a/gix-refspec/src/match_group/util.rs b/gix-refspec/src/match_group/util.rs index c4012717e57..7a55e90621c 100644 --- a/gix-refspec/src/match_group/util.rs +++ b/gix-refspec/src/match_group/util.rs @@ -6,6 +6,7 @@ use gix_hash::ObjectId; use crate::{match_group::Item, RefSpecRef}; /// A type keeping enough information about a ref-spec to be able to efficiently match it against multiple matcher items. +#[derive(Debug)] pub struct Matcher<'a> { pub(crate) lhs: Option>, pub(crate) rhs: Option>, @@ -46,6 +47,7 @@ pub(crate) enum Needle<'a> { FullName(&'a BStr), PartialName(&'a BStr), Glob { name: &'a BStr, asterisk_pos: usize }, + Pattern(&'a BStr), Object(ObjectId), } @@ -168,9 +170,15 @@ impl<'a> From<&'a BStr> for Needle<'a> { impl<'a> From> for Matcher<'a> { fn from(v: RefSpecRef<'a>) -> Self { - Matcher { + let mut m = Matcher { lhs: v.src.map(Into::into), rhs: v.dst.map(Into::into), + }; + if m.rhs.is_none() { + if let Some(src) = v.src { + m.lhs = Some(Needle::Pattern(src)) + } } + m } } From d6835f25374d17e940017359f41b3cce2ff1ea20 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 2 Nov 2025 14:55:10 +0000 Subject: [PATCH 5/6] Implement Needle::Pattern for full glob support in one-sided refspecs - Added Pattern matching case in Needle::matches() using gix_glob::wildmatch - Added Pattern handling in to_bstr_replace() method - Added is_complex_pattern() helper to detect patterns requiring wildmatch - Only use Needle::Pattern for complex patterns (multiple asterisks or [, ], ?, \) - Simple single-asterisk globs continue using efficient Needle::Glob - Fixed test pattern from v[0.9]* to v[0-9]* to match v1.0 correctly Co-authored-by: Byron <63622+Byron@users.noreply.github.com> --- gix-refspec/src/match_group/util.rs | 27 +++++++++++++++++++++++- gix-refspec/tests/refspec/match_group.rs | 2 +- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/gix-refspec/src/match_group/util.rs b/gix-refspec/src/match_group/util.rs index 7a55e90621c..aa8d3653116 100644 --- a/gix-refspec/src/match_group/util.rs +++ b/gix-refspec/src/match_group/util.rs @@ -104,6 +104,13 @@ impl<'a> Needle<'a> { let end = item.full_ref_name.len() - tail.len(); Match::GlobRange(*asterisk_pos..end) } + Needle::Pattern(pattern) => { + if gix_glob::wildmatch(pattern, item.full_ref_name, gix_glob::wildmatch::Mode::empty()) { + Match::Normal + } else { + Match::None + } + } Needle::Object(id) => { if *id == item.target { return Match::Normal; @@ -139,7 +146,11 @@ impl<'a> Needle<'a> { name.insert_str(0, "refs/heads/"); Cow::Owned(name.into()) } + (Needle::Pattern(name), None) => Cow::Borrowed(name), (Needle::Glob { .. }, None) => unreachable!("BUG: no range provided for glob pattern"), + (Needle::Pattern(_), Some(_)) => { + unreachable!("BUG: range provided for pattern, but patterns don't use ranges") + } (_, Some(_)) => { unreachable!("BUG: range provided even though needle wasn't a glob. Globs are symmetric.") } @@ -176,9 +187,23 @@ impl<'a> From> for Matcher<'a> { }; if m.rhs.is_none() { if let Some(src) = v.src { - m.lhs = Some(Needle::Pattern(src)) + // Only use Pattern for complex globs (multiple asterisks or other glob features) + // Simple single-asterisk globs can use the more efficient Needle::Glob + if is_complex_pattern(src) { + m.lhs = Some(Needle::Pattern(src)); + } } } m } } + +/// Check if a pattern is complex enough to require wildmatch instead of simple glob matching +fn is_complex_pattern(pattern: &BStr) -> bool { + let asterisk_count = pattern.iter().filter(|&&b| b == b'*').count(); + if asterisk_count > 1 { + return true; + } + // Check for other glob features: ?, [, ], \ + pattern.iter().any(|&b| b == b'?' || b == b'[' || b == b']' || b == b'\\') +} diff --git a/gix-refspec/tests/refspec/match_group.rs b/gix-refspec/tests/refspec/match_group.rs index 981130ffbb3..e522e105564 100644 --- a/gix-refspec/tests/refspec/match_group.rs +++ b/gix-refspec/tests/refspec/match_group.rs @@ -254,7 +254,7 @@ mod complex_globs { // Test: refs/tags/* should match all refs under refs/tags/ let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); - let spec = gix_refspec::parse("refs/tags/v[0.9]*".into(), Operation::Fetch).unwrap(); + let spec = gix_refspec::parse("refs/tags/v[0-9]*".into(), Operation::Fetch).unwrap(); let group = MatchGroup::from_fetch_specs([spec]); let outcome = group.match_lhs(items.iter().copied()); From ba2301f16261e9e1c49b51b94d6ac43548ec2114 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 2 Nov 2025 20:42:17 +0100 Subject: [PATCH 6/6] refactor --- gix-refspec/src/match_group/util.rs | 17 ++++++++++------- gix-refspec/tests/refspec/match_group.rs | 4 ++-- gix/tests/gix/config/tree.rs | 8 ++++---- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/gix-refspec/src/match_group/util.rs b/gix-refspec/src/match_group/util.rs index aa8d3653116..532b04630fd 100644 --- a/gix-refspec/src/match_group/util.rs +++ b/gix-refspec/src/match_group/util.rs @@ -105,7 +105,11 @@ impl<'a> Needle<'a> { Match::GlobRange(*asterisk_pos..end) } Needle::Pattern(pattern) => { - if gix_glob::wildmatch(pattern, item.full_ref_name, gix_glob::wildmatch::Mode::empty()) { + if gix_glob::wildmatch( + pattern, + item.full_ref_name, + gix_glob::wildmatch::Mode::NO_MATCH_SLASH_LITERAL, + ) { Match::Normal } else { Match::None @@ -187,9 +191,7 @@ impl<'a> From> for Matcher<'a> { }; if m.rhs.is_none() { if let Some(src) = v.src { - // Only use Pattern for complex globs (multiple asterisks or other glob features) - // Simple single-asterisk globs can use the more efficient Needle::Glob - if is_complex_pattern(src) { + if must_use_pattern_matching(src) { m.lhs = Some(Needle::Pattern(src)); } } @@ -199,11 +201,12 @@ impl<'a> From> for Matcher<'a> { } /// Check if a pattern is complex enough to require wildmatch instead of simple glob matching -fn is_complex_pattern(pattern: &BStr) -> bool { +fn must_use_pattern_matching(pattern: &BStr) -> bool { let asterisk_count = pattern.iter().filter(|&&b| b == b'*').count(); if asterisk_count > 1 { return true; } - // Check for other glob features: ?, [, ], \ - pattern.iter().any(|&b| b == b'?' || b == b'[' || b == b']' || b == b'\\') + pattern + .iter() + .any(|&b| b == b'?' || b == b'[' || b == b']' || b == b'\\') } diff --git a/gix-refspec/tests/refspec/match_group.rs b/gix-refspec/tests/refspec/match_group.rs index e522e105564..032dc360338 100644 --- a/gix-refspec/tests/refspec/match_group.rs +++ b/gix-refspec/tests/refspec/match_group.rs @@ -214,7 +214,7 @@ mod complex_globs { #[test] fn one_sided_simple_glob_patterns_match() { // Test that simple glob patterns (one asterisk) work correctly with matching - let refs = vec![ + let refs = [ new_ref("refs/heads/feature/foo", "1111111111111111111111111111111111111111"), new_ref("refs/heads/bugfix/bar", "2222222222222222222222222222222222222222"), new_ref("refs/tags/v1.0", "3333333333333333333333333333333333333333"), @@ -277,7 +277,7 @@ mod complex_globs { #[test] fn one_sided_glob_with_suffix_matches() { // Test that glob patterns with suffix work correctly - let refs = vec![ + let refs = [ new_ref("refs/heads/feature", "1111111111111111111111111111111111111111"), new_ref("refs/heads/feat", "2222222222222222222222222222222222222222"), new_ref("refs/heads/main", "3333333333333333333333333333333333333333"), diff --git a/gix/tests/gix/config/tree.rs b/gix/tests/gix/config/tree.rs index 686f26f461d..22190235f94 100644 --- a/gix/tests/gix/config/tree.rs +++ b/gix/tests/gix/config/tree.rs @@ -1125,17 +1125,17 @@ mod remote { assert_eq!( Remote::FETCH - .try_into_refspec(bcow("*/*/*"), gix_refspec::parse::Operation::Fetch) + .try_into_refspec(bcow("*/*/*:refs/heads/*"), gix_refspec::parse::Operation::Fetch) .unwrap_err() .to_string(), - "The refspec at \"remote..fetch=*/*/*\" could not be parsed" + "The refspec at \"remote..fetch=*/*/*:refs/heads/*\" could not be parsed" ); assert_eq!( Remote::PUSH - .try_into_refspec(bcow("*/*/*"), gix_refspec::parse::Operation::Push) + .try_into_refspec(bcow("*/*/*:refs/heads/*"), gix_refspec::parse::Operation::Push) .unwrap_err() .to_string(), - "The refspec at \"remote..push=*/*/*\" could not be parsed" + "The refspec at \"remote..push=*/*/*:refs/heads/*\" could not be parsed" ); } }