Skip to content

Commit 9a6ab2d

Browse files
committed
Use git command instead of os/exec
1 parent 764faef commit 9a6ab2d

File tree

2 files changed

+37
-85
lines changed

2 files changed

+37
-85
lines changed

modules/git/blame.go

Lines changed: 28 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,7 @@ import (
1010
"fmt"
1111
"io"
1212
"os"
13-
"os/exec"
1413
"regexp"
15-
16-
"code.gitea.io/gitea/modules/process"
17-
"code.gitea.io/gitea/modules/setting"
1814
)
1915

2016
// BlamePart represents block of blame - continuous lines with one sha
@@ -25,12 +21,11 @@ type BlamePart struct {
2521

2622
// BlameReader returns part of file blame one by one
2723
type BlameReader struct {
28-
cmd *exec.Cmd
29-
output io.ReadCloser
30-
reader *bufio.Reader
31-
lastSha *string
32-
cancel context.CancelFunc // Cancels the context that this reader runs in
33-
finished process.FinishedFunc // Tells the process manager we're finished and it can remove the associated process from the process table
24+
cmd *CommandProxy
25+
output io.WriteCloser
26+
reader io.ReadCloser
27+
done chan error
28+
lastSha *string
3429
}
3530

3631
var shaLineRegex = regexp.MustCompile("^([a-z0-9]{40})")
@@ -39,7 +34,7 @@ var shaLineRegex = regexp.MustCompile("^([a-z0-9]{40})")
3934
func (r *BlameReader) NextPart() (*BlamePart, error) {
4035
var blamePart *BlamePart
4136

42-
reader := r.reader
37+
reader := bufio.NewReader(r.reader)
4338

4439
if r.lastSha != nil {
4540
blamePart = &BlamePart{*r.lastSha, make([]string, 0)}
@@ -101,51 +96,40 @@ func (r *BlameReader) NextPart() (*BlamePart, error) {
10196

10297
// Close BlameReader - don't run NextPart after invoking that
10398
func (r *BlameReader) Close() error {
104-
defer r.finished() // Only remove the process from the process table when the underlying command is closed
105-
r.cancel() // However, first cancel our own context early
106-
99+
err := <-r.done
100+
_ = r.reader.Close()
107101
_ = r.output.Close()
108102

109-
if err := r.cmd.Wait(); err != nil {
110-
return fmt.Errorf("Wait: %v", err)
111-
}
112-
113-
return nil
103+
return err
114104
}
115105

116106
// CreateBlameReader creates reader for given repository, commit and file
117107
func CreateBlameReader(ctx context.Context, repoPath, commitID, file string) (*BlameReader, error) {
118-
return createBlameReader(ctx, repoPath, setting.Git.Path, "blame", commitID, "--porcelain", "--", file)
119-
}
108+
cmd := NewCommandContextNoGlobals(ctx, "blame", commitID, "--porcelain", "--", file)
109+
cmd.SetDescription(fmt.Sprintf("GetBlame [repo_path: %s]", repoPath))
120110

121-
func createBlameReader(ctx context.Context, dir string, command ...string) (*BlameReader, error) {
122-
// Here we use the provided context - this should be tied to the request performing the blame so that it does not hang around.
123-
ctx, cancel, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("GetBlame [repo_path: %s]", dir))
124-
125-
cmd := exec.CommandContext(ctx, command[0], command[1:]...)
126-
cmd.Dir = dir
127-
cmd.Stderr = os.Stderr
128-
process.SetSysProcAttribute(cmd)
129-
130-
stdout, err := cmd.StdoutPipe()
111+
reader, stdout, err := os.Pipe()
131112
if err != nil {
132-
defer finished()
133-
return nil, fmt.Errorf("StdoutPipe: %v", err)
113+
return nil, err
134114
}
135115

136-
if err = cmd.Start(); err != nil {
137-
defer finished()
138-
_ = stdout.Close()
139-
return nil, fmt.Errorf("Start: %v", err)
140-
}
116+
done := make(chan error, 1)
141117

142-
reader := bufio.NewReader(stdout)
118+
go func(cmd *CommandProxy, dir string, stdout io.WriteCloser, done chan error) {
119+
if err := cmd.Run(&RunOpts{
120+
Dir: dir,
121+
Stdout: stdout,
122+
Stderr: os.Stderr,
123+
}); err == nil {
124+
stdout.Close()
125+
}
126+
done <- err
127+
}(cmd, repoPath, stdout, done)
143128

144129
return &BlameReader{
145-
cmd: cmd,
146-
output: stdout,
147-
reader: reader,
148-
cancel: cancel,
149-
finished: finished,
130+
cmd: cmd,
131+
output: stdout,
132+
reader: reader,
133+
done: done,
150134
}, nil
151135
}

modules/git/blame_test.go

Lines changed: 9 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package git
66

77
import (
88
"context"
9-
"os"
109
"testing"
1110

1211
"github.com/stretchr/testify/assert"
@@ -65,7 +64,7 @@ summary Add code of delete user
6564
previous be0ba9ea88aff8a658d0495d36accf944b74888d gogs.go
6665
filename gogs.go
6766
// license that can be found in the LICENSE file.
68-
67+
6968
e2aa991e10ffd924a828ec149951f2f20eecead2 6 6 2
7069
author Lunny Xiao
7170
author-mail <[email protected]>
@@ -84,61 +83,30 @@ e2aa991e10ffd924a828ec149951f2f20eecead2 7 7
8483
`
8584

8685
func TestReadingBlameOutput(t *testing.T) {
87-
tempFile, err := os.CreateTemp("", ".txt")
88-
if err != nil {
89-
panic(err)
90-
}
91-
92-
defer tempFile.Close()
93-
94-
if _, err = tempFile.WriteString(exampleBlame); err != nil {
95-
panic(err)
96-
}
9786
ctx, cancel := context.WithCancel(context.Background())
9887
defer cancel()
9988

100-
blameReader, err := createBlameReader(ctx, "", "cat", tempFile.Name())
101-
if err != nil {
102-
panic(err)
103-
}
89+
blameReader, err := CreateBlameReader(ctx, "./tests/repos/repo5_pulls", "f32b0a9dfd09a60f616f29158f772cedd89942d2", "README.md")
90+
assert.NoError(t, err)
10491
defer blameReader.Close()
10592

10693
parts := []*BlamePart{
10794
{
108-
"4b92a6c2df28054ad766bc262f308db9f6066596",
109-
[]string{
110-
"// Copyright 2014 The Gogs Authors. All rights reserved.",
111-
},
112-
},
113-
{
114-
"ce21ed6c3490cdfad797319cbb1145e2330a8fef",
115-
[]string{
116-
"// Copyright 2016 The Gitea Authors. All rights reserved.",
117-
},
118-
},
119-
{
120-
"4b92a6c2df28054ad766bc262f308db9f6066596",
95+
"72866af952e98d02a73003501836074b286a78f6",
12196
[]string{
122-
"// Use of this source code is governed by a MIT-style",
123-
"// license that can be found in the LICENSE file.",
124-
"",
97+
"# test_repo",
98+
"Test repository for testing migration from github to gitea",
12599
},
126100
},
127101
{
128-
"e2aa991e10ffd924a828ec149951f2f20eecead2",
129-
[]string{
130-
"// Gitea (git with a cup of tea) is a painless self-hosted Git Service.",
131-
"package main // import \"code.gitea.io/gitea\"",
132-
},
102+
"f32b0a9dfd09a60f616f29158f772cedd89942d2",
103+
[]string{},
133104
},
134-
nil,
135105
}
136106

137107
for _, part := range parts {
138108
actualPart, err := blameReader.NextPart()
139-
if err != nil {
140-
panic(err)
141-
}
109+
assert.NoError(t, err)
142110
assert.Equal(t, part, actualPart)
143111
}
144112
}

0 commit comments

Comments
 (0)