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.

Signed-off-by: Mike Lundy <mike@fluffypenguin.org>

2 files changed, 62 insertions, 11 deletions

diff --git a/config/refspec.go b/config/refspec.go
index 391705c..7f486c9 100644
--- a/config/refspec.go
+++ b/config/refspec.go
@@ -99,11 +99,11 @@ func (s RefSpec) matchGlob(n plumbing.ReferenceName) bool {
 
 	var prefix, suffix string
 	prefix = src[0:wildcard]
-	if len(src) < wildcard {
-		suffix = src[wildcard+1 : len(suffix)]
+	if len(src) > wildcard+1 {
+		suffix = src[wildcard+1:]
 	}
 
-	return len(name) > len(prefix)+len(suffix) &&
+	return len(name) >= len(prefix)+len(suffix) &&
 		strings.HasPrefix(name, prefix) &&
 		strings.HasSuffix(name, suffix)
 }
diff --git a/config/refspec_test.go b/config/refspec_test.go
index 675e075..2d2b057 100644
--- a/config/refspec_test.go
+++ b/config/refspec_test.go
@@ -96,9 +96,38 @@ func (s *RefSpecSuite) TestRefSpecMatch(c *C) {
 }
 
 func (s *RefSpecSuite) TestRefSpecMatchGlob(c *C) {
-	spec := RefSpec("refs/heads/*:refs/remotes/origin/*")
-	c.Assert(spec.Match(plumbing.ReferenceName("refs/tag/foo")), Equals, false)
-	c.Assert(spec.Match(plumbing.ReferenceName("refs/heads/foo")), Equals, true)
+	tests := map[string]map[string]bool{
+		"refs/heads/*:refs/remotes/origin/*": {
+			"refs/tag/foo":   false,
+			"refs/heads/foo": true,
+		},
+		"refs/heads/*bc:refs/remotes/origin/*bc": {
+			"refs/heads/abc": true,
+			"refs/heads/bc":  true,
+			"refs/heads/abx": false,
+		},
+		"refs/heads/a*c:refs/remotes/origin/a*c": {
+			"refs/heads/abc": true,
+			"refs/heads/ac":  true,
+			"refs/heads/abx": false,
+		},
+		"refs/heads/ab*:refs/remotes/origin/ab*": {
+			"refs/heads/abc": true,
+			"refs/heads/ab":  true,
+			"refs/heads/xbc": false,
+		},
+	}
+
+	for specStr, data := range tests {
+		spec := RefSpec(specStr)
+		for ref, matches := range data {
+			c.Assert(spec.Match(plumbing.ReferenceName(ref)),
+				Equals,
+				matches,
+				Commentf("while matching spec %q against ref %q", specStr, ref),
+			)
+		}
+	}
 }
 
 func (s *RefSpecSuite) TestRefSpecDst(c *C) {
@@ -110,11 +139,33 @@ func (s *RefSpecSuite) TestRefSpecDst(c *C) {
 }
 
 func (s *RefSpecSuite) TestRefSpecDstBlob(c *C) {
-	spec := RefSpec("refs/heads/*:refs/remotes/origin/*")
-	c.Assert(
-		spec.Dst(plumbing.ReferenceName("refs/heads/foo")).String(), Equals,
-		"refs/remotes/origin/foo",
-	)
+	ref := "refs/heads/abc"
+	tests := map[string]string{
+		"refs/heads/*:refs/remotes/origin/*":       "refs/remotes/origin/abc",
+		"refs/heads/*bc:refs/remotes/origin/*":     "refs/remotes/origin/a",
+		"refs/heads/*bc:refs/remotes/origin/*bc":   "refs/remotes/origin/abc",
+		"refs/heads/a*c:refs/remotes/origin/*":     "refs/remotes/origin/b",
+		"refs/heads/a*c:refs/remotes/origin/a*c":   "refs/remotes/origin/abc",
+		"refs/heads/ab*:refs/remotes/origin/*":     "refs/remotes/origin/c",
+		"refs/heads/ab*:refs/remotes/origin/ab*":   "refs/remotes/origin/abc",
+		"refs/heads/*abc:refs/remotes/origin/*abc": "refs/remotes/origin/abc",
+		"refs/heads/abc*:refs/remotes/origin/abc*": "refs/remotes/origin/abc",
+		// for these two cases, git specifically logs:
+		// error: * Ignoring funny ref 'refs/remotes/origin/' locally
+		// and ignores the ref; go-git does not currently do this validation,
+		// but probably should.
+		// "refs/heads/*abc:refs/remotes/origin/*": "",
+		// "refs/heads/abc*:refs/remotes/origin/*": "",
+	}
+
+	for specStr, dst := range tests {
+		spec := RefSpec(specStr)
+		c.Assert(spec.Dst(plumbing.ReferenceName(ref)).String(),
+			Equals,
+			dst,
+			Commentf("while getting dst from spec %q with ref %q", specStr, ref),
+		)
+	}
 }
 func (s *RefSpecSuite) TestMatchAny(c *C) {
 	specs := []RefSpec{