Merge pull request #1146 from novas0x2a/fix-tag-oid
improve ResolveRevision's Ref lookup path
diff --git a/repository.go b/repository.go
index e5b12b0..a94dc2f 100644
--- a/repository.go
+++ b/repository.go
@@ -1306,16 +1306,6 @@
return &Worktree{r: r, Filesystem: r.wt}, nil
}
-func countTrue(vals ...bool) int {
- sum := 0
- for _, v := range vals {
- if v {
- sum++
- }
- }
- return sum
-}
-
// ResolveRevision resolves revision to corresponding hash. It will always
// resolve to a commit hash, not a tree or annotated tag.
//
@@ -1336,54 +1326,57 @@
switch item.(type) {
case revision.Ref:
revisionRef := item.(revision.Ref)
- var ref *plumbing.Reference
- var hashCommit, refCommit, tagCommit *object.Commit
- var rErr, hErr, tErr error
+
+ var tryHashes []plumbing.Hash
+
+ maybeHash := plumbing.NewHash(string(revisionRef))
+
+ if !maybeHash.IsZero() {
+ tryHashes = append(tryHashes, maybeHash)
+ }
for _, rule := range append([]string{"%s"}, plumbing.RefRevParseRules...) {
- ref, err = storer.ResolveReference(r.Storer, plumbing.ReferenceName(fmt.Sprintf(rule, revisionRef)))
+ ref, err := storer.ResolveReference(r.Storer, plumbing.ReferenceName(fmt.Sprintf(rule, revisionRef)))
if err == nil {
+ tryHashes = append(tryHashes, ref.Hash())
break
}
}
- if ref != nil {
- tag, tObjErr := r.TagObject(ref.Hash())
- if tObjErr != nil {
- tErr = tObjErr
- } else {
- tagCommit, tErr = tag.Commit()
+ // in ambiguous cases, `git rev-parse` will emit a warning, but
+ // will always return the oid in preference to a ref; we don't have
+ // the ability to emit a warning here, so (for speed purposes)
+ // don't bother to detect the ambiguity either, just return in the
+ // priority that git would.
+ gotOne := false
+ for _, hash := range tryHashes {
+ commitObj, err := r.CommitObject(hash)
+ if err == nil {
+ commit = commitObj
+ gotOne = true
+ break
}
- refCommit, rErr = r.CommitObject(ref.Hash())
- } else {
- rErr = plumbing.ErrReferenceNotFound
- tErr = plumbing.ErrReferenceNotFound
+
+ tagObj, err := r.TagObject(hash)
+ if err == nil {
+ // If the tag target lookup fails here, this most likely
+ // represents some sort of repo corruption, so let the
+ // error bubble up.
+ tagCommit, err := tagObj.Commit()
+ if err != nil {
+ return &plumbing.ZeroHash, err
+ }
+ commit = tagCommit
+ gotOne = true
+ break
+ }
}
- maybeHash := plumbing.NewHash(string(revisionRef)).String() == string(revisionRef)
- if maybeHash {
- hashCommit, hErr = r.CommitObject(plumbing.NewHash(string(revisionRef)))
- } else {
- hErr = plumbing.ErrReferenceNotFound
- }
-
- isTag := tErr == nil
- isCommit := rErr == nil
- isHash := hErr == nil
-
- switch {
- case countTrue(isTag, isCommit, isHash) > 1:
- return &plumbing.ZeroHash, fmt.Errorf(`refname "%s" is ambiguous`, revisionRef)
- case isTag:
- commit = tagCommit
- case isCommit:
- commit = refCommit
- case isHash:
- commit = hashCommit
- default:
+ if !gotOne {
return &plumbing.ZeroHash, plumbing.ErrReferenceNotFound
}
+
case revision.CaretPath:
depth := item.(revision.CaretPath).Depth
diff --git a/repository_test.go b/repository_test.go
index ccbe29b..0148c78 100644
--- a/repository_test.go
+++ b/repository_test.go
@@ -2415,7 +2415,7 @@
for rev, hash := range datas {
h, err := r.ResolveRevision(plumbing.Revision(rev))
- c.Assert(err, IsNil)
+ c.Assert(err, IsNil, Commentf("while checking %s", rev))
c.Check(h.String(), Equals, hash, Commentf("while checking %s", rev))
}
}
@@ -2427,13 +2427,14 @@
c.Assert(err, IsNil)
datas := map[string]string{
- "refs/tags/annotated-tag": "f7b877701fbf855b44c0a9e86f3fdce2c298b07f",
+ "refs/tags/annotated-tag": "f7b877701fbf855b44c0a9e86f3fdce2c298b07f",
+ "b742a2a9fa0afcfa9a6fad080980fbc26b007c69": "f7b877701fbf855b44c0a9e86f3fdce2c298b07f",
}
for rev, hash := range datas {
h, err := r.ResolveRevision(plumbing.Revision(rev))
- c.Assert(err, IsNil)
+ c.Assert(err, IsNil, Commentf("while checking %s", rev))
c.Check(h.String(), Equals, hash, Commentf("while checking %s", rev))
}
}
@@ -2459,12 +2460,11 @@
"HEAD^3": `Revision invalid : "3" found must be 0, 1 or 2 after "^"`,
"HEAD^{/whatever}": `No commit message match regexp : "whatever"`,
"4e1243bd22c66e76c2ba9eddc1f91394e57f9f83": "reference not found",
- "918c48b83bd081e863dbe1b80f8998f058cd8294": `refname "918c48b83bd081e863dbe1b80f8998f058cd8294" is ambiguous`,
}
for rev, rerr := range datas {
_, err := r.ResolveRevision(plumbing.Revision(rev))
-
+ c.Assert(err, NotNil)
c.Assert(err.Error(), Equals, rerr)
}
}
