Skip to content

Commit cb5979f

Browse files
CopilotByron
andcommitted
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 <[email protected]>
1 parent 80ec1f2 commit cb5979f

File tree

5 files changed

+180
-14
lines changed

5 files changed

+180
-14
lines changed

gix-refspec/src/parse.rs

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,11 @@ pub(crate) mod function {
116116
*spec = "HEAD".into();
117117
}
118118
}
119-
let (src, src_had_pattern) = validated(src, operation == Operation::Push && dst.is_some())?;
120-
let (dst, dst_had_pattern) = validated(dst, false)?;
121-
if mode != Mode::Negative && src_had_pattern != dst_had_pattern {
119+
let is_one_sided = dst.is_none();
120+
let (src, src_had_pattern) = validated(src, operation == Operation::Push && dst.is_some(), is_one_sided)?;
121+
let (dst, dst_had_pattern) = validated(dst, false, false)?;
122+
// For one-sided refspecs, we don't need to check for pattern balance
123+
if !is_one_sided && mode != Mode::Negative && src_had_pattern != dst_had_pattern {
122124
return Err(Error::PatternUnbalanced);
123125
}
124126

@@ -149,20 +151,26 @@ pub(crate) mod function {
149151
spec.len() >= gix_hash::Kind::shortest().len_in_hex() && spec.iter().all(u8::is_ascii_hexdigit)
150152
}
151153

152-
fn validated(spec: Option<&BStr>, allow_revspecs: bool) -> Result<(Option<&BStr>, bool), Error> {
154+
fn validated(spec: Option<&BStr>, allow_revspecs: bool, is_one_sided: bool) -> Result<(Option<&BStr>, bool), Error> {
153155
match spec {
154156
Some(spec) => {
155157
let glob_count = spec.iter().filter(|b| **b == b'*').take(2).count();
156158
if glob_count > 1 {
157-
return Err(Error::PatternUnsupported { pattern: spec.into() });
159+
// For one-sided refspecs, allow any number of globs without validation
160+
if !is_one_sided {
161+
return Err(Error::PatternUnsupported { pattern: spec.into() });
162+
}
158163
}
159-
let has_globs = glob_count == 1;
164+
let has_globs = glob_count >= 1;
160165
if has_globs {
161-
let mut buf = smallvec::SmallVec::<[u8; 256]>::with_capacity(spec.len());
162-
buf.extend_from_slice(spec);
163-
let glob_pos = buf.find_byte(b'*').expect("glob present");
164-
buf[glob_pos] = b'a';
165-
gix_validate::reference::name_partial(buf.as_bstr())?;
166+
// For one-sided refspecs, skip validation of glob patterns
167+
if !is_one_sided {
168+
let mut buf = smallvec::SmallVec::<[u8; 256]>::with_capacity(spec.len());
169+
buf.extend_from_slice(spec);
170+
let glob_pos = buf.find_byte(b'*').expect("glob present");
171+
buf[glob_pos] = b'a';
172+
gix_validate::reference::name_partial(buf.as_bstr())?;
173+
}
166174
} else {
167175
gix_validate::reference::name_partial(spec)
168176
.map_err(Error::from)

gix-refspec/tests/refspec/match_group.rs

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,3 +184,102 @@ mod multiple {
184184
);
185185
}
186186
}
187+
188+
mod complex_globs {
189+
use gix_refspec::{parse::Operation, MatchGroup};
190+
use gix_hash::ObjectId;
191+
use bstr::BString;
192+
use std::borrow::Cow;
193+
194+
#[test]
195+
fn one_sided_complex_glob_patterns_can_be_parsed() {
196+
// The key change is that complex glob patterns with multiple asterisks
197+
// can now be parsed for one-sided refspecs
198+
let spec1 = gix_refspec::parse("refs/*/foo/*".into(), Operation::Fetch);
199+
assert!(spec1.is_ok(), "Should parse complex glob pattern for one-sided refspec");
200+
201+
let spec2 = gix_refspec::parse("refs/*/*/bar".into(), Operation::Fetch);
202+
assert!(spec2.is_ok(), "Should parse complex glob pattern with multiple asterisks");
203+
204+
let spec3 = gix_refspec::parse("refs/heads/*/release/*".into(), Operation::Fetch);
205+
assert!(spec3.is_ok(), "Should parse complex glob pattern");
206+
207+
// Two-sided refspecs with multiple asterisks should still fail
208+
let spec4 = gix_refspec::parse("refs/*/foo/*:refs/remotes/*".into(), Operation::Fetch);
209+
assert!(spec4.is_err(), "Two-sided refspecs with multiple asterisks should fail");
210+
}
211+
212+
#[test]
213+
fn one_sided_simple_glob_patterns_match() {
214+
// Test that simple glob patterns (one asterisk) work correctly with matching
215+
let refs = vec![
216+
create_ref("refs/heads/feature/foo", "1111111111111111111111111111111111111111"),
217+
create_ref("refs/heads/bugfix/bar", "2222222222222222222222222222222222222222"),
218+
create_ref("refs/tags/v1.0", "3333333333333333333333333333333333333333"),
219+
create_ref("refs/pull/123", "4444444444444444444444444444444444444444"),
220+
];
221+
let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect();
222+
223+
// Test: refs/heads/* should match all refs under refs/heads/
224+
let spec = gix_refspec::parse("refs/heads/*".into(), Operation::Fetch).unwrap();
225+
let group = MatchGroup::from_fetch_specs([spec]);
226+
let outcome = group.match_lhs(items.iter().copied());
227+
let mappings = outcome.mappings;
228+
229+
assert_eq!(mappings.len(), 2, "Should match two refs under refs/heads/");
230+
231+
// Test: refs/tags/* should match all refs under refs/tags/
232+
let items2: Vec<_> = refs.iter().map(|r| r.to_item()).collect();
233+
let spec2 = gix_refspec::parse("refs/tags/*".into(), Operation::Fetch).unwrap();
234+
let group2 = MatchGroup::from_fetch_specs([spec2]);
235+
let outcome2 = group2.match_lhs(items2.iter().copied());
236+
let mappings2 = outcome2.mappings;
237+
238+
assert_eq!(mappings2.len(), 1, "Should match one ref under refs/tags/");
239+
}
240+
241+
#[test]
242+
fn one_sided_glob_with_suffix_matches() {
243+
// Test that glob patterns with suffix work correctly
244+
let refs = vec![
245+
create_ref("refs/heads/feature", "1111111111111111111111111111111111111111"),
246+
create_ref("refs/heads/feat", "2222222222222222222222222222222222222222"),
247+
create_ref("refs/heads/main", "3333333333333333333333333333333333333333"),
248+
];
249+
let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect();
250+
251+
// Test: refs/heads/feat* should match refs/heads/feature and refs/heads/feat
252+
let spec = gix_refspec::parse("refs/heads/feat*".into(), Operation::Fetch).unwrap();
253+
let group = MatchGroup::from_fetch_specs([spec]);
254+
let outcome = group.match_lhs(items.iter().copied());
255+
let mappings = outcome.mappings;
256+
257+
assert_eq!(mappings.len(), 2, "Should match two refs starting with feat");
258+
}
259+
260+
// Helper function to create a ref
261+
fn create_ref(name: &str, id_hex: &str) -> Ref {
262+
Ref {
263+
name: name.into(),
264+
target: ObjectId::from_hex(id_hex.as_bytes()).unwrap(),
265+
object: None,
266+
}
267+
}
268+
269+
#[derive(Debug, Clone)]
270+
struct Ref {
271+
name: BString,
272+
target: ObjectId,
273+
object: Option<ObjectId>,
274+
}
275+
276+
impl Ref {
277+
fn to_item(&self) -> gix_refspec::match_group::Item<'_> {
278+
gix_refspec::match_group::Item {
279+
full_ref_name: self.name.as_ref(),
280+
target: &self.target,
281+
object: self.object.as_deref(),
282+
}
283+
}
284+
}
285+
}

gix-refspec/tests/refspec/parse/fetch.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,3 +174,44 @@ fn ampersand_on_left_hand_side_is_head() {
174174
fn empty_refspec_is_enough_for_fetching_head_into_fetchhead() {
175175
assert_parse("", Instruction::Fetch(Fetch::Only { src: b("HEAD") }));
176176
}
177+
178+
#[test]
179+
fn complex_glob_patterns_are_allowed_in_one_sided_refspecs() {
180+
// Complex patterns with multiple asterisks should work for one-sided refspecs
181+
assert_parse(
182+
"refs/*/foo/*",
183+
Instruction::Fetch(Fetch::Only {
184+
src: b("refs/*/foo/*"),
185+
}),
186+
);
187+
188+
assert_parse(
189+
"+refs/heads/*/release/*",
190+
Instruction::Fetch(Fetch::Only {
191+
src: b("refs/heads/*/release/*"),
192+
}),
193+
);
194+
195+
// Even more complex patterns
196+
assert_parse(
197+
"refs/*/*/branch",
198+
Instruction::Fetch(Fetch::Only {
199+
src: b("refs/*/*/branch"),
200+
}),
201+
);
202+
}
203+
204+
#[test]
205+
fn complex_glob_patterns_still_fail_for_two_sided_refspecs() {
206+
// Two-sided refspecs with complex patterns (multiple asterisks) should still fail
207+
for spec in [
208+
"refs/*/foo/*:refs/remotes/origin/*",
209+
"refs/*/*:refs/remotes/*",
210+
"a/*/c/*:b/*",
211+
] {
212+
assert!(matches!(
213+
try_parse(spec, Operation::Fetch).unwrap_err(),
214+
Error::PatternUnsupported { .. }
215+
));
216+
}
217+
}

gix-refspec/tests/refspec/parse/invalid.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,31 +27,45 @@ fn whitespace() {
2727

2828
#[test]
2929
fn complex_patterns_with_more_than_one_asterisk() {
30+
// For one-sided refspecs, complex patterns are now allowed
3031
for op in [Operation::Fetch, Operation::Push] {
31-
for spec in ["a/*/c/*", "a**:**b", "+:**/"] {
32+
assert!(try_parse("a/*/c/*", op).is_ok());
33+
}
34+
35+
// For two-sided refspecs, complex patterns should still fail
36+
for op in [Operation::Fetch, Operation::Push] {
37+
for spec in ["a/*/c/*:x/*/y/*", "a**:**b", "+:**/"] {
3238
assert!(matches!(
3339
try_parse(spec, op).unwrap_err(),
3440
Error::PatternUnsupported { .. }
3541
));
3642
}
3743
}
44+
45+
// Negative specs with multiple patterns still fail
3846
assert!(matches!(
3947
try_parse("^*/*", Operation::Fetch).unwrap_err(),
40-
Error::PatternUnsupported { .. }
48+
Error::NegativeGlobPattern
4149
));
4250
}
4351

4452
#[test]
4553
fn both_sides_need_pattern_if_one_uses_it() {
54+
// For two-sided refspecs, both sides still need patterns if one uses it
4655
for op in [Operation::Fetch, Operation::Push] {
47-
for spec in ["refs/*/a", ":a/*", "+:a/*", "a*:b/c", "a:b/*"] {
56+
for spec in [":a/*", "+:a/*", "a*:b/c", "a:b/*"] {
4857
assert!(
4958
matches!(try_parse(spec, op).unwrap_err(), Error::PatternUnbalanced),
5059
"{}",
5160
spec
5261
);
5362
}
5463
}
64+
65+
// One-sided refspecs with patterns are now allowed
66+
for op in [Operation::Fetch, Operation::Push] {
67+
assert!(try_parse("refs/*/a", op).is_ok());
68+
}
5569
}
5670

5771
#[test]

gix-refspec/tests/refspec/parse/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ fn baseline() {
4545
),
4646
true,
4747
) => {} // we prefer failing fast, git let's it pass
48+
// We now allow complex glob patterns in one-sided refspecs
49+
(None, false) if matches!(op, Operation::Fetch) && spec.to_str().map(|s| s.contains('*') && !s.contains(':')).unwrap_or(false) => {
50+
// This is an intentional behavior change: we allow complex globs in one-sided refspecs
51+
}
4852
_ => {
4953
eprintln!("{err_code} {res:?} {} {:?}", kind.as_bstr(), spec.as_bstr());
5054
mismatch += 1;

0 commit comments

Comments
 (0)