Merge pull request #319 from ajnavarro/fix/parents-with-path

references.go: fix Parents from commit iterator

Author: smola

references.go

@@ -76,8 +76,10 @@ func walkGraph(result *[]*object.Commit, seen *map[plumbing.Hash]struct{}, curre
 
 	// optimization: don't traverse branches that does not
 	// contain the path.
-	parents := parentsContainingPath(path, current)
-
+	parents, err := parentsContainingPath(path, current)
+	if err != nil {
+		return err
+	}
 	switch len(parents) {
 	// if the path is not found in any of its parents, the path was
 	// created by this commit; we must add it to the revisions list and
@@ -110,18 +112,18 @@ func walkGraph(result *[]*object.Commit, seen *map[plumbing.Hash]struct{}, curre
 	return nil
 }
 
-func parentsContainingPath(path string, c *object.Commit) []*object.Commit {
+func parentsContainingPath(path string, c *object.Commit) ([]*object.Commit, error) {
 	// TODO: benchmark this method making git.object.Commit.parent public instead of using
 	// an iterator
 	var result []*object.Commit
 	iter := c.Parents()
 	for {
 		parent, err := iter.Next()
+		if err == io.EOF {
+			return result, nil
+		}
 		if err != nil {
-			if err == io.EOF {
-				return result
-			}
-			panic("unreachable")
+			return nil, err
 		}
 		if _, err := parent.File(path); err == nil {
 			result = append(result, parent)

references_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/src-d/go-git-fixtures"
 	"gopkg.in/src-d/go-git.v4/plumbing"
 	"gopkg.in/src-d/go-git.v4/plumbing/object"
+	"gopkg.in/src-d/go-git.v4/storage/memory"
 
 	. "gopkg.in/check.v1"
 )
@@ -287,6 +288,26 @@ var referencesTests = [...]struct {
 	*/
 }
 
+func (s *ReferencesSuite) TestObjectNotFoundError(c *C) {
+	h1 := plumbing.NewHash("af2d6a6954d532f8ffb47615169c8fdf9d383a1a")
+	hParent := plumbing.NewHash("1669dce138d9b841a518c64b10914d88f5e488ea")
+
+	url := fixtures.ByURL("https://github.com/git-fixtures/basic.git").One().DotGit().Base()
+	storer := memory.NewStorage()
+	r, err := Clone(storer, nil, &CloneOptions{
+		URL: "file://" + url,
+	})
+	c.Assert(err, IsNil)
+
+	delete(storer.Objects, hParent)
+
+	commit, err := r.CommitObject(h1)
+	c.Assert(err, IsNil)
+
+	_, err = references(commit, "LICENSE")
+	c.Assert(err, Equals, plumbing.ErrObjectNotFound)
+}
+
 func (s *ReferencesSuite) TestRevList(c *C) {
 	for _, t := range referencesTests {
 		r := s.NewRepositoryFromPackfile(fixtures.ByURL(t.repo).One())