Skip to content

Commit b231d0d

Browse files
wxiaoguangzeripathlafriks6543
authored
Ignore Sync errors on pipes when doing CheckAttributeReader.CheckPath, fix the hang of git cat-file (#17096)
* Ignore Sync errors on pipes when doing `CheckAttributeReader.CheckPath` * apply env patch * Drop the Sync and fix a number of issues with the Close function Signed-off-by: Andrew Thornton <[email protected]> * add logs for DBIndexer and CheckPath * Fix some more closing bugs Signed-off-by: Andrew Thornton <[email protected]> * Add test case for language_stats Signed-off-by: Andrew Thornton <[email protected]> * Update modules/indexer/stats/db.go Co-authored-by: Lauris BH <[email protected]> Co-authored-by: Andrew Thornton <[email protected]> Co-authored-by: Lauris BH <[email protected]> Co-authored-by: 6543 <[email protected]>
1 parent 5ac857f commit b231d0d

35 files changed

+960
-65
lines changed

‎modules/git/repo_attribute.go

Lines changed: 52 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import (
1212
"os"
1313
"strconv"
1414
"strings"
15+
16+
"code.gitea.io/gitea/modules/log"
1517
)
1618

1719
// CheckAttributeOpts represents the possible options to CheckAttribute
@@ -112,56 +114,63 @@ func (c *CheckAttributeReader) Init(ctx context.Context) error {
112114

113115
if len(c.IndexFile) > 0 && CheckGitVersionAtLeast("1.7.8") == nil {
114116
cmdArgs = append(cmdArgs, "--cached")
115-
c.env = []string{"GIT_INDEX_FILE=" + c.IndexFile}
117+
c.env = append(c.env, "GIT_INDEX_FILE="+c.IndexFile)
116118
}
117119

118120
if len(c.WorkTree) > 0 && CheckGitVersionAtLeast("1.7.8") == nil {
119-
c.env = []string{"GIT_WORK_TREE=" + c.WorkTree}
121+
c.env = append(c.env, "GIT_WORK_TREE="+c.WorkTree)
120122
}
121123

122-
if len(c.Attributes) > 0 {
123-
cmdArgs = append(cmdArgs, c.Attributes...)
124-
cmdArgs = append(cmdArgs, "--")
125-
} else {
124+
c.env = append(c.env, "GIT_FLUSH=1")
125+
126+
if len(c.Attributes) == 0 {
126127
lw := new(nulSeparatedAttributeWriter)
127128
lw.attributes = make(chan attributeTriple)
129+
lw.closed = make(chan struct{})
128130

129131
c.stdOut = lw
130132
c.stdOut.Close()
131133
return fmt.Errorf("no provided Attributes to check")
132134
}
133135

136+
cmdArgs = append(cmdArgs, c.Attributes...)
137+
cmdArgs = append(cmdArgs, "--")
138+
134139
c.ctx, c.cancel = context.WithCancel(ctx)
135140
c.cmd = NewCommandContext(c.ctx, cmdArgs...)
141+
136142
var err error
143+
137144
c.stdinReader, c.stdinWriter, err = os.Pipe()
138145
if err != nil {
146+
c.cancel()
139147
return err
140148
}
141149

142150
if CheckGitVersionAtLeast("1.8.5") == nil {
143151
lw := new(nulSeparatedAttributeWriter)
144152
lw.attributes = make(chan attributeTriple, 5)
145-
153+
lw.closed = make(chan struct{})
146154
c.stdOut = lw
147155
} else {
148156
lw := new(lineSeparatedAttributeWriter)
149157
lw.attributes = make(chan attributeTriple, 5)
150-
158+
lw.closed = make(chan struct{})
151159
c.stdOut = lw
152160
}
153161
return nil
154162
}
155163

156164
// Run run cmd
157165
func (c *CheckAttributeReader) Run() error {
166+
defer func() {
167+
_ = c.Close()
168+
}()
158169
stdErr := new(bytes.Buffer)
159170
err := c.cmd.RunInDirTimeoutEnvFullPipelineFunc(c.env, -1, c.Repo.Path, c.stdOut, stdErr, c.stdinReader, func(_ context.Context, _ context.CancelFunc) error {
160171
close(c.running)
161172
return nil
162173
})
163-
defer c.cancel()
164-
_ = c.stdOut.Close()
165174
if err != nil && c.ctx.Err() != nil && err.Error() != "signal: killed" {
166175
return fmt.Errorf("failed to run attr-check. Error: %w\nStderr: %s", err, stdErr.String())
167176
}
@@ -170,27 +179,31 @@ func (c *CheckAttributeReader) Run() error {
170179
}
171180

172181
// CheckPath check attr for given path
173-
func (c *CheckAttributeReader) CheckPath(path string) (map[string]string, error) {
182+
func (c *CheckAttributeReader) CheckPath(path string) (rs map[string]string, err error) {
183+
defer func() {
184+
if err != nil {
185+
log.Error("CheckPath returns error: %v", err)
186+
}
187+
}()
188+
174189
select {
175190
case <-c.ctx.Done():
176191
return nil, c.ctx.Err()
177192
case <-c.running:
178193
}
179194

180-
if _, err := c.stdinWriter.Write([]byte(path + "\x00")); err != nil {
181-
defer c.cancel()
195+
if _, err = c.stdinWriter.Write([]byte(path + "\x00")); err != nil {
196+
defer c.Close()
182197
return nil, err
183198
}
184199

185-
if err := c.stdinWriter.Sync(); err != nil {
186-
defer c.cancel()
187-
return nil, err
188-
}
189-
190-
rs := make(map[string]string)
200+
rs = make(map[string]string)
191201
for range c.Attributes {
192202
select {
193-
case attr := <-c.stdOut.ReadAttribute():
203+
case attr, ok := <-c.stdOut.ReadAttribute():
204+
if !ok {
205+
return nil, c.ctx.Err()
206+
}
194207
rs[attr.Attribute] = attr.Value
195208
case <-c.ctx.Done():
196209
return nil, c.ctx.Err()
@@ -201,13 +214,16 @@ func (c *CheckAttributeReader) CheckPath(path string) (map[string]string, error)
201214

202215
// Close close pip after use
203216
func (c *CheckAttributeReader) Close() error {
217+
err := c.stdinWriter.Close()
218+
_ = c.stdinReader.Close()
219+
_ = c.stdOut.Close()
220+
c.cancel()
204221
select {
205222
case <-c.running:
206223
default:
207224
close(c.running)
208225
}
209-
defer c.cancel()
210-
return c.stdinWriter.Close()
226+
return err
211227
}
212228

213229
type attributeWriter interface {
@@ -224,6 +240,7 @@ type attributeTriple struct {
224240
type nulSeparatedAttributeWriter struct {
225241
tmp []byte
226242
attributes chan attributeTriple
243+
closed chan struct{}
227244
working attributeTriple
228245
pos int
229246
}
@@ -267,13 +284,20 @@ func (wr *nulSeparatedAttributeWriter) ReadAttribute() <-chan attributeTriple {
267284
}
268285

269286
func (wr *nulSeparatedAttributeWriter) Close() error {
287+
select {
288+
case <-wr.closed:
289+
return nil
290+
default:
291+
}
270292
close(wr.attributes)
293+
close(wr.closed)
271294
return nil
272295
}
273296

274297
type lineSeparatedAttributeWriter struct {
275298
tmp []byte
276299
attributes chan attributeTriple
300+
closed chan struct{}
277301
}
278302

279303
func (wr *lineSeparatedAttributeWriter) Write(p []byte) (n int, err error) {
@@ -356,6 +380,12 @@ func (wr *lineSeparatedAttributeWriter) ReadAttribute() <-chan attributeTriple {
356380
}
357381

358382
func (wr *lineSeparatedAttributeWriter) Close() error {
383+
select {
384+
case <-wr.closed:
385+
return nil
386+
default:
387+
}
359388
close(wr.attributes)
389+
close(wr.closed)
360390
return nil
361391
}

‎modules/git/repo_language_stats_gogit.go

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515

1616
"code.gitea.io/gitea/modules/analyze"
1717
"code.gitea.io/gitea/modules/log"
18+
"code.gitea.io/gitea/modules/util"
1819

1920
"github.com/go-enry/go-enry/v2"
2021
"github.com/go-git/go-git/v5"
@@ -50,25 +51,32 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err
5051
indexFilename, deleteTemporaryFile, err := repo.ReadTreeToTemporaryIndex(commitID)
5152
if err == nil {
5253
defer deleteTemporaryFile()
53-
54-
checker = &CheckAttributeReader{
55-
Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language"},
56-
Repo: repo,
57-
IndexFile: indexFilename,
58-
}
59-
ctx, cancel := context.WithCancel(DefaultContext)
60-
if err := checker.Init(ctx); err != nil {
61-
log.Error("Unable to open checker for %s. Error: %v", commitID, err)
62-
} else {
63-
go func() {
64-
err = checker.Run()
65-
if err != nil {
66-
log.Error("Unable to open checker for %s. Error: %v", commitID, err)
67-
cancel()
68-
}
54+
tmpWorkTree, err := ioutil.TempDir("", "empty-work-dir")
55+
if err == nil {
56+
defer func() {
57+
_ = util.RemoveAll(tmpWorkTree)
6958
}()
59+
60+
checker = &CheckAttributeReader{
61+
Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language"},
62+
Repo: repo,
63+
IndexFile: indexFilename,
64+
WorkTree: tmpWorkTree,
65+
}
66+
ctx, cancel := context.WithCancel(DefaultContext)
67+
if err := checker.Init(ctx); err != nil {
68+
log.Error("Unable to open checker for %s. Error: %v", commitID, err)
69+
} else {
70+
go func() {
71+
err = checker.Run()
72+
if err != nil {
73+
log.Error("Unable to open checker for %s. Error: %v", commitID, err)
74+
cancel()
75+
}
76+
}()
77+
}
78+
defer cancel()
7079
}
71-
defer cancel()
7280
}
7381
}
7482

@@ -99,7 +107,7 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err
99107
if language, has := attrs["linguist-language"]; has && language != "unspecified" && language != "" {
100108
// group languages, such as Pug -> HTML; SCSS -> CSS
101109
group := enry.GetLanguageGroup(language)
102-
if len(group) == 0 {
110+
if len(group) != 0 {
103111
language = group
104112
}
105113

‎modules/git/repo_language_stats_nogogit.go

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,12 @@ import (
1212
"bytes"
1313
"context"
1414
"io"
15+
"io/ioutil"
1516
"math"
1617

1718
"code.gitea.io/gitea/modules/analyze"
1819
"code.gitea.io/gitea/modules/log"
20+
"code.gitea.io/gitea/modules/util"
1921

2022
"github.com/go-enry/go-enry/v2"
2123
)
@@ -69,25 +71,32 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err
6971
indexFilename, deleteTemporaryFile, err := repo.ReadTreeToTemporaryIndex(commitID)
7072
if err == nil {
7173
defer deleteTemporaryFile()
72-
73-
checker = &CheckAttributeReader{
74-
Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language"},
75-
Repo: repo,
76-
IndexFile: indexFilename,
77-
}
78-
ctx, cancel := context.WithCancel(DefaultContext)
79-
if err := checker.Init(ctx); err != nil {
80-
log.Error("Unable to open checker for %s. Error: %v", commitID, err)
81-
} else {
82-
go func() {
83-
err = checker.Run()
84-
if err != nil {
85-
log.Error("Unable to open checker for %s. Error: %v", commitID, err)
86-
cancel()
87-
}
74+
tmpWorkTree, err := ioutil.TempDir("", "empty-work-dir")
75+
if err == nil {
76+
defer func() {
77+
_ = util.RemoveAll(tmpWorkTree)
8878
}()
79+
80+
checker = &CheckAttributeReader{
81+
Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language"},
82+
Repo: repo,
83+
IndexFile: indexFilename,
84+
WorkTree: tmpWorkTree,
85+
}
86+
ctx, cancel := context.WithCancel(DefaultContext)
87+
if err := checker.Init(ctx); err != nil {
88+
log.Error("Unable to open checker for %s. Error: %v", commitID, err)
89+
} else {
90+
go func() {
91+
err = checker.Run()
92+
if err != nil {
93+
log.Error("Unable to open checker for %s. Error: %v", commitID, err)
94+
cancel()
95+
}
96+
}()
97+
}
98+
defer cancel()
8999
}
90-
defer cancel()
91100
}
92101
}
93102

@@ -123,12 +132,11 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err
123132
if language, has := attrs["linguist-language"]; has && language != "unspecified" && language != "" {
124133
// group languages, such as Pug -> HTML; SCSS -> CSS
125134
group := enry.GetLanguageGroup(language)
126-
if len(group) == 0 {
135+
if len(group) != 0 {
127136
language = group
128137
}
129138

130139
sizes[language] += f.Size()
131-
132140
continue
133141
}
134142
}
@@ -186,7 +194,6 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err
186194
}
187195

188196
sizes[language] += f.Size()
189-
190197
continue
191198
}
192199

‎modules/git/repo_language_stats_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Copyright 2020 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
//go:build !gogit
6+
// +build !gogit
7+
8+
package git
9+
10+
import (
11+
"path/filepath"
12+
"testing"
13+
14+
"github.com/stretchr/testify/assert"
15+
)
16+
17+
func TestRepository_GetLanguageStats(t *testing.T) {
18+
repoPath := filepath.Join(testReposDir, "language_stats_repo")
19+
gitRepo, err := OpenRepository(repoPath)
20+
if !assert.NoError(t, err) {
21+
t.Fatal()
22+
}
23+
defer gitRepo.Close()
24+
25+
stats, err := gitRepo.GetLanguageStats("8fee858da5796dfb37704761701bb8e800ad9ef3")
26+
if !assert.NoError(t, err) {
27+
t.Fatal()
28+
}
29+
30+
assert.EqualValues(t, map[string]int64{
31+
"Python": 134,
32+
"Java": 112,
33+
}, stats)
34+
}

‎modules/git/tests/repos/language_stats_repo/COMMIT_EDITMSG

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Add some test files for GetLanguageStats
2+
3+
Signed-off-by: Andrew Thornton <[email protected]>

‎modules/git/tests/repos/language_stats_repo/HEAD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ref: refs/heads/master

‎modules/git/tests/repos/language_stats_repo/config

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
[core]
2+
repositoryformatversion = 0
3+
filemode = true
4+
bare = false
5+
logallrefupdates = true

‎modules/git/tests/repos/language_stats_repo/description

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Unnamed repository; edit this file 'description' to name the repository.

0 commit comments

Comments
 (0)