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

Commit 07e4c43

Browse files
alcortesmalcortesm
committed
Fix commit.File() gorutine leak
Commit.File() was leaking a goroutine because it was looping over an iterator without closing its channel. Now commit.File() calls the new Tree.File() method that searches the file in the repository by trasversing the dir tree instead of using the tree.Files() iterator. This not only prevent the goroutine leak, but also speeds up file searching.
1 parent 37cc5cf commit 07e4c43

File tree

3 files changed

+215
-10
lines changed

3 files changed

+215
-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: 80 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,84 @@ 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+
func (t *Tree) File(path string) (*File, error) {
35+
hash, err := t.hashOf(path)
36+
if err != nil {
37+
return nil, ErrFileNotFound
38+
}
39+
40+
obj, ok := t.r.Storage.Get(*hash)
41+
if !ok {
42+
return nil, ErrFileNotFound // a git submodule
43+
}
44+
45+
if obj.Type() != core.BlobObject {
46+
return nil, ErrFileNotFound // a directory
47+
}
48+
49+
blob := &Blob{}
50+
blob.Decode(obj)
51+
52+
return &File{Name: path, Reader: blob.Reader(), Hash: *hash}, nil
53+
}
54+
55+
func (t *Tree) hashOf(path string) (*core.Hash, error) {
56+
pathParts := strings.Split(path, "/")
57+
58+
var tree *Tree
59+
var err error
60+
for tree = t; len(pathParts) > 1; pathParts = pathParts[1:] {
61+
if tree, err = tree.dir(pathParts[0]); err != nil {
62+
return nil, err
63+
}
64+
}
65+
66+
entry, err := tree.entry(pathParts[0])
67+
if err != nil {
68+
return nil, err
69+
}
70+
71+
return &entry.Hash, nil
72+
}
73+
74+
var errDirNotFound = errors.New("directory not found")
75+
76+
func (t *Tree) dir(baseName string) (*Tree, error) {
77+
entry, err := t.entry(baseName)
78+
if err != nil {
79+
return nil, errDirNotFound
80+
}
81+
82+
obj, ok := t.r.Storage.Get(entry.Hash)
83+
if !ok { // git submodule
84+
return nil, errDirNotFound
85+
}
86+
87+
if obj.Type() != core.TreeObject {
88+
return nil, errDirNotFound // a file
89+
}
90+
91+
tree := &Tree{r: t.r}
92+
tree.Decode(obj)
93+
94+
return tree, nil
95+
}
96+
97+
var errEntryNotFound = errors.New("entry not found")
98+
99+
func (t *Tree) entry(baseName string) (*TreeEntry, error) {
100+
for _, entry := range t.Entries {
101+
if entry.Name == baseName {
102+
return &entry, nil
103+
}
104+
}
105+
106+
return nil, errEntryNotFound
107+
}
108+
29109
func (t *Tree) Files() chan *File {
30110
ch := make(chan *File, 1)
31111

‎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)