From efa708501be70201f048c2936addfd1bcf570d31 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 3 Jan 2023 16:17:13 +0800 Subject: [PATCH] Use git command instead of exec.Cmd in blame (#22098) extract from #18147 Co-authored-by: wxiaoguang --- modules/git/blame.go | 76 ++++++++++-------------- modules/git/blame_test.go | 119 +++----------------------------------- 2 files changed, 39 insertions(+), 156 deletions(-) diff --git a/modules/git/blame.go b/modules/git/blame.go index fea75b4818..3b6e4c95db 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -9,10 +9,7 @@ import ( "fmt" "io" "os" - "os/exec" "regexp" - - "code.gitea.io/gitea/modules/process" ) // BlamePart represents block of blame - continuous lines with one sha @@ -23,12 +20,11 @@ type BlamePart struct { // BlameReader returns part of file blame one by one type BlameReader struct { - cmd *exec.Cmd - output io.ReadCloser - reader *bufio.Reader - lastSha *string - cancel context.CancelFunc // Cancels the context that this reader runs in - finished process.FinishedFunc // Tells the process manager we're finished and it can remove the associated process from the process table + cmd *Command + output io.WriteCloser + reader io.ReadCloser + done chan error + lastSha *string } var shaLineRegex = regexp.MustCompile("^([a-z0-9]{40})") @@ -37,7 +33,7 @@ var shaLineRegex = regexp.MustCompile("^([a-z0-9]{40})") func (r *BlameReader) NextPart() (*BlamePart, error) { var blamePart *BlamePart - reader := r.reader + reader := bufio.NewReader(r.reader) if r.lastSha != nil { blamePart = &BlamePart{*r.lastSha, make([]string, 0)} @@ -99,51 +95,41 @@ func (r *BlameReader) NextPart() (*BlamePart, error) { // Close BlameReader - don't run NextPart after invoking that func (r *BlameReader) Close() error { - defer r.finished() // Only remove the process from the process table when the underlying command is closed - r.cancel() // However, first cancel our own context early - + err := <-r.done + _ = r.reader.Close() _ = r.output.Close() - - if err := r.cmd.Wait(); err != nil { - return fmt.Errorf("Wait: %w", err) - } - - return nil + return err } // CreateBlameReader creates reader for given repository, commit and file func CreateBlameReader(ctx context.Context, repoPath, commitID, file string) (*BlameReader, error) { - return createBlameReader(ctx, repoPath, GitExecutable, "blame", commitID, "--porcelain", "--", file) -} - -func createBlameReader(ctx context.Context, dir string, command ...string) (*BlameReader, error) { - // Here we use the provided context - this should be tied to the request performing the blame so that it does not hang around. - ctx, cancel, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("GetBlame [repo_path: %s]", dir)) - - cmd := exec.CommandContext(ctx, command[0], command[1:]...) - cmd.Dir = dir - cmd.Stderr = os.Stderr - process.SetSysProcAttribute(cmd) - - stdout, err := cmd.StdoutPipe() + cmd := NewCommandContextNoGlobals(ctx, "blame", "--porcelain"). + AddDynamicArguments(commitID). + AddDashesAndList(file). + SetDescription(fmt.Sprintf("GetBlame [repo_path: %s]", repoPath)) + reader, stdout, err := os.Pipe() if err != nil { - defer finished() - return nil, fmt.Errorf("StdoutPipe: %w", err) + return nil, err } - if err = cmd.Start(); err != nil { - defer finished() - _ = stdout.Close() - return nil, fmt.Errorf("Start: %w", err) - } + done := make(chan error, 1) - reader := bufio.NewReader(stdout) + go func(cmd *Command, dir string, stdout io.WriteCloser, done chan error) { + if err := cmd.Run(&RunOpts{ + UseContextTimeout: true, + Dir: dir, + Stdout: stdout, + Stderr: os.Stderr, + }); err == nil { + stdout.Close() + } + done <- err + }(cmd, repoPath, stdout, done) return &BlameReader{ - cmd: cmd, - output: stdout, - reader: reader, - cancel: cancel, - finished: finished, + cmd: cmd, + output: stdout, + reader: reader, + done: done, }, nil } diff --git a/modules/git/blame_test.go b/modules/git/blame_test.go index 94277b7c1d..a2c8fe8e75 100644 --- a/modules/git/blame_test.go +++ b/modules/git/blame_test.go @@ -5,139 +5,36 @@ package git import ( "context" - "os" "testing" "github.com/stretchr/testify/assert" ) -const exampleBlame = ` -4b92a6c2df28054ad766bc262f308db9f6066596 1 1 1 -author Unknown -author-mail -author-time 1392833071 -author-tz -0500 -committer Unknown -committer-mail -committer-time 1392833071 -committer-tz -0500 -summary Add code of delete user -previous be0ba9ea88aff8a658d0495d36accf944b74888d gogs.go -filename gogs.go - // Copyright 2014 The Gogs Authors. All rights reserved. -ce21ed6c3490cdfad797319cbb1145e2330a8fef 2 2 1 -author Joubert RedRat -author-mail -author-time 1482322397 -author-tz -0200 -committer Lunny Xiao -committer-mail -committer-time 1482322397 -committer-tz +0800 -summary Remove remaining Gogs reference on locales and cmd (#430) -previous 618407c018cdf668ceedde7454c42fb22ba422d8 main.go -filename main.go - // Copyright 2016 The Gitea Authors. All rights reserved. -4b92a6c2df28054ad766bc262f308db9f6066596 2 3 2 -author Unknown -author-mail -author-time 1392833071 -author-tz -0500 -committer Unknown -committer-mail -committer-time 1392833071 -committer-tz -0500 -summary Add code of delete user -previous be0ba9ea88aff8a658d0495d36accf944b74888d gogs.go -filename gogs.go - // Use of this source code is governed by a MIT-style -4b92a6c2df28054ad766bc262f308db9f6066596 3 4 -author Unknown -author-mail -author-time 1392833071 -author-tz -0500 -committer Unknown -committer-mail -committer-time 1392833071 -committer-tz -0500 -summary Add code of delete user -previous be0ba9ea88aff8a658d0495d36accf944b74888d gogs.go -filename gogs.go - // license that can be found in the LICENSE file. - -e2aa991e10ffd924a828ec149951f2f20eecead2 6 6 2 -author Lunny Xiao -author-mail -author-time 1478872595 -author-tz +0800 -committer Sandro Santilli -committer-mail -committer-time 1478872595 -committer-tz +0100 -summary ask for go get from code.gitea.io/gitea and change gogs to gitea on main file (#146) -previous 5fc370e332171b8658caed771b48585576f11737 main.go -filename main.go - // Gitea (git with a cup of tea) is a painless self-hosted Git Service. -e2aa991e10ffd924a828ec149951f2f20eecead2 7 7 - package main // import "code.gitea.io/gitea" -` - func TestReadingBlameOutput(t *testing.T) { - tempFile, err := os.CreateTemp("", ".txt") - if err != nil { - panic(err) - } - - defer tempFile.Close() - - if _, err = tempFile.WriteString(exampleBlame); err != nil { - panic(err) - } ctx, cancel := context.WithCancel(context.Background()) defer cancel() - blameReader, err := createBlameReader(ctx, "", "cat", tempFile.Name()) - if err != nil { - panic(err) - } + blameReader, err := CreateBlameReader(ctx, "./tests/repos/repo5_pulls", "f32b0a9dfd09a60f616f29158f772cedd89942d2", "README.md") + assert.NoError(t, err) defer blameReader.Close() parts := []*BlamePart{ { - "4b92a6c2df28054ad766bc262f308db9f6066596", + "72866af952e98d02a73003501836074b286a78f6", []string{ - "// Copyright 2014 The Gogs Authors. All rights reserved.", + "# test_repo", + "Test repository for testing migration from github to gitea", }, }, { - "ce21ed6c3490cdfad797319cbb1145e2330a8fef", - []string{ - "// Copyright 2016 The Gitea Authors. All rights reserved.", - }, + "f32b0a9dfd09a60f616f29158f772cedd89942d2", + []string{}, }, - { - "4b92a6c2df28054ad766bc262f308db9f6066596", - []string{ - "// Use of this source code is governed by a MIT-style", - "// license that can be found in the LICENSE file.", - "", - }, - }, - { - "e2aa991e10ffd924a828ec149951f2f20eecead2", - []string{ - "// Gitea (git with a cup of tea) is a painless self-hosted Git Service.", - "package main // import \"code.gitea.io/gitea\"", - }, - }, - nil, } for _, part := range parts { actualPart, err := blameReader.NextPart() - if err != nil { - panic(err) - } + assert.NoError(t, err) assert.Equal(t, part, actualPart) } }