git: Don't return tag object with Tag, adjust docs for Tag and Tags

I've mainly noticed that in using the current Tag function, that there
were lots of times that I was ignoring the ref or the object, depending
on what I needed. This was evident in the tests as well. As such, I
think it just makes more sense for a singular tag fetcher to return just
a ref, through which the caller may grab the annotation if they need it,
and if it exists.

Also, contrary to the docs on Tags, all tags have a ref, even if they
are annotated. The difference between a lightweight tag and an annotated
tag is the presence of the tag object, which the ref will point to if
the tag is annotated. As such I've adjusted the docs with an example as
to how one can get the annotation for a tag ref through the iterator.

Source: https://git-scm.com/book/en/v2/Git-Internals-Git-References,
tags section.

Signed-off-by: Chris Marchesi <[email protected]>

repository.go

@@ -581,34 +581,52 @@ func (r *Repository) buildTagSignature(tag *object.Tag, signKey *openpgp.Entity)
 	return b.String(), nil
 }
 
-// Tag fetches a tag from the repository. The tag is returned as a raw
-// reference. If the tag is annotated, a non-nil tag object is returned.
-func (r *Repository) Tag(name string) (*plumbing.Reference, *object.Tag, error) {
+// Tag fetches a tag from the repository.
+//
+// If you want to check to see if the tag is an annotated tag, you can call
+// TagObject on the hash of the reference in ForEach:
+//
+//   ref, err := r.Tag("v0.1.0")
+//   if err != nil {
+//     // Handle error
+//   }
+//
+//   obj, err := r.TagObject(ref.Hash())
+//   switch err {
+//   case nil:
+//     // Tag object present
+//   case plumbing.ErrObjectNotFound:
+//     // Not a tag object
+//   default:
+//     // Some other error
+//   }
+//
+func (r *Repository) Tag(name string) (*plumbing.Reference, error) {
 	ref, err := r.Reference(plumbing.ReferenceName(path.Join("refs", "tags", name)), false)
 	if err != nil {
 		if err == plumbing.ErrReferenceNotFound {
 			// Return a friendly error for this one, versus just ReferenceNotFound.
-			return nil, nil, ErrTagNotFound
+			return nil, ErrTagNotFound
 		}
 
-		return nil, nil, err
-	}
-
-	obj, err := r.TagObject(ref.Hash())
-	if err != nil && err != plumbing.ErrObjectNotFound {
-		return nil, nil, err
+		return nil, err
 	}
 
-	return ref, obj, nil
+	return ref, nil
 }
 
 // DeleteTag deletes a tag from the repository.
 func (r *Repository) DeleteTag(name string) error {
-	_, obj, err := r.Tag(name)
+	ref, err := r.Tag(name)
 	if err != nil {
 		return err
 	}
 
+	obj, err := r.TagObject(ref.Hash())
+	if err != nil && err != plumbing.ErrObjectNotFound {
+		return err
+	}
+
 	if err = r.Storer.RemoveReference(plumbing.ReferenceName(path.Join("refs", "tags", name))); err != nil {
 		return err
 	}
@@ -982,9 +1000,31 @@ func (r *Repository) Log(o *LogOptions) (object.CommitIter, error) {
 	return nil, fmt.Errorf("invalid Order=%v", o.Order)
 }
 
-// Tags returns all the References from Tags. This method returns only lightweight
-// tags. Note that not all the tags are lightweight ones. To return annotated tags
-// too, you need to call TagObjects() method.
+// Tags returns all the tag References in a repository.
+//
+// If you want to check to see if the tag is an annotated tag, you can call
+// TagObject on the hash Reference passed in through ForEach:
+//
+//   iter, err := r.Tags()
+//   if err != nil {
+//     // Handle error
+//   }
+//
+//   if err := iter.ForEach(func (ref *plumbing.Reference) error {
+//     obj, err := r.TagObject(ref.Hash())
+//     switch err {
+//     case nil:
+//       // Tag object present
+//     case plumbing.ErrObjectNotFound:
+//       // Not a tag object
+//     default:
+//       // Some other error
+//       return err
+//     }
+//   }); err != nil {
+//     // Handle outer iterator error
+//   }
+//
 func (r *Repository) Tags() (storer.ReferenceIter, error) {
 	refIter, err := r.Storer.IterReferences()
 	if err != nil {

repository_test.go

@@ -1294,9 +1294,8 @@ func (s *RepositorySuite) TestCreateTagLightweight(c *C) {
 	c.Assert(err, IsNil)
 	c.Assert(ref, NotNil)
 
-	actual, obj, err := r.Tag("foobar")
+	actual, err := r.Tag("foobar")
 	c.Assert(err, IsNil)
-	c.Assert(obj, IsNil)
 
 	c.Assert(expected.Hash(), Equals, actual.Hash())
 }
@@ -1338,9 +1337,11 @@ func (s *RepositorySuite) TestCreateTagAnnotated(c *C) {
 	})
 	c.Assert(err, IsNil)
 
-	tag, obj, err := r.Tag("foobar")
+	tag, err := r.Tag("foobar")
+	c.Assert(err, IsNil)
+
+	obj, err := r.TagObject(tag.Hash())
 	c.Assert(err, IsNil)
-	c.Assert(obj, NotNil)
 
 	c.Assert(ref, DeepEquals, tag)
 	c.Assert(obj.Hash, Equals, ref.Hash())
@@ -1412,9 +1413,11 @@ func (s *RepositorySuite) TestCreateTagSigned(c *C) {
 	})
 	c.Assert(err, IsNil)
 
-	_, obj, err := r.Tag("foobar")
+	tag, err := r.Tag("foobar")
+	c.Assert(err, IsNil)
+
+	obj, err := r.TagObject(tag.Hash())
 	c.Assert(err, IsNil)
-	c.Assert(obj, NotNil)
 
 	// Verify the tag.
 	pks := new(bytes.Buffer)
@@ -1472,9 +1475,11 @@ func (s *RepositorySuite) TestCreateTagCanonicalize(c *C) {
 	})
 	c.Assert(err, IsNil)
 
-	_, obj, err := r.Tag("foobar")
+	tag, err := r.Tag("foobar")
+	c.Assert(err, IsNil)
+
+	obj, err := r.TagObject(tag.Hash())
 	c.Assert(err, IsNil)
-	c.Assert(obj, NotNil)
 
 	// Assert the new canonicalized message.
 	c.Assert(obj.Message, Equals, "foo bar baz qux\n\nsome message here\n")
@@ -1505,9 +1510,8 @@ func (s *RepositorySuite) TestTagLightweight(c *C) {
 
 	expected := plumbing.NewHash("f7b877701fbf855b44c0a9e86f3fdce2c298b07f")
 
-	tag, obj, err := r.Tag("lightweight-tag")
+	tag, err := r.Tag("lightweight-tag")
 	c.Assert(err, IsNil)
-	c.Assert(obj, IsNil)
 
 	actual := tag.Hash()
 	c.Assert(expected, Equals, actual)
@@ -1522,34 +1526,11 @@ func (s *RepositorySuite) TestTagLightweightMissingTag(c *C) {
 	err := r.clone(context.Background(), &CloneOptions{URL: url})
 	c.Assert(err, IsNil)
 
-	tag, obj, err := r.Tag("lightweight-tag-tag")
+	tag, err := r.Tag("lightweight-tag-tag")
 	c.Assert(tag, IsNil)
-	c.Assert(obj, IsNil)
 	c.Assert(err, Equals, ErrTagNotFound)
 }
 
-func (s *RepositorySuite) TestTagAnnotated(c *C) {
-	url := s.GetLocalRepositoryURL(
-		fixtures.ByURL("https://github.com/git-fixtures/tags.git").One(),
-	)
-
-	r, _ := Init(memory.NewStorage(), nil)
-	err := r.clone(context.Background(), &CloneOptions{URL: url})
-	c.Assert(err, IsNil)
-
-	tag, obj, err := r.Tag("annotated-tag")
-	c.Assert(err, IsNil)
-	c.Assert(obj, NotNil)
-
-	expectedHash := plumbing.NewHash("b742a2a9fa0afcfa9a6fad080980fbc26b007c69")
-	expectedTarget := plumbing.NewHash("f7b877701fbf855b44c0a9e86f3fdce2c298b07f")
-	actualHash := tag.Hash()
-	c.Assert(expectedHash, Equals, actualHash)
-	c.Assert(obj.Hash, Equals, expectedHash)
-	c.Assert(obj.Type(), Equals, plumbing.TagObject)
-	c.Assert(obj.Target, Equals, expectedTarget)
-}
-
 func (s *RepositorySuite) TestDeleteTag(c *C) {
 	url := s.GetLocalRepositoryURL(
 		fixtures.ByURL("https://github.com/git-fixtures/tags.git").One(),
@@ -1562,7 +1543,7 @@ func (s *RepositorySuite) TestDeleteTag(c *C) {
 	err = r.DeleteTag("lightweight-tag")
 	c.Assert(err, IsNil)
 
-	_, _, err = r.Tag("lightweight-tag")
+	_, err = r.Tag("lightweight-tag")
 	c.Assert(err, Equals, ErrTagNotFound)
 }