Merge pull request #14 from alcortesm/fix-file-iterator-gorutine-leak

Fix commit.File() gorutine leak

3 files changed, 217 insertions, 10 deletions

diff --git a/commit.go b/commit.go
index 1dd09ab..ea6419f 100644
--- a/commit.go
+++ b/commit.go
@@ -3,7 +3,6 @@ package git
 import (
 	"bufio"
 	"bytes"
-	"errors"
 	"fmt"
 	"io"
 	"sort"
@@ -11,9 +10,6 @@ import (
 	"gopkg.in/src-d/go-git.v2/core"
 )
 
-// New errors defined by this package.
-var ErrFileNotFound = errors.New("file not found")
-
 type Hash core.Hash
 
 // Commit points to a single tree, marking it as what the project looked like
@@ -59,12 +55,7 @@ func (c *Commit) NumParents() int {
 // nil error if the file exists. If the file does not exists, it returns
 // a nil file and the ErrFileNotFound error.
 func (c *Commit) File(path string) (file *File, err error) {
-	for file := range c.Tree().Files() {
-		if file.Name == path {
-			return file, nil
-		}
-	}
-	return nil, ErrFileNotFound
+	return c.Tree().File(path)
 }
 
 // Decode transform an core.Object into a Blob struct
diff --git a/tree.go b/tree.go
index 7b988a1..523baac 100644
--- a/tree.go
+++ b/tree.go
@@ -2,10 +2,12 @@ package git
 
 import (
 	"bufio"
+	"errors"
 	"io"
 	"os"
 	"path/filepath"
 	"strconv"
+	"strings"
 
 	"gopkg.in/src-d/go-git.v2/core"
 )
@@ -26,6 +28,86 @@ type TreeEntry struct {
 	Hash core.Hash
 }
 
+// New errors defined by this package.
+var ErrFileNotFound = errors.New("file not found")
+
+// File returns the hash of the file identified by the `path` argument.
+// The path is interpreted as relative to the tree receiver.
+func (t *Tree) File(path string) (*File, error) {
+	hash, err := t.findHash(path)
+	if err != nil {
+		return nil, ErrFileNotFound
+	}
+
+	obj, ok := t.r.Storage.Get(*hash)
+	if !ok {
+		return nil, ErrFileNotFound // a git submodule
+	}
+
+	if obj.Type() != core.BlobObject {
+		return nil, ErrFileNotFound // a directory
+	}
+
+	blob := &Blob{}
+	blob.Decode(obj)
+
+	return &File{Name: path, Reader: blob.Reader(), Hash: *hash}, nil
+}
+
+func (t *Tree) findHash(path string) (*core.Hash, error) {
+	pathParts := strings.Split(path, "/")
+
+	var tree *Tree
+	var err error
+	for tree = t; len(pathParts) > 1; pathParts = pathParts[1:] {
+		if tree, err = tree.dir(pathParts[0]); err != nil {
+			return nil, err
+		}
+	}
+
+	entry, err := tree.entry(pathParts[0])
+	if err != nil {
+		return nil, err
+	}
+
+	return &entry.Hash, nil
+}
+
+var errDirNotFound = errors.New("directory not found")
+
+func (t *Tree) dir(baseName string) (*Tree, error) {
+	entry, err := t.entry(baseName)
+	if err != nil {
+		return nil, errDirNotFound
+	}
+
+	obj, ok := t.r.Storage.Get(entry.Hash)
+	if !ok { // git submodule
+		return nil, errDirNotFound
+	}
+
+	if obj.Type() != core.TreeObject {
+		return nil, errDirNotFound // a file
+	}
+
+	tree := &Tree{r: t.r}
+	tree.Decode(obj)
+
+	return tree, nil
+}
+
+var errEntryNotFound = errors.New("entry not found")
+
+func (t *Tree) entry(baseName string) (*TreeEntry, error) {
+	for _, entry := range t.Entries {
+		if entry.Name == baseName {
+			return &entry, nil
+		}
+	}
+
+	return nil, errEntryNotFound
+}
+
 func (t *Tree) Files() chan *File {
 	ch := make(chan *File, 1)
 
diff --git a/tree_test.go b/tree_test.go
new file mode 100644
index 0000000..049161c
--- /dev/null
+++ b/tree_test.go
@@ -0,0 +1,134 @@
+package git
+
+import (
+	"os"
+
+	"gopkg.in/src-d/go-git.v2/core"
+	"gopkg.in/src-d/go-git.v2/formats/packfile"
+
+	. "gopkg.in/check.v1"
+)
+
+type SuiteTree struct {
+	repos map[string]*Repository
+}
+
+var _ = Suite(&SuiteTree{})
+
+// create the repositories of the fixtures
+func (s *SuiteTree) SetUpSuite(c *C) {
+	fixtureRepos := [...]struct {
+		url      string
+		packfile string
+	}{
+		{"https://github.com/tyba/git-fixture.git", "formats/packfile/fixtures/git-fixture.ofs-delta"},
+		{"https://github.com/cpcs499/Final_Pres_P.git", "formats/packfile/fixtures/Final_Pres_P.ofs-delta"},
+		{"https://github.com/jamesob/desk.git", "formats/packfile/fixtures/jamesob-desk.pack"},
+		{"https://github.com/spinnaker/spinnaker.git", "formats/packfile/fixtures/spinnaker-spinnaker.pack"},
+		{"https://github.com/alcortesm/binary-relations.git", "formats/packfile/fixtures/alcortesm-binary-relations.pack"},
+	}
+	s.repos = make(map[string]*Repository, 0)
+	for _, fixRepo := range fixtureRepos {
+		s.repos[fixRepo.url] = NewPlainRepository()
+
+		d, err := os.Open(fixRepo.packfile)
+		c.Assert(err, IsNil)
+
+		r := packfile.NewReader(d)
+		r.Format = packfile.OFSDeltaFormat // TODO: how to know the format of a pack file ahead of time?
+
+		_, err = r.Read(s.repos[fixRepo.url].Storage)
+		c.Assert(err, IsNil)
+
+		c.Assert(d.Close(), IsNil)
+	}
+}
+
+func (s *SuiteTree) TestFile(c *C) {
+	for i, t := range []struct {
+		repo     string // the repo name as in localRepos
+		commit   string // the commit to search for the file
+		path     string // the path of the file to find
+		blobHash string // expected hash of the returned file
+		found    bool   // expected found value
+	}{
+		// use git ls-tree commit to get the hash of the blobs
+		{"https://github.com/tyba/git-fixture.git", "b029517f6300c2da0f4b651b8642506cd6aaf45d", "not-found",
+			"", false},
+		{"https://github.com/tyba/git-fixture.git", "b029517f6300c2da0f4b651b8642506cd6aaf45d", ".gitignore",
+			"32858aad3c383ed1ff0a0f9bdf231d54a00c9e88", true},
+		{"https://github.com/tyba/git-fixture.git", "b029517f6300c2da0f4b651b8642506cd6aaf45d", "LICENSE",
+			"c192bd6a24ea1ab01d78686e417c8bdc7c3d197f", true},
+
+		{"https://github.com/tyba/git-fixture.git", "6ecf0ef2c2dffb796033e5a02219af86ec6584e5", "not-found",
+			"", false},
+		{"https://github.com/tyba/git-fixture.git", "6ecf0ef2c2dffb796033e5a02219af86ec6584e5", ".gitignore",
+			"32858aad3c383ed1ff0a0f9bdf231d54a00c9e88", true},
+		{"https://github.com/tyba/git-fixture.git", "6ecf0ef2c2dffb796033e5a02219af86ec6584e5", "binary.jpg",
+			"d5c0f4ab811897cadf03aec358ae60d21f91c50d", true},
+		{"https://github.com/tyba/git-fixture.git", "6ecf0ef2c2dffb796033e5a02219af86ec6584e5", "LICENSE",
+			"c192bd6a24ea1ab01d78686e417c8bdc7c3d197f", true},
+
+		{"https://github.com/tyba/git-fixture.git", "35e85108805c84807bc66a02d91535e1e24b38b9", "binary.jpg",
+			"d5c0f4ab811897cadf03aec358ae60d21f91c50d", true},
+		{"https://github.com/tyba/git-fixture.git", "b029517f6300c2da0f4b651b8642506cd6aaf45d", "binary.jpg",
+			"", false},
+
+		{"https://github.com/tyba/git-fixture.git", "6ecf0ef2c2dffb796033e5a02219af86ec6584e5", "CHANGELOG",
+			"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", true},
+		{"https://github.com/tyba/git-fixture.git", "1669dce138d9b841a518c64b10914d88f5e488ea", "CHANGELOG",
+			"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", true},
+		{"https://github.com/tyba/git-fixture.git", "a5b8b09e2f8fcb0bb99d3ccb0958157b40890d69", "CHANGELOG",
+			"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", true},
+		{"https://github.com/tyba/git-fixture.git", "35e85108805c84807bc66a02d91535e1e24b38b9", "CHANGELOG",
+			"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", false},
+		{"https://github.com/tyba/git-fixture.git", "b8e471f58bcbca63b07bda20e428190409c2db47", "CHANGELOG",
+			"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", true},
+		{"https://github.com/tyba/git-fixture.git", "b029517f6300c2da0f4b651b8642506cd6aaf45d", "CHANGELOG",
+			"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", false},
+
+		// git submodule
+		{"https://github.com/cpcs499/Final_Pres_P.git", "70bade703ce556c2c7391a8065c45c943e8b6bc3", "Final",
+			"", false},
+		{"https://github.com/cpcs499/Final_Pres_P.git", "70bade703ce556c2c7391a8065c45c943e8b6bc3", "Final/not-found",
+			"", false},
+
+		{"https://github.com/jamesob/desk.git", "d4edaf0e8101fcea437ebd982d899fe2cc0f9f7b", "LICENSE",
+			"49c45e6cc893d6f5ebd5c9343fe4492360f339bf", true},
+		{"https://github.com/jamesob/desk.git", "d4edaf0e8101fcea437ebd982d899fe2cc0f9f7b", "examples",
+			"", false},
+		{"https://github.com/jamesob/desk.git", "d4edaf0e8101fcea437ebd982d899fe2cc0f9f7b", "examples/desk.sh",
+			"d9c7751138824cd2d539c23d5afe3f9d29836854", true},
+		{"https://github.com/jamesob/desk.git", "d4edaf0e8101fcea437ebd982d899fe2cc0f9f7b", "examples/not-found",
+			"", false},
+		{"https://github.com/jamesob/desk.git", "d4edaf0e8101fcea437ebd982d899fe2cc0f9f7b", "test/bashrc",
+			"e69de29bb2d1d6434b8b29ae775ad8c2e48c5391", true},
+		{"https://github.com/jamesob/desk.git", "d4edaf0e8101fcea437ebd982d899fe2cc0f9f7b", "test/not-found",
+			"", false},
+
+		{"https://github.com/spinnaker/spinnaker.git", "b32b2aecae2cfca4840dd480f8082da206a538da", "etc/apache2/sites-available/spinnaker.conf",
+			"1d452c616be4fb16d2cc6b8a7e7a2208a6e64d2d", true},
+
+		{"https://github.com/alcortesm/binary-relations.git", "c44b5176e99085c8fe36fa27b045590a7b9d34c9", "Makefile",
+			"2dd2ad8c14de6612ed15813679a6554bad99330b", true},
+		{"https://github.com/alcortesm/binary-relations.git", "c44b5176e99085c8fe36fa27b045590a7b9d34c9", "src/binrels",
+			"", false},
+		{"https://github.com/alcortesm/binary-relations.git", "c44b5176e99085c8fe36fa27b045590a7b9d34c9", "src/map-slice",
+			"", false},
+		{"https://github.com/alcortesm/binary-relations.git", "c44b5176e99085c8fe36fa27b045590a7b9d34c9", "src/map-slice/map-slice.go",
+			"12431e98381dd5097e1a19fe53429c72ef1f328e", true},
+		{"https://github.com/alcortesm/binary-relations.git", "c44b5176e99085c8fe36fa27b045590a7b9d34c9", "src/map-slice/map-slice.go/not-found",
+			"", false},
+	} {
+		commit, err := s.repos[t.repo].Commit(core.NewHash(t.commit))
+		c.Assert(err, IsNil, Commentf("subtest %d: %v (%s)", i, err, t.commit))
+
+		tree := commit.Tree()
+		file, err := tree.File(t.path)
+		found := err == nil
+		c.Assert(found, Equals, t.found, Commentf("subtest %d, path=%s, commit=%s", i, t.path, t.commit))
+		if found {
+			c.Assert(file.Hash.String(), Equals, t.blobHash, Commentf("subtest %d, commit=%s, path=%s", i, t.commit, t.path))
+		}
+	}
+}