Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

Commit abe470a

Browse files
committed
Merge pull request #14 from alcortesm/fix-file-iterator-gorutine-leak
Fix commit.File() gorutine leak
2 parents 37cc5cf + 5ec2de7 commit abe470a

File tree

3 files changed

+217
-10
lines changed

3 files changed

+217
-10
lines changed

commit.go

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,13 @@ package git
33
import (
44
"bufio"
55
"bytes"
6-
"errors"
76
"fmt"
87
"io"
98
"sort"
109

1110
"gopkg.in/src-d/go-git.v2/core"
1211
)
1312

14-
// New errors defined by this package.
15-
var ErrFileNotFound = errors.New("file not found")
16-
1713
type Hash core.Hash
1814

1915
// Commit points to a single tree, marking it as what the project looked like
@@ -59,12 +55,7 @@ func (c *Commit) NumParents() int {
5955
// nil error if the file exists. If the file does not exists, it returns
6056
// a nil file and the ErrFileNotFound error.
6157
func (c *Commit) File(path string) (file *File, err error) {
62-
for file := range c.Tree().Files() {
63-
if file.Name == path {
64-
return file, nil
65-
}
66-
}
67-
return nil, ErrFileNotFound
58+
return c.Tree().File(path)
6859
}
6960

7061
// Decode transform an core.Object into a Blob struct

tree.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ package git
22

33
import (
44
"bufio"
5+
"errors"
56
"io"
67
"os"
78
"path/filepath"
89
"strconv"
10+
"strings"
911

1012
"gopkg.in/src-d/go-git.v2/core"
1113
)
@@ -26,6 +28,86 @@ type TreeEntry struct {
2628
Hash core.Hash
2729
}
2830

31+
// New errors defined by this package.
32+
var ErrFileNotFound = errors.New("file not found")
33+
34+
// File returns the hash of the file identified by the `path` argument.
35+
// The path is interpreted as relative to the tree receiver.
36+
func (t *Tree) File(path string) (*File, error) {
37+
hash, err := t.findHash(path)
38+
if err != nil {
39+
return nil, ErrFileNotFound
40+
}
41+
42+
obj, ok := t.r.Storage.Get(*hash)
43+
if !ok {
44+
return nil, ErrFileNotFound // a git submodule
45+
}
46+
47+
if obj.Type() != core.BlobObject {
48+
return nil, ErrFileNotFound // a directory
49+
}
50+
51+
blob := &Blob{}
52+
blob.Decode(obj)
53+
54+
return &File{Name: path, Reader: blob.Reader(), Hash: *hash}, nil
55+
}
56+
57+
func (t *Tree) findHash(path string) (*core.Hash, error) {
58+
pathParts := strings.Split(path, "/")
59+
60+
var tree *Tree
61+
var err error
62+
for tree = t; len(pathParts) > 1; pathParts = pathParts[1:] {
63+
if tree, err = tree.dir(pathParts[0]); err != nil {
64+
return nil, err
65+
}
66+
}
67+
68+
entry, err := tree.entry(pathParts[0])
69+
if err != nil {
70+
return nil, err
71+
}
72+
73+
return &entry.Hash, nil
74+
}
75+
76+
var errDirNotFound = errors.New("directory not found")
77+
78+
func (t *Tree) dir(baseName string) (*Tree, error) {
79+
entry, err := t.entry(baseName)
80+
if err != nil {
81+
return nil, errDirNotFound
82+
}
83+
84+
obj, ok := t.r.Storage.Get(entry.Hash)
85+
if !ok { // git submodule
86+
return nil, errDirNotFound
87+
}
88+
89+
if obj.Type() != core.TreeObject {
90+
return nil, errDirNotFound // a file
91+
}
92+
93+
tree := &Tree{r: t.r}
94+
tree.Decode(obj)
95+
96+
return tree, nil
97+
}
98+
99+
var errEntryNotFound = errors.New("entry not found")
100+
101+
func (t *Tree) entry(baseName string) (*TreeEntry, error) {
102+
for _, entry := range t.Entries {
103+
if entry.Name == baseName {
104+
return &entry, nil
105+
}
106+
}
107+
108+
return nil, errEntryNotFound
109+
}
110+
29111
func (t *Tree) Files() chan *File {
30112
ch := make(chan *File, 1)
31113

tree_test.go

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
package git
2+
3+
import (
4+
"os"
5+
6+
"gopkg.in/src-d/go-git.v2/core"
7+
"gopkg.in/src-d/go-git.v2/formats/packfile"
8+
9+
. "gopkg.in/check.v1"
10+
)
11+
12+
type SuiteTree struct {
13+
repos map[string]*Repository
14+
}
15+
16+
var _ = Suite(&SuiteTree{})
17+
18+
// create the repositories of the fixtures
19+
func (s *SuiteTree) SetUpSuite(c *C) {
20+
fixtureRepos := [...]struct {
21+
url string
22+
packfile string
23+
}{
24+
{"https://github.com/tyba/git-fixture.git", "formats/packfile/fixtures/git-fixture.ofs-delta"},
25+
{"https://github.com/cpcs499/Final_Pres_P.git", "formats/packfile/fixtures/Final_Pres_P.ofs-delta"},
26+
{"https://github.com/jamesob/desk.git", "formats/packfile/fixtures/jamesob-desk.pack"},
27+
{"https://github.com/spinnaker/spinnaker.git", "formats/packfile/fixtures/spinnaker-spinnaker.pack"},
28+
{"https://github.com/alcortesm/binary-relations.git", "formats/packfile/fixtures/alcortesm-binary-relations.pack"},
29+
}
30+
s.repos = make(map[string]*Repository, 0)
31+
for _, fixRepo := range fixtureRepos {
32+
s.repos[fixRepo.url] = NewPlainRepository()
33+
34+
d, err := os.Open(fixRepo.packfile)
35+
c.Assert(err, IsNil)
36+
37+
r := packfile.NewReader(d)
38+
r.Format = packfile.OFSDeltaFormat // TODO: how to know the format of a pack file ahead of time?
39+
40+
_, err = r.Read(s.repos[fixRepo.url].Storage)
41+
c.Assert(err, IsNil)
42+
43+
c.Assert(d.Close(), IsNil)
44+
}
45+
}
46+
47+
func (s *SuiteTree) TestFile(c *C) {
48+
for i, t := range []struct {
49+
repo string // the repo name as in localRepos
50+
commit string // the commit to search for the file
51+
path string // the path of the file to find
52+
blobHash string // expected hash of the returned file
53+
found bool // expected found value
54+
}{
55+
// use git ls-tree commit to get the hash of the blobs
56+
{"https://github.com/tyba/git-fixture.git", "b029517f6300c2da0f4b651b8642506cd6aaf45d", "not-found",
57+
"", false},
58+
{"https://github.com/tyba/git-fixture.git", "b029517f6300c2da0f4b651b8642506cd6aaf45d", ".gitignore",
59+
"32858aad3c383ed1ff0a0f9bdf231d54a00c9e88", true},
60+
{"https://github.com/tyba/git-fixture.git", "b029517f6300c2da0f4b651b8642506cd6aaf45d", "LICENSE",
61+
"c192bd6a24ea1ab01d78686e417c8bdc7c3d197f", true},
62+
63+
{"https://github.com/tyba/git-fixture.git", "6ecf0ef2c2dffb796033e5a02219af86ec6584e5", "not-found",
64+
"", false},
65+
{"https://github.com/tyba/git-fixture.git", "6ecf0ef2c2dffb796033e5a02219af86ec6584e5", ".gitignore",
66+
"32858aad3c383ed1ff0a0f9bdf231d54a00c9e88", true},
67+
{"https://github.com/tyba/git-fixture.git", "6ecf0ef2c2dffb796033e5a02219af86ec6584e5", "binary.jpg",
68+
"d5c0f4ab811897cadf03aec358ae60d21f91c50d", true},
69+
{"https://github.com/tyba/git-fixture.git", "6ecf0ef2c2dffb796033e5a02219af86ec6584e5", "LICENSE",
70+
"c192bd6a24ea1ab01d78686e417c8bdc7c3d197f", true},
71+
72+
{"https://github.com/tyba/git-fixture.git", "35e85108805c84807bc66a02d91535e1e24b38b9", "binary.jpg",
73+
"d5c0f4ab811897cadf03aec358ae60d21f91c50d", true},
74+
{"https://github.com/tyba/git-fixture.git", "b029517f6300c2da0f4b651b8642506cd6aaf45d", "binary.jpg",
75+
"", false},
76+
77+
{"https://github.com/tyba/git-fixture.git", "6ecf0ef2c2dffb796033e5a02219af86ec6584e5", "CHANGELOG",
78+
"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", true},
79+
{"https://github.com/tyba/git-fixture.git", "1669dce138d9b841a518c64b10914d88f5e488ea", "CHANGELOG",
80+
"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", true},
81+
{"https://github.com/tyba/git-fixture.git", "a5b8b09e2f8fcb0bb99d3ccb0958157b40890d69", "CHANGELOG",
82+
"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", true},
83+
{"https://github.com/tyba/git-fixture.git", "35e85108805c84807bc66a02d91535e1e24b38b9", "CHANGELOG",
84+
"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", false},
85+
{"https://github.com/tyba/git-fixture.git", "b8e471f58bcbca63b07bda20e428190409c2db47", "CHANGELOG",
86+
"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", true},
87+
{"https://github.com/tyba/git-fixture.git", "b029517f6300c2da0f4b651b8642506cd6aaf45d", "CHANGELOG",
88+
"d3ff53e0564a9f87d8e84b6e28e5060e517008aa", false},
89+
90+
// git submodule
91+
{"https://github.com/cpcs499/Final_Pres_P.git", "70bade703ce556c2c7391a8065c45c943e8b6bc3", "Final",
92+
"", false},
93+
{"https://github.com/cpcs499/Final_Pres_P.git", "70bade703ce556c2c7391a8065c45c943e8b6bc3", "Final/not-found",
94+
"", false},
95+
96+
{"https://github.com/jamesob/desk.git", "d4edaf0e8101fcea437ebd982d899fe2cc0f9f7b", "LICENSE",
97+
"49c45e6cc893d6f5ebd5c9343fe4492360f339bf", true},
98+
{"https://github.com/jamesob/desk.git", "d4edaf0e8101fcea437ebd982d899fe2cc0f9f7b", "examples",
99+
"", false},
100+
{"https://github.com/jamesob/desk.git", "d4edaf0e8101fcea437ebd982d899fe2cc0f9f7b", "examples/desk.sh",
101+
"d9c7751138824cd2d539c23d5afe3f9d29836854", true},
102+
{"https://github.com/jamesob/desk.git", "d4edaf0e8101fcea437ebd982d899fe2cc0f9f7b", "examples/not-found",
103+
"", false},
104+
{"https://github.com/jamesob/desk.git", "d4edaf0e8101fcea437ebd982d899fe2cc0f9f7b", "test/bashrc",
105+
"e69de29bb2d1d6434b8b29ae775ad8c2e48c5391", true},
106+
{"https://github.com/jamesob/desk.git", "d4edaf0e8101fcea437ebd982d899fe2cc0f9f7b", "test/not-found",
107+
"", false},
108+
109+
{"https://github.com/spinnaker/spinnaker.git", "b32b2aecae2cfca4840dd480f8082da206a538da", "etc/apache2/sites-available/spinnaker.conf",
110+
"1d452c616be4fb16d2cc6b8a7e7a2208a6e64d2d", true},
111+
112+
{"https://github.com/alcortesm/binary-relations.git", "c44b5176e99085c8fe36fa27b045590a7b9d34c9", "Makefile",
113+
"2dd2ad8c14de6612ed15813679a6554bad99330b", true},
114+
{"https://github.com/alcortesm/binary-relations.git", "c44b5176e99085c8fe36fa27b045590a7b9d34c9", "src/binrels",
115+
"", false},
116+
{"https://github.com/alcortesm/binary-relations.git", "c44b5176e99085c8fe36fa27b045590a7b9d34c9", "src/map-slice",
117+
"", false},
118+
{"https://github.com/alcortesm/binary-relations.git", "c44b5176e99085c8fe36fa27b045590a7b9d34c9", "src/map-slice/map-slice.go",
119+
"12431e98381dd5097e1a19fe53429c72ef1f328e", true},
120+
{"https://github.com/alcortesm/binary-relations.git", "c44b5176e99085c8fe36fa27b045590a7b9d34c9", "src/map-slice/map-slice.go/not-found",
121+
"", false},
122+
} {
123+
commit, err := s.repos[t.repo].Commit(core.NewHash(t.commit))
124+
c.Assert(err, IsNil, Commentf("subtest %d: %v (%s)", i, err, t.commit))
125+
126+
tree := commit.Tree()
127+
file, err := tree.File(t.path)
128+
found := err == nil
129+
c.Assert(found, Equals, t.found, Commentf("subtest %d, path=%s, commit=%s", i, t.path, t.commit))
130+
if found {
131+
c.Assert(file.Hash.String(), Equals, t.blobHash, Commentf("subtest %d, commit=%s, path=%s", i, t.commit, t.path))
132+
}
133+
}
134+
}

0 commit comments

Comments
 (0)