Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

Commit 3db5510

Browse files
committed
fix wildcard handling in RefSpec matching
1) The guard logic here was inverted, resulting in an always-false branch, which meant that the suffix after the wildcard was incorrectly ignored. 2) Wildcards were treated as 1-or-more matches, but git treats them as 0-or-more. This change aligns go-git with git, but represents a bit of a breaking change for go-git.
1 parent 37b8072 commit 3db5510

File tree

2 files changed

+76
-11
lines changed

2 files changed

+76
-11
lines changed

config/refspec.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,11 @@ func (s RefSpec) matchGlob(n plumbing.ReferenceName) bool {
9999

100100
var prefix, suffix string
101101
prefix = src[0:wildcard]
102-
if len(src) < wildcard {
103-
suffix = src[wildcard+1 : len(suffix)]
102+
if len(src) > wildcard+1 {
103+
suffix = src[wildcard+1:]
104104
}
105105

106-
return len(name) > len(prefix)+len(suffix) &&
106+
return len(name) >= len(prefix)+len(suffix) &&
107107
strings.HasPrefix(name, prefix) &&
108108
strings.HasSuffix(name, suffix)
109109
}

config/refspec_test.go

+73-8
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,38 @@ func (s *RefSpecSuite) TestRefSpecMatch(c *C) {
9696
}
9797

9898
func (s *RefSpecSuite) TestRefSpecMatchGlob(c *C) {
99-
spec := RefSpec("refs/heads/*:refs/remotes/origin/*")
100-
c.Assert(spec.Match(plumbing.ReferenceName("refs/tag/foo")), Equals, false)
101-
c.Assert(spec.Match(plumbing.ReferenceName("refs/heads/foo")), Equals, true)
99+
tests := map[string]map[string]bool{
100+
"refs/heads/*:refs/remotes/origin/*": {
101+
"refs/tag/foo": false,
102+
"refs/heads/foo": true,
103+
},
104+
"refs/heads/*bc:refs/remotes/origin/*bc": {
105+
"refs/heads/abc": true,
106+
"refs/heads/bc": true,
107+
"refs/heads/abx": false,
108+
},
109+
"refs/heads/a*c:refs/remotes/origin/a*c": {
110+
"refs/heads/abc": true,
111+
"refs/heads/ac": true,
112+
"refs/heads/abx": false,
113+
},
114+
"refs/heads/ab*:refs/remotes/origin/ab*": {
115+
"refs/heads/abc": true,
116+
"refs/heads/ab": true,
117+
"refs/heads/xbc": false,
118+
},
119+
}
120+
121+
for specStr, data := range tests {
122+
spec := RefSpec(specStr)
123+
for ref, matches := range data {
124+
c.Assert(spec.Match(plumbing.ReferenceName(ref)),
125+
Equals,
126+
matches,
127+
Commentf("while matching spec %q against ref %q", specStr, ref),
128+
)
129+
}
130+
}
102131
}
103132

104133
func (s *RefSpecSuite) TestRefSpecDst(c *C) {
@@ -107,14 +136,50 @@ func (s *RefSpecSuite) TestRefSpecDst(c *C) {
107136
spec.Dst(plumbing.ReferenceName("refs/heads/master")).String(), Equals,
108137
"refs/remotes/origin/master",
109138
)
139+
140+
spec = RefSpec("refs/heads/*bc:refs/remotes/origin/*bc")
141+
c.Assert(spec.Match(plumbing.ReferenceName("refs/heads/abc")), Equals, true)
142+
c.Assert(spec.Match(plumbing.ReferenceName("refs/heads/bc")), Equals, true)
143+
c.Assert(spec.Match(plumbing.ReferenceName("refs/heads/abx")), Equals, false)
144+
145+
spec = RefSpec("refs/heads/a*c:refs/remotes/origin/a*c")
146+
c.Assert(spec.Match(plumbing.ReferenceName("refs/heads/abc")), Equals, true)
147+
c.Assert(spec.Match(plumbing.ReferenceName("refs/heads/ac")), Equals, true)
148+
c.Assert(spec.Match(plumbing.ReferenceName("refs/heads/abx")), Equals, false)
149+
150+
spec = RefSpec("refs/heads/ab*:refs/remotes/origin/ab*")
151+
c.Assert(spec.Match(plumbing.ReferenceName("refs/heads/abc")), Equals, true)
152+
c.Assert(spec.Match(plumbing.ReferenceName("refs/heads/xbc")), Equals, false)
110153
}
111154

112155
func (s *RefSpecSuite) TestRefSpecDstBlob(c *C) {
113-
spec := RefSpec("refs/heads/*:refs/remotes/origin/*")
114-
c.Assert(
115-
spec.Dst(plumbing.ReferenceName("refs/heads/foo")).String(), Equals,
116-
"refs/remotes/origin/foo",
117-
)
156+
ref := "refs/heads/abc"
157+
tests := map[string]string{
158+
"refs/heads/*:refs/remotes/origin/*": "refs/remotes/origin/abc",
159+
"refs/heads/*bc:refs/remotes/origin/*": "refs/remotes/origin/a",
160+
"refs/heads/*bc:refs/remotes/origin/*bc": "refs/remotes/origin/abc",
161+
"refs/heads/a*c:refs/remotes/origin/*": "refs/remotes/origin/b",
162+
"refs/heads/a*c:refs/remotes/origin/a*c": "refs/remotes/origin/abc",
163+
"refs/heads/ab*:refs/remotes/origin/*": "refs/remotes/origin/c",
164+
"refs/heads/ab*:refs/remotes/origin/ab*": "refs/remotes/origin/abc",
165+
"refs/heads/*abc:refs/remotes/origin/*abc": "refs/remotes/origin/abc",
166+
"refs/heads/abc*:refs/remotes/origin/abc*": "refs/remotes/origin/abc",
167+
// for these two cases, git specifically logs:
168+
// error: * Ignoring funny ref 'refs/remotes/origin/' locally
169+
// and ignores the ref; go-git does not currently do this validation,
170+
// but probably should.
171+
// "refs/heads/*abc:refs/remotes/origin/*": "",
172+
// "refs/heads/abc*:refs/remotes/origin/*": "",
173+
}
174+
175+
for specStr, dst := range tests {
176+
spec := RefSpec(specStr)
177+
c.Assert(spec.Dst(plumbing.ReferenceName(ref)).String(),
178+
Equals,
179+
dst,
180+
Commentf("while getting dst from spec %q with ref %q", specStr, ref),
181+
)
182+
}
118183
}
119184
func (s *RefSpecSuite) TestMatchAny(c *C) {
120185
specs := []RefSpec{

0 commit comments

Comments
 (0)