realaravinth/bench-forgejo
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Always use git command but not os.Command (#18363)

Browse source
This commit is contained in:
Lunny Xiao 2022-01-23 13:57:52 +08:00 committed by GitHub
parent f066b293ac
commit 35fdefc1ff
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 46 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

32
modules/git/diff.go
View file

@ -11,13 +11,11 @@ import (
"fmt" "fmt"
"io" "io"
"os" "os"
"os/exec"
"regexp" "regexp"
"strconv" "strconv"
"strings" "strings"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/process"
) )
// RawDiffType type of a raw diff. // RawDiffType type of a raw diff.
@ -55,43 +53,41 @@ func GetRepoRawDiffForFile(repo *Repository, startCommit, endCommit string, diff
if len(file) > 0 { if len(file) > 0 {
fileArgs = append(fileArgs, "--", file) fileArgs = append(fileArgs, "--", file)
} }
// FIXME: graceful: These commands should have a timeout
ctx, _, finished := process.GetManager().AddContext(repo.Ctx, fmt.Sprintf("GetRawDiffForFile: [repo_path: %s]", repo.Path))
defer finished()
var cmd *exec.Cmd var args []string
switch diffType { switch diffType {
case RawDiffNormal: case RawDiffNormal:
if len(startCommit) != 0 { if len(startCommit) != 0 {
cmd = exec.CommandContext(ctx, GitExecutable, append([]string{"diff", "-M", startCommit, endCommit}, fileArgs...)...) args = append([]string{"diff", "-M", startCommit, endCommit}, fileArgs...)
} else if commit.ParentCount() == 0 { } else if commit.ParentCount() == 0 {
cmd = exec.CommandContext(ctx, GitExecutable, append([]string{"show", endCommit}, fileArgs...)...) args = append([]string{"show", endCommit}, fileArgs...)
} else { } else {
c, _ := commit.Parent(0) c, _ := commit.Parent(0)
cmd = exec.CommandContext(ctx, GitExecutable, append([]string{"diff", "-M", c.ID.String(), endCommit}, fileArgs...)...) args = append([]string{"diff", "-M", c.ID.String(), endCommit}, fileArgs...)
} }
case RawDiffPatch: case RawDiffPatch:
if len(startCommit) != 0 { if len(startCommit) != 0 {
query := fmt.Sprintf("%s...%s", endCommit, startCommit) query := fmt.Sprintf("%s...%s", endCommit, startCommit)
cmd = exec.CommandContext(ctx, GitExecutable, append([]string{"format-patch", "--no-signature", "--stdout", "--root", query}, fileArgs...)...) args = append([]string{"format-patch", "--no-signature", "--stdout", "--root", query}, fileArgs...)
} else if commit.ParentCount() == 0 { } else if commit.ParentCount() == 0 {
cmd = exec.CommandContext(ctx, GitExecutable, append([]string{"format-patch", "--no-signature", "--stdout", "--root", endCommit}, fileArgs...)...) args = append([]string{"format-patch", "--no-signature", "--stdout", "--root", endCommit}, fileArgs...)
} else { } else {
c, _ := commit.Parent(0) c, _ := commit.Parent(0)
query := fmt.Sprintf("%s...%s", endCommit, c.ID.String()) query := fmt.Sprintf("%s...%s", endCommit, c.ID.String())
cmd = exec.CommandContext(ctx, GitExecutable, append([]string{"format-patch", "--no-signature", "--stdout", query}, fileArgs...)...) args = append([]string{"format-patch", "--no-signature", "--stdout", query}, fileArgs...)
} }
default: default:
return fmt.Errorf("invalid diffType: %s", diffType) return fmt.Errorf("invalid diffType: %s", diffType)
} }
stderr := new(bytes.Buffer) stderr := new(bytes.Buffer)
cmd := NewCommandContextNoGlobals(repo.Ctx, args...)
cmd.Dir = repo.Path if err = cmd.RunWithContext(&RunContext{
cmd.Stdout = writer Timeout: -1,
cmd.Stderr = stderr Dir: repo.Path,
Stdout: writer,
if err = cmd.Run(); err != nil { Stderr: stderr,
}); err != nil {
return fmt.Errorf("Run: %v - %s", err, stderr) return fmt.Errorf("Run: %v - %s", err, stderr)
} }
return nil return nil

23
routers/web/repo/http.go
View file

@ -12,7 +12,6 @@ import (
"fmt" "fmt"
"net/http" "net/http"
"os" "os"
"os/exec"
"path" "path"
"regexp" "regexp"
"strconv" "strconv"
@ -30,7 +29,6 @@ import (
"code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/process"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/util"
@ -486,18 +484,17 @@ func serviceRPC(ctx gocontext.Context, h serviceHandler, service string) {
h.environ = append(h.environ, "GIT_PROTOCOL="+protocol) h.environ = append(h.environ, "GIT_PROTOCOL="+protocol)
} }
ctx, _, finished := process.GetManager().AddContext(h.r.Context(), fmt.Sprintf("%s %s %s [repo_path: %s]", git.GitExecutable, service, "--stateless-rpc", h.dir))
defer finished()
var stderr bytes.Buffer var stderr bytes.Buffer
cmd := exec.CommandContext(ctx, git.GitExecutable, service, "--stateless-rpc", h.dir) cmd := git.NewCommandContextNoGlobals(h.r.Context(), service, "--stateless-rpc", h.dir)
cmd.Dir = h.dir cmd.SetDescription(fmt.Sprintf("%s %s %s [repo_path: %s]", git.GitExecutable, service, "--stateless-rpc", h.dir))
cmd.Env = append(os.Environ(), h.environ...) if err := cmd.RunWithContext(&git.RunContext{
cmd.Stdout = h.w Timeout: -1,
cmd.Stdin = reqBody Dir: h.dir,
cmd.Stderr = &stderr Env: append(os.Environ(), h.environ...),
Stdout: h.w,
if err := cmd.Run(); err != nil { Stdin: reqBody,
Stderr: &stderr,
}); err != nil {
log.Error("Fail to serve RPC(%s) in %s: %v - %s", service, h.dir, err, stderr.String()) log.Error("Fail to serve RPC(%s) in %s: %v - %s", service, h.dir, err, stderr.String())
return return
} }

45
services/gitdiff/gitdiff.go
View file

@ -15,7 +15,6 @@ import (
"io" "io"
"net/url" "net/url"
"os" "os"
"os/exec"
"regexp" "regexp"
"sort" "sort"
"strings" "strings"
@ -30,7 +29,6 @@ import (
"code.gitea.io/gitea/modules/highlight" "code.gitea.io/gitea/modules/highlight"
"code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/lfs"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/process"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"github.com/sergi/go-diff/diffmatchpatch" "github.com/sergi/go-diff/diffmatchpatch"
@ -1322,10 +1320,6 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff
return nil, err return nil, err
} }
timeout := time.Duration(setting.Git.Timeout.Default) * time.Second
ctx, _, finished := process.GetManager().AddContextTimeout(gitRepo.Ctx, timeout, fmt.Sprintf("GetDiffRange [repo_path: %s]", repoPath))
defer finished()
argsLength := 6 argsLength := 6
if len(opts.WhitespaceBehavior) > 0 { if len(opts.WhitespaceBehavior) > 0 {
argsLength++ argsLength++
@ -1375,21 +1369,28 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff
diffArgs = append(diffArgs, files...) diffArgs = append(diffArgs, files...)
} }
cmd := exec.CommandContext(ctx, git.GitExecutable, diffArgs...) reader, writer := io.Pipe()
defer func() {
_ = reader.Close()
_ = writer.Close()
}()
cmd.Dir = repoPath go func(ctx context.Context, diffArgs []string, repoPath string, writer *io.PipeWriter) {
cmd.Stderr = os.Stderr cmd := git.NewCommandContextNoGlobals(ctx, diffArgs...)
cmd.SetDescription(fmt.Sprintf("GetDiffRange [repo_path: %s]", repoPath))
stdout, err := cmd.StdoutPipe() if err := cmd.RunWithContext(&git.RunContext{
if err != nil { Timeout: time.Duration(setting.Git.Timeout.Default) * time.Second,
return nil, fmt.Errorf("error creating StdoutPipe: %w", err) Dir: repoPath,
Stderr: os.Stderr,
Stdout: writer,
}); err != nil {
log.Error("error during RunWithContext: %w", err)
} }
if err = cmd.Start(); err != nil { _ = writer.Close()
return nil, fmt.Errorf("error during Start: %w", err) }(gitRepo.Ctx, diffArgs, repoPath, writer)
}
diff, err := ParsePatch(opts.MaxLines, opts.MaxLineCharacters, opts.MaxFiles, stdout, parsePatchSkipToFile) diff, err := ParsePatch(opts.MaxLines, opts.MaxLineCharacters, opts.MaxFiles, reader, parsePatchSkipToFile)
if err != nil { if err != nil {
return nil, fmt.Errorf("unable to ParsePatch: %w", err) return nil, fmt.Errorf("unable to ParsePatch: %w", err)
} }
@ -1408,7 +1409,7 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff
IndexFile: indexFilename, IndexFile: indexFilename,
WorkTree: worktree, WorkTree: worktree,
} }
ctx, cancel := context.WithCancel(ctx) ctx, cancel := context.WithCancel(gitRepo.Ctx)
if err := checker.Init(ctx); err != nil { if err := checker.Init(ctx); err != nil {
log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err) log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err)
} else { } else {
@ -1472,10 +1473,6 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff
} }
} }
if err = cmd.Wait(); err != nil {
return nil, fmt.Errorf("error during cmd.Wait: %w", err)
}
separator := "..." separator := "..."
if opts.DirectComparison { if opts.DirectComparison {
separator = ".." separator = ".."
@ -1485,12 +1482,12 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff
if len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == git.EmptySHA { if len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == git.EmptySHA {
shortstatArgs = []string{git.EmptyTreeSHA, opts.AfterCommitID} shortstatArgs = []string{git.EmptyTreeSHA, opts.AfterCommitID}
} }
diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(ctx, repoPath, shortstatArgs...) diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(gitRepo.Ctx, repoPath, shortstatArgs...)
if err != nil && strings.Contains(err.Error(), "no merge base") { if err != nil && strings.Contains(err.Error(), "no merge base") {
// git >= 2.28 now returns an error if base and head have become unrelated. // git >= 2.28 now returns an error if base and head have become unrelated.
// previously it would return the results of git diff --shortstat base head so let's try that... // previously it would return the results of git diff --shortstat base head so let's try that...
shortstatArgs = []string{opts.BeforeCommitID, opts.AfterCommitID} shortstatArgs = []string{opts.BeforeCommitID, opts.AfterCommitID}
diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(ctx, repoPath, shortstatArgs...) diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(gitRepo.Ctx, repoPath, shortstatArgs...)
} }
if err != nil { if err != nil {
return nil, err return nil, err