Skip to content

Commit 35fdefc

Browse files
lunnylunny
authored
Always use git command but not os.Command (#18363)
1 parent f066b29 commit 35fdefc

File tree

3 files changed

+46
-56
lines changed

3 files changed

+46
-56
lines changed

‎modules/git/diff.go

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,11 @@ import (
1111
"fmt"
1212
"io"
1313
"os"
14-
"os/exec"
1514
"regexp"
1615
"strconv"
1716
"strings"
1817

1918
"code.gitea.io/gitea/modules/log"
20-
"code.gitea.io/gitea/modules/process"
2119
)
2220

2321
// RawDiffType type of a raw diff.
@@ -55,43 +53,41 @@ func GetRepoRawDiffForFile(repo *Repository, startCommit, endCommit string, diff
5553
if len(file) > 0 {
5654
fileArgs = append(fileArgs, "--", file)
5755
}
58-
// FIXME: graceful: These commands should have a timeout
59-
ctx, _, finished := process.GetManager().AddContext(repo.Ctx, fmt.Sprintf("GetRawDiffForFile: [repo_path: %s]", repo.Path))
60-
defer finished()
6156

62-
var cmd *exec.Cmd
57+
var args []string
6358
switch diffType {
6459
case RawDiffNormal:
6560
if len(startCommit) != 0 {
66-
cmd = exec.CommandContext(ctx, GitExecutable, append([]string{"diff", "-M", startCommit, endCommit}, fileArgs...)...)
61+
args = append([]string{"diff", "-M", startCommit, endCommit}, fileArgs...)
6762
} else if commit.ParentCount() == 0 {
68-
cmd = exec.CommandContext(ctx, GitExecutable, append([]string{"show", endCommit}, fileArgs...)...)
63+
args = append([]string{"show", endCommit}, fileArgs...)
6964
} else {
7065
c, _ := commit.Parent(0)
71-
cmd = exec.CommandContext(ctx, GitExecutable, append([]string{"diff", "-M", c.ID.String(), endCommit}, fileArgs...)...)
66+
args = append([]string{"diff", "-M", c.ID.String(), endCommit}, fileArgs...)
7267
}
7368
case RawDiffPatch:
7469
if len(startCommit) != 0 {
7570
query := fmt.Sprintf("%s...%s", endCommit, startCommit)
76-
cmd = exec.CommandContext(ctx, GitExecutable, append([]string{"format-patch", "--no-signature", "--stdout", "--root", query}, fileArgs...)...)
71+
args = append([]string{"format-patch", "--no-signature", "--stdout", "--root", query}, fileArgs...)
7772
} else if commit.ParentCount() == 0 {
78-
cmd = exec.CommandContext(ctx, GitExecutable, append([]string{"format-patch", "--no-signature", "--stdout", "--root", endCommit}, fileArgs...)...)
73+
args = append([]string{"format-patch", "--no-signature", "--stdout", "--root", endCommit}, fileArgs...)
7974
} else {
8075
c, _ := commit.Parent(0)
8176
query := fmt.Sprintf("%s...%s", endCommit, c.ID.String())
82-
cmd = exec.CommandContext(ctx, GitExecutable, append([]string{"format-patch", "--no-signature", "--stdout", query}, fileArgs...)...)
77+
args = append([]string{"format-patch", "--no-signature", "--stdout", query}, fileArgs...)
8378
}
8479
default:
8580
return fmt.Errorf("invalid diffType: %s", diffType)
8681
}
8782

8883
stderr := new(bytes.Buffer)
89-
90-
cmd.Dir = repo.Path
91-
cmd.Stdout = writer
92-
cmd.Stderr = stderr
93-
94-
if err = cmd.Run(); err != nil {
84+
cmd := NewCommandContextNoGlobals(repo.Ctx, args...)
85+
if err = cmd.RunWithContext(&RunContext{
86+
Timeout: -1,
87+
Dir: repo.Path,
88+
Stdout: writer,
89+
Stderr: stderr,
90+
}); err != nil {
9591
return fmt.Errorf("Run: %v - %s", err, stderr)
9692
}
9793
return nil

‎routers/web/repo/http.go

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"fmt"
1313
"net/http"
1414
"os"
15-
"os/exec"
1615
"path"
1716
"regexp"
1817
"strconv"
@@ -30,7 +29,6 @@ import (
3029
"code.gitea.io/gitea/modules/context"
3130
"code.gitea.io/gitea/modules/git"
3231
"code.gitea.io/gitea/modules/log"
33-
"code.gitea.io/gitea/modules/process"
3432
"code.gitea.io/gitea/modules/setting"
3533
"code.gitea.io/gitea/modules/structs"
3634
"code.gitea.io/gitea/modules/util"
@@ -486,18 +484,17 @@ func serviceRPC(ctx gocontext.Context, h serviceHandler, service string) {
486484
h.environ = append(h.environ, "GIT_PROTOCOL="+protocol)
487485
}
488486

489-
ctx, _, finished := process.GetManager().AddContext(h.r.Context(), fmt.Sprintf("%s %s %s [repo_path: %s]", git.GitExecutable, service, "--stateless-rpc", h.dir))
490-
defer finished()
491-
492487
var stderr bytes.Buffer
493-
cmd := exec.CommandContext(ctx, git.GitExecutable, service, "--stateless-rpc", h.dir)
494-
cmd.Dir = h.dir
495-
cmd.Env = append(os.Environ(), h.environ...)
496-
cmd.Stdout = h.w
497-
cmd.Stdin = reqBody
498-
cmd.Stderr = &stderr
499-
500-
if err := cmd.Run(); err != nil {
488+
cmd := git.NewCommandContextNoGlobals(h.r.Context(), service, "--stateless-rpc", h.dir)
489+
cmd.SetDescription(fmt.Sprintf("%s %s %s [repo_path: %s]", git.GitExecutable, service, "--stateless-rpc", h.dir))
490+
if err := cmd.RunWithContext(&git.RunContext{
491+
Timeout: -1,
492+
Dir: h.dir,
493+
Env: append(os.Environ(), h.environ...),
494+
Stdout: h.w,
495+
Stdin: reqBody,
496+
Stderr: &stderr,
497+
}); err != nil {
501498
log.Error("Fail to serve RPC(%s) in %s: %v - %s", service, h.dir, err, stderr.String())
502499
return
503500
}

‎services/gitdiff/gitdiff.go

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"io"
1616
"net/url"
1717
"os"
18-
"os/exec"
1918
"regexp"
2019
"sort"
2120
"strings"
@@ -30,7 +29,6 @@ import (
3029
"code.gitea.io/gitea/modules/highlight"
3130
"code.gitea.io/gitea/modules/lfs"
3231
"code.gitea.io/gitea/modules/log"
33-
"code.gitea.io/gitea/modules/process"
3432
"code.gitea.io/gitea/modules/setting"
3533

3634
"github.com/sergi/go-diff/diffmatchpatch"
@@ -1322,10 +1320,6 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff
13221320
return nil, err
13231321
}
13241322

1325-
timeout := time.Duration(setting.Git.Timeout.Default) * time.Second
1326-
ctx, _, finished := process.GetManager().AddContextTimeout(gitRepo.Ctx, timeout, fmt.Sprintf("GetDiffRange [repo_path: %s]", repoPath))
1327-
defer finished()
1328-
13291323
argsLength := 6
13301324
if len(opts.WhitespaceBehavior) > 0 {
13311325
argsLength++
@@ -1375,21 +1369,28 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff
13751369
diffArgs = append(diffArgs, files...)
13761370
}
13771371

1378-
cmd := exec.CommandContext(ctx, git.GitExecutable, diffArgs...)
1379-
1380-
cmd.Dir = repoPath
1381-
cmd.Stderr = os.Stderr
1372+
reader, writer := io.Pipe()
1373+
defer func() {
1374+
_ = reader.Close()
1375+
_ = writer.Close()
1376+
}()
13821377

1383-
stdout, err := cmd.StdoutPipe()
1384-
if err != nil {
1385-
return nil, fmt.Errorf("error creating StdoutPipe: %w", err)
1386-
}
1378+
go func(ctx context.Context, diffArgs []string, repoPath string, writer *io.PipeWriter) {
1379+
cmd := git.NewCommandContextNoGlobals(ctx, diffArgs...)
1380+
cmd.SetDescription(fmt.Sprintf("GetDiffRange [repo_path: %s]", repoPath))
1381+
if err := cmd.RunWithContext(&git.RunContext{
1382+
Timeout: time.Duration(setting.Git.Timeout.Default) * time.Second,
1383+
Dir: repoPath,
1384+
Stderr: os.Stderr,
1385+
Stdout: writer,
1386+
}); err != nil {
1387+
log.Error("error during RunWithContext: %w", err)
1388+
}
13871389

1388-
if err = cmd.Start(); err != nil {
1389-
return nil, fmt.Errorf("error during Start: %w", err)
1390-
}
1390+
_ = writer.Close()
1391+
}(gitRepo.Ctx, diffArgs, repoPath, writer)
13911392

1392-
diff, err := ParsePatch(opts.MaxLines, opts.MaxLineCharacters, opts.MaxFiles, stdout, parsePatchSkipToFile)
1393+
diff, err := ParsePatch(opts.MaxLines, opts.MaxLineCharacters, opts.MaxFiles, reader, parsePatchSkipToFile)
13931394
if err != nil {
13941395
return nil, fmt.Errorf("unable to ParsePatch: %w", err)
13951396
}
@@ -1408,7 +1409,7 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff
14081409
IndexFile: indexFilename,
14091410
WorkTree: worktree,
14101411
}
1411-
ctx, cancel := context.WithCancel(ctx)
1412+
ctx, cancel := context.WithCancel(gitRepo.Ctx)
14121413
if err := checker.Init(ctx); err != nil {
14131414
log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err)
14141415
} else {
@@ -1472,10 +1473,6 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff
14721473
}
14731474
}
14741475

1475-
if err = cmd.Wait(); err != nil {
1476-
return nil, fmt.Errorf("error during cmd.Wait: %w", err)
1477-
}
1478-
14791476
separator := "..."
14801477
if opts.DirectComparison {
14811478
separator = ".."
@@ -1485,12 +1482,12 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff
14851482
if len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == git.EmptySHA {
14861483
shortstatArgs = []string{git.EmptyTreeSHA, opts.AfterCommitID}
14871484
}
1488-
diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(ctx, repoPath, shortstatArgs...)
1485+
diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(gitRepo.Ctx, repoPath, shortstatArgs...)
14891486
if err != nil && strings.Contains(err.Error(), "no merge base") {
14901487
// git >= 2.28 now returns an error if base and head have become unrelated.
14911488
// previously it would return the results of git diff --shortstat base head so let's try that...
14921489
shortstatArgs = []string{opts.BeforeCommitID, opts.AfterCommitID}
1493-
diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(ctx, repoPath, shortstatArgs...)
1490+
diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(gitRepo.Ctx, repoPath, shortstatArgs...)
14941491
}
14951492
if err != nil {
14961493
return nil, err

0 commit comments

Comments
 (0)