ForEach review and Commit.Tree err return

19 files changed, 200 insertions, 192 deletions

diff --git a/commit.go b/commit.go
index 5c9c113..4a4a202 100644
--- a/commit.go
+++ b/commit.go
@@ -30,9 +30,8 @@ type Commit struct {
 }
 
 // Tree returns the Tree from the commit
-func (c *Commit) Tree() *Tree {
-	tree, _ := c.r.Tree(c.tree) // FIXME: Return error as well?
-	return tree
+func (c *Commit) Tree() (*Tree, error) {
+	return c.r.Tree(c.tree)
 }
 
 // Parents return a CommitIter to the parent Commits
@@ -51,8 +50,13 @@ func (c *Commit) NumParents() int {
 // File returns the file with the specified "path" in the commit and a
 // nil error if the file exists. If the file does not exist, it returns
 // a nil file and the ErrFileNotFound error.
-func (c *Commit) File(path string) (file *File, err error) {
-	return c.Tree().File(path)
+func (c *Commit) File(path string) (*File, error) {
+	tree, err := c.Tree()
+	if err != nil {
+		return nil, err
+	}
+
+	return tree.File(path)
 }
 
 // ID returns the object ID of the commit. The returned value will always match
@@ -156,7 +160,7 @@ func (iter *CommitIter) Next() (*Commit, error) {
 
 // ForEach call the cb function for each commit contained on this iter until
 // an error happends or the end of the iter is reached. If ErrStop is sent
-// the iteration is stop but no error is returned
+// the iteration is stop but no error is returned. The iterator is closed.
 func (iter *CommitIter) ForEach(cb func(*Commit) error) error {
 	return iter.ObjectIter.ForEach(func(obj core.Object) error {
 		commit := &Commit{r: iter.r}
diff --git a/commit_test.go b/commit_test.go
index c9a17c0..82133da 100644
--- a/commit_test.go
+++ b/commit_test.go
@@ -9,160 +9,89 @@ import (
 )
 
 type SuiteCommit struct {
-	repos map[string]*Repository
+	BaseSuite
+	Commit *Commit
 }
 
 var _ = Suite(&SuiteCommit{})
 
-// create the repositories of the fixtures
-func (s *SuiteCommit) SetUpSuite(c *C) {
-	commitFixtures := []packedFixture{
-		{"https://github.com/tyba/git-fixture.git", "formats/packfile/fixtures/git-fixture.ofs-delta"},
-	}
-	s.repos = unpackFixtures(c, commitFixtures)
-}
+func (s *SuiteCommit) SetUpTest(c *C) {
+	s.BaseSuite.SetUpTest(c)
 
-var commitIterTests = []struct {
-	repo    string   // the repo name in the test suite's map of fixtures
-	commits []string // the commit hashes to iterate over in the test
-}{
-	{"https://github.com/tyba/git-fixture.git", []string{
-		"6ecf0ef2c2dffb796033e5a02219af86ec6584e5",
-		"918c48b83bd081e863dbe1b80f8998f058cd8294",
-		"af2d6a6954d532f8ffb47615169c8fdf9d383a1a",
-		"1669dce138d9b841a518c64b10914d88f5e488ea",
-		"35e85108805c84807bc66a02d91535e1e24b38b9",
-		"b029517f6300c2da0f4b651b8642506cd6aaf45d",
-		"a5b8b09e2f8fcb0bb99d3ccb0958157b40890d69",
-		"b029517f6300c2da0f4b651b8642506cd6aaf45d", // Intentional duplicate
-		"b8e471f58bcbca63b07bda20e428190409c2db47",
-		"b029517f6300c2da0f4b651b8642506cd6aaf45d"}}, // Intentional duplicate
-}
+	hash := core.NewHash("1669dce138d9b841a518c64b10914d88f5e488ea")
 
-func (s *SuiteCommit) TestIterSlice(c *C) {
-	for i, t := range commitIterTests {
-		r := s.repos[t.repo]
-		iter := NewCommitIter(r, core.NewObjectSliceIter(makeObjectSlice(t.commits, r.s.ObjectStorage())))
-		s.checkIter(c, r, i, iter, t.commits)
-	}
+	var err error
+	s.Commit, err = s.Repository.Commit(hash)
+	c.Assert(err, IsNil)
 }
 
-func (s *SuiteCommit) TestIterLookup(c *C) {
-	for i, t := range commitIterTests {
-		r := s.repos[t.repo]
-		iter := NewCommitIter(r, core.NewObjectLookupIter(r.s.ObjectStorage(), makeHashSlice(t.commits)))
-		s.checkIter(c, r, i, iter, t.commits)
-	}
+func (s *SuiteCommit) TestDecodeNonCommit(c *C) {
+	hash := core.NewHash("9a48f23120e880dfbe41f7c9b7b708e9ee62a492")
+	blob, err := s.Repository.s.ObjectStorage().Get(hash)
+	c.Assert(err, IsNil)
+
+	commit := &Commit{}
+	err = commit.Decode(blob)
+	c.Assert(err, Equals, ErrUnsupportedObject)
 }
 
-func (s *SuiteCommit) checkIter(c *C, r *Repository, subtest int, iter *CommitIter, commits []string) {
-	for k := 0; k < len(commits); k++ {
-		commit, err := iter.Next()
-		c.Assert(err, IsNil, Commentf("subtest %d, iter %d, err=%v", subtest, k, err))
-		c.Assert(commit.Hash.String(), Equals, commits[k], Commentf("subtest %d, iter %d, hash=%v, expected=%s", subtest, k, commit.Hash.String(), commits[k]))
-	}
-	_, err := iter.Next()
-	c.Assert(err, Equals, io.EOF)
+func (s *SuiteCommit) TestType(c *C) {
+	c.Assert(s.Commit.Type(), Equals, core.CommitObject)
 }
 
-func (s *SuiteCommit) TestIterSliceClose(c *C) {
-	for i, t := range commitIterTests {
-		r := s.repos[t.repo]
-		iter := NewCommitIter(r, core.NewObjectSliceIter(makeObjectSlice(t.commits, r.s.ObjectStorage())))
-		s.checkIterClose(c, i, iter)
-	}
+func (s *SuiteCommit) TestTree(c *C) {
+	tree, err := s.Commit.Tree()
+	c.Assert(err, IsNil)
+	c.Assert(tree.ID().String(), Equals, "eba74343e2f15d62adedfd8c883ee0262b5c8021")
 }
 
-func (s *SuiteCommit) TestIterLookupClose(c *C) {
-	for i, t := range commitIterTests {
-		r := s.repos[t.repo]
-		iter := NewCommitIter(r, core.NewObjectLookupIter(r.s.ObjectStorage(), makeHashSlice(t.commits)))
-		s.checkIterClose(c, i, iter)
+func (s *SuiteCommit) TestParents(c *C) {
+	expected := []string{
+		"35e85108805c84807bc66a02d91535e1e24b38b9",
+		"a5b8b09e2f8fcb0bb99d3ccb0958157b40890d69",
 	}
-}
 
-func (s *SuiteCommit) checkIterClose(c *C, subtest int, iter *CommitIter) {
-	iter.Close()
-	_, err := iter.Next()
-	c.Assert(err, Equals, io.EOF, Commentf("subtest %d, close 1, err=%v", subtest, err))
+	var output []string
+	i := s.Commit.Parents()
+	err := i.ForEach(func(commit *Commit) error {
+		output = append(output, commit.ID().String())
+		return nil
+	})
 
-	iter.Close()
-	_, err = iter.Next()
-	c.Assert(err, Equals, io.EOF, Commentf("subtest %d, close 2, err=%v", subtest, err))
+	c.Assert(err, IsNil)
+	c.Assert(output, DeepEquals, expected)
 }
 
-var fileTests = []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},
+func (s *SuiteCommit) TestFile(c *C) {
+	file, err := s.Commit.File("CHANGELOG")
+	c.Assert(err, IsNil)
+	c.Assert(file.Name, Equals, "CHANGELOG")
 }
 
-func (s *SuiteCommit) TestFile(c *C) {
-	for i, t := range fileTests {
-		commit, err := s.repos[t.repo].Commit(core.NewHash(t.commit))
-		c.Assert(err, IsNil, Commentf("subtest %d: %v (%s)", i, err, t.commit))
-
-		file, err := commit.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))
-		}
-	}
+func (s *SuiteCommit) TestNumParents(c *C) {
+	c.Assert(s.Commit.NumParents(), Equals, 2)
 }
 
-func makeObjectSlice(hashes []string, storage core.ObjectStorage) []core.Object {
-	series := make([]core.Object, 0, len(hashes))
-	for _, member := range hashes {
-		obj, err := storage.Get(core.NewHash(member))
-		if err == nil {
-			series = append(series, obj)
-		}
-	}
-	return series
+func (s *SuiteCommit) TestString(c *C) {
+	c.Assert(s.Commit.String(), Equals, ""+
+		"commit 1669dce138d9b841a518c64b10914d88f5e488ea\n"+
+		"Author: Máximo Cuadros Ortiz <mcuadros@gmail.com>\n"+
+		"Date:   2015-03-31 13:48:14 +0200 +0200\n",
+	)
 }
 
-func makeHashSlice(hashes []string) []core.Hash {
-	series := make([]core.Hash, 0, len(hashes))
-	for _, member := range hashes {
-		series = append(series, core.NewHash(member))
-	}
-	return series
+func (s *SuiteCommit) TestCommitIterNext(c *C) {
+	i := s.Commit.Parents()
+
+	commit, err := i.Next()
+	c.Assert(err, IsNil)
+	c.Assert(commit.ID().String(), Equals, "35e85108805c84807bc66a02d91535e1e24b38b9")
+
+	commit, err = i.Next()
+	c.Assert(err, IsNil)
+	c.Assert(commit.ID().String(), Equals, "a5b8b09e2f8fcb0bb99d3ccb0958157b40890d69")
+
+	commit, err = i.Next()
+	c.Assert(err, Equals, io.EOF)
+	c.Assert(commit, IsNil)
 }
diff --git a/common_test.go b/common_test.go
index 6239afe..dbacddd 100644
--- a/common_test.go
+++ b/common_test.go
@@ -18,12 +18,18 @@ import (
 
 func Test(t *testing.T) { TestingT(t) }
 
-type BaseSuite struct{}
+type BaseSuite struct {
+	Repository *Repository
+}
 
 func (s *BaseSuite) SetUpTest(c *C) {
 	clients.InstallProtocol("mock", func(end common.Endpoint) common.GitUploadPackService {
 		return &MockGitUploadPackService{endpoint: end}
 	})
+
+	s.Repository = NewMemoryRepository()
+	err := s.Repository.Clone(&CloneOptions{URL: RepositoryFixture})
+	c.Assert(err, IsNil)
 }
 
 const RepositoryFixture = "mock://formats/packfile/fixtures/git-fixture.ref-delta"
diff --git a/config/config_test.go b/config/config_test.go
new file mode 100644
index 0000000..853e518
--- /dev/null
+++ b/config/config_test.go
@@ -0,0 +1,31 @@
+package config
+
+import . "gopkg.in/check.v1"
+
+type ConfigSuite struct{}
+
+var _ = Suite(&ConfigSuite{})
+
+func (s *ConfigSuite) TestRemoteConfigValidateMissingURL(c *C) {
+	config := &RemoteConfig{Name: "foo"}
+	c.Assert(config.Validate(), Equals, ErrRemoteConfigEmptyURL)
+}
+
+func (s *ConfigSuite) TestRemoteConfigValidateInvalidURL(c *C) {
+	config := &RemoteConfig{Name: "foo", URL: "foo"}
+	c.Assert(config.Validate(), NotNil)
+}
+
+func (s *ConfigSuite) TestRemoteConfigValidateMissingName(c *C) {
+	config := &RemoteConfig{}
+	c.Assert(config.Validate(), Equals, ErrRemoteConfigEmptyName)
+}
+
+func (s *ConfigSuite) TestRemoteConfigValidateDefault(c *C) {
+	config := &RemoteConfig{Name: "foo", URL: "http://foo/bar"}
+	c.Assert(config.Validate(), IsNil)
+
+	fetch := config.Fetch
+	c.Assert(fetch, HasLen, 1)
+	c.Assert(fetch[0].String(), Equals, "+refs/heads/*:refs/remotes/foo/*")
+}
diff --git a/core/object.go b/core/object.go
index d777f4a..9c9a74e 100644
--- a/core/object.go
+++ b/core/object.go
@@ -141,8 +141,10 @@ func (iter *ObjectLookupIter) Next() (Object, error) {
 
 // ForEach call the cb function for each object contained on this iter until
 // an error happends or the end of the iter is reached. If ErrStop is sent
-// the iteration is stop but no error is returned
+// the iteration is stop but no error is returned. The iterator is closed.
 func (iter *ObjectLookupIter) ForEach(cb func(Object) error) error {
+	defer iter.Close()
+
 	for _, hash := range iter.series {
 		obj, err := iter.storage.Get(hash)
 		if err != nil {
@@ -197,8 +199,9 @@ func (iter *ObjectSliceIter) Next() (Object, error) {
 
 // ForEach call the cb function for each object contained on this iter until
 // an error happends or the end of the iter is reached. If ErrStop is sent
-// the iteration is stop but no error is returned
+// the iteration is stop but no error is returned. The iterator is closed.
 func (iter *ObjectSliceIter) ForEach(cb func(Object) error) error {
+	defer iter.Close()
 	for _, o := range iter.series {
 		if err := cb(o); err != nil {
 			if err == ErrStop {
diff --git a/core/reference.go b/core/reference.go
index c33402c..16b60ac 100644
--- a/core/reference.go
+++ b/core/reference.go
@@ -182,8 +182,9 @@ func (iter *ReferenceSliceIter) Next() (*Reference, error) {
 
 // ForEach call the cb function for each reference contained on this iter until
 // an error happends or the end of the iter is reached. If ErrStop is sent
-// the iteration is stop but no error is returned
+// the iteration is stop but no error is returned. The iterator is closed.
 func (iter *ReferenceSliceIter) ForEach(cb func(*Reference) error) error {
+	defer iter.Close()
 	for _, r := range iter.series {
 		if err := cb(r); err != nil {
 			if err == ErrStop {
diff --git a/cshared/commit_cshared.go b/cshared/commit_cshared.go
index e73fb4a..8cb91aa 100644
--- a/cshared/commit_cshared.go
+++ b/cshared/commit_cshared.go
@@ -62,7 +62,11 @@ func c_Commit_Tree(c uint64) uint64 {
 		return IH
 	}
 	commit := obj.(*git.Commit)
-	tree := commit.Tree()
+	tree, err := commit.Tree()
+	if err != nil {
+		return IH
+	}
+
 	tree_handle := RegisterObject(tree)
 	return uint64(tree_handle)
 }
@@ -156,14 +160,14 @@ func c_Commit_References(c uint64, path string) (*C.char, int, int, *C.char) {
 		return nil, 0, ErrorCodeInternal, C.CString(err.Error())
 	}
 	handles := make([]uint64, len(refs))
-	for i, c := range(refs) {
+	for i, c := range refs {
 		handles[i] = uint64(RegisterObject(c))
 	}
 	size := 8 * len(handles)
 	dest := C.malloc(C.size_t(size))
 	header := (*reflect.SliceHeader)(unsafe.Pointer(&handles))
 	header.Len *= 8
-	copy((*[1<<30]byte)(dest)[:], *(*[]byte)(unsafe.Pointer(header)))
+	copy((*[1 << 30]byte)(dest)[:], *(*[]byte)(unsafe.Pointer(header)))
 	return (*C.char)(dest), size / 8, ErrorCodeSuccess, nil
 }
 
diff --git a/examples/basic/main.go b/examples/basic/main.go
index e192cef..85d6a57 100644
--- a/examples/basic/main.go
+++ b/examples/basic/main.go
@@ -35,16 +35,15 @@ func main() {
 	color.Blue("git ls-tree -r HEAD")
 
 	// ... retrieve the tree from the commit
-	tree := commit.Tree()
-	// ... create a tree walker, allows to you intereste all nested trees
-	walker := git.NewTreeWalker(r, tree)
-	walker.ForEach(func(fullpath string, e git.TreeEntry) error {
+	tree, _ := commit.Tree()
+	// ... get the files iterator and print the file
+	tree.Files().ForEach(func(f *git.File) error {
 		// we ignore the tree
-		if e.Mode.Perm() == 0 {
+		if f.Mode.Perm() == 0 {
 			return nil
 		}
 
-		fmt.Printf("100644 blob %s    %s\n", e.Hash, fullpath)
+		fmt.Printf("100644 blob %s    %s\n", f.Hash, f.Name)
 		return nil
 	})
 }
diff --git a/file.go b/file.go
index 58c76d2..ef17c6e 100644
--- a/file.go
+++ b/file.go
@@ -75,13 +75,12 @@ func (iter *FileIter) Next() (*File, error) {
 
 // ForEach call the cb function for each file contained on this iter until
 // an error happends or the end of the iter is reached. If core.ErrStop is sent
-// the iteration is stop but no error is returned
+// the iteration is stop but no error is returned. The iterator is closed.
 func (iter *FileIter) ForEach(cb func(*File) error) error {
-	i := &FileIter{w: *NewTreeWalker(iter.w.r, iter.w.t)}
-	defer i.Close()
+	defer iter.Close()
 
 	for {
-		f, err := i.Next()
+		f, err := iter.Next()
 		if err != nil {
 			if err == io.EOF {
 				return nil
diff --git a/file_test.go b/file_test.go
index e702ce7..e49970a 100644
--- a/file_test.go
+++ b/file_test.go
@@ -9,6 +9,7 @@ import (
 )
 
 type SuiteFile struct {
+	BaseSuite
 	repos map[string]*Repository
 }
 
@@ -53,7 +54,10 @@ func (s *SuiteFile) TestIter(c *C) {
 		commit, err := r.Commit(core.NewHash(t.commit))
 		c.Assert(err, IsNil, Commentf("subtest %d: %v (%s)", i, err, t.commit))
 
-		iter := NewFileIter(r, commit.Tree())
+		tree, err := commit.Tree()
+		c.Assert(err, IsNil)
+
+		iter := NewFileIter(r, tree)
 		for k := 0; k < len(t.files); k++ {
 			exp := t.files[k]
 			file, err := iter.Next()
@@ -186,7 +190,10 @@ func (s *SuiteFile) TestIgnoreEmptyDirEntries(c *C) {
 		commit, err := s.repos[t.repo].Commit(core.NewHash(t.commit))
 		c.Assert(err, IsNil, Commentf("subtest %d: %v (%s)", i, err, t.commit))
 
-		iter := commit.Tree().Files()
+		tree, err := commit.Tree()
+		c.Assert(err, IsNil)
+
+		iter := tree.Files()
 		defer iter.Close()
 		for file, err := iter.Next(); err == nil; file, err = iter.Next() {
 			_, _ = file.Contents()
@@ -194,3 +201,38 @@ func (s *SuiteFile) TestIgnoreEmptyDirEntries(c *C) {
 		}
 	}
 }
+
+func (s *SuiteFile) TestFileIter(c *C) {
+	hash := core.NewHash("1669dce138d9b841a518c64b10914d88f5e488ea")
+
+	commit, err := s.Repository.Commit(hash)
+	c.Assert(err, IsNil)
+
+	tree, err := commit.Tree()
+
+	expected := []string{
+		".gitignore",
+		"CHANGELOG",
+		"LICENSE",
+		"binary.jpg",
+	}
+
+	var count int
+	i := tree.Files()
+	i.ForEach(func(f *File) error {
+		c.Assert(f.Name, Equals, expected[count])
+		count++
+		return nil
+	})
+
+	c.Assert(count, Equals, 4)
+
+	count = 0
+	i = tree.Files()
+	i.ForEach(func(f *File) error {
+		count++
+		return core.ErrStop
+	})
+
+	c.Assert(count, Equals, 1)
+}
diff --git a/objects_test.go b/objects_test.go
index 7d86b1c..b36bb85 100644
--- a/objects_test.go
+++ b/objects_test.go
@@ -31,7 +31,10 @@ func (s *ObjectsSuite) TestNewCommit(c *C) {
 
 	c.Assert(commit.Hash, Equals, commit.ID())
 	c.Assert(commit.Hash.String(), Equals, "a5b8b09e2f8fcb0bb99d3ccb0958157b40890d69")
-	c.Assert(commit.Tree().Hash.String(), Equals, "c2d30fa8ef288618f65f6eed6e168e0d514886f4")
+
+	tree, err := commit.Tree()
+	c.Assert(err, IsNil)
+	c.Assert(tree.Hash.String(), Equals, "c2d30fa8ef288618f65f6eed6e168e0d514886f4")
 
 	parents := commit.Parents()
 	parentCommit, err := parents.Next()
diff --git a/repository_test.go b/repository_test.go
index 6ad0d1e..ce53ead 100644
--- a/repository_test.go
+++ b/repository_test.go
@@ -209,7 +209,11 @@ func (s *RepositorySuite) TestCommit(c *C) {
 	c.Assert(commit.Hash, Equals, commit.ID())
 	c.Assert(commit.Hash, Equals, hash)
 	c.Assert(commit.Type(), Equals, core.CommitObject)
-	c.Assert(commit.Tree().Hash.IsZero(), Equals, false)
+
+	tree, err := commit.Tree()
+	c.Assert(err, IsNil)
+	c.Assert(tree.Hash.IsZero(), Equals, false)
+
 	c.Assert(commit.Author.Email, Equals, "daniel@lordran.local")
 }
 
diff --git a/tag.go b/tag.go
index 16600fd..fec2abf 100644
--- a/tag.go
+++ b/tag.go
@@ -125,7 +125,7 @@ func (t *Tag) Tree() (*Tree, error) {
 		if err != nil {
 			return nil, err
 		}
-		return commit.Tree(), nil
+		return commit.Tree()
 	case core.TreeObject:
 		return t.r.Tree(t.Target)
 	default:
@@ -184,7 +184,7 @@ func (iter *TagIter) Next() (*Tag, error) {
 
 // ForEach call the cb function for each tag contained on this iter until
 // an error happends or the end of the iter is reached. If ErrStop is sent
-// the iteration is stop but no error is returned
+// the iteration is stop but no error is returned. The iterator is closed.
 func (iter *TagIter) ForEach(cb func(*Tag) error) error {
 	return iter.ObjectIter.ForEach(func(obj core.Object) error {
 		tag := &Tag{r: iter.r}
diff --git a/tree.go b/tree.go
index 150cdab..cb25a0d 100644
--- a/tree.go
+++ b/tree.go
@@ -243,13 +243,12 @@ func (iter *TreeIter) Next() (*Tree, error) {
 
 // ForEach call the cb function for each tree contained on this iter until
 // an error happends or the end of the iter is reached. If core.ErrStop is sent
-// the iteration is stop but no error is returned
+// the iteration is stop but no error is returned. The iterator is closed.
 func (iter *TreeIter) ForEach(cb func(*Tree) error) error {
-	i := &TreeIter{w: *NewTreeWalker(iter.w.r, iter.w.t)}
-	defer i.Close()
+	defer iter.Close()
 
 	for {
-		t, err := i.Next()
+		t, err := iter.Next()
 		if err != nil {
 			if err == io.EOF {
 				return nil
diff --git a/tree_diff_test.go b/tree_diff_test.go
index 40c7f94..7733c90 100644
--- a/tree_diff_test.go
+++ b/tree_diff_test.go
@@ -394,5 +394,5 @@ func tree(repo *Repository, commitHashStr string) (*Tree, error) {
 		return nil, err
 	}
 
-	return commit.Tree(), nil
+	return commit.Tree()
 }
diff --git a/tree_test.go b/tree_test.go
index 92ae156..e27d4ad 100644
--- a/tree_test.go
+++ b/tree_test.go
@@ -188,7 +188,9 @@ func (s *SuiteTree) TestFile(c *C) {
 		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()
+		tree, err := commit.Tree()
+		c.Assert(err, IsNil)
+
 		file, err := tree.File(t.path)
 		found := err == nil
 
@@ -260,7 +262,9 @@ func (s *SuiteTree) TestFiles(c *C) {
 		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()
+		tree, err := commit.Tree()
+		c.Assert(err, IsNil)
+
 		var output []string
 		iter := tree.Files()
 		defer iter.Close()
diff --git a/tree_walker.go b/tree_walker.go
index 1692c2f..5568e1b 100644
--- a/tree_walker.go
+++ b/tree_walker.go
@@ -3,8 +3,6 @@ package git
 import (
 	"io"
 	"path"
-
-	"gopkg.in/src-d/go-git.v4/core"
 )
 
 const (
@@ -93,27 +91,6 @@ func (w *TreeWalker) Next() (name string, entry TreeEntry, obj Object, err error
 	return
 }
 
-func (w *TreeWalker) ForEach(cb func(fullpath string, e TreeEntry) error) error {
-	for {
-		path, e, _, err := w.Next()
-		if err != nil {
-			if err == io.EOF {
-				return nil
-			}
-
-			return err
-		}
-
-		if err := cb(path, e); err != nil {
-			if err == core.ErrStop {
-				return nil
-			}
-
-			return err
-		}
-	}
-}
-
 // Tree returns the tree that the tree walker most recently operated on.
 func (w *TreeWalker) Tree() *Tree {
 	current := len(w.stack) - 1
diff --git a/tree_walker_test.go b/tree_walker_test.go
index 28e41f2..3b92edf 100644
--- a/tree_walker_test.go
+++ b/tree_walker_test.go
@@ -99,7 +99,10 @@ func (s *SuiteTreeWalker) TestNext(c *C) {
 		commit, err := r.Commit(core.NewHash(t.commit))
 		c.Assert(err, IsNil, Commentf("subtest %d: %v (%s)", i, err, t.commit))
 
-		walker := NewTreeWalker(r, commit.Tree())
+		tree, err := commit.Tree()
+		c.Assert(err, IsNil)
+
+		walker := NewTreeWalker(r, tree)
 		for k := 0; k < len(t.objs); k++ {
 			info := t.objs[k]
 			mode, err := strconv.ParseInt(info.Mode, 8, 32)
diff --git a/utils/difftree/difftree_test.go b/utils/difftree/difftree_test.go
index 9f874e4..767807c 100644
--- a/utils/difftree/difftree_test.go
+++ b/utils/difftree/difftree_test.go
@@ -400,5 +400,5 @@ func tree(repo *git.Repository, commitHashStr string) (*git.Tree, error) {
 		return nil, err
 	}
 
-	return commit.Tree(), nil
+	return commit.Tree()
 }