Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use git command instead of exec.Cmd in blame #22098

Merged
merged 18 commits into from
Jan 3, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 31 additions & 45 deletions modules/git/blame.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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})")
Expand All @@ -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)}
Expand Down Expand Up @@ -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").
AddDynamicArguments(commitID).
AddArguments("--porcelain").
lunny marked this conversation as resolved.
Show resolved Hide resolved
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{
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
}
119 changes: 8 additions & 111 deletions modules/git/blame_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 <[email protected]>
author-time 1392833071
author-tz -0500
committer Unknown
committer-mail <[email protected]>
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 <[email protected]>
author-time 1482322397
author-tz -0200
committer Lunny Xiao
committer-mail <[email protected]>
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 <[email protected]>
author-time 1392833071
author-tz -0500
committer Unknown
committer-mail <[email protected]>
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 <[email protected]>
author-time 1392833071
author-tz -0500
committer Unknown
committer-mail <[email protected]>
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 <[email protected]>
author-time 1478872595
author-tz +0800
committer Sandro Santilli
committer-mail <[email protected]>
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",
[]string{
"// Copyright 2014 The Gogs Authors. All rights reserved.",
},
},
{
"ce21ed6c3490cdfad797319cbb1145e2330a8fef",
[]string{
"// Copyright 2016 The Gitea Authors. All rights reserved.",
},
},
{
"4b92a6c2df28054ad766bc262f308db9f6066596",
"72866af952e98d02a73003501836074b286a78f6",
[]string{
"// Use of this source code is governed by a MIT-style",
"// license that can be found in the LICENSE file.",
"",
"# test_repo",
"Test repository for testing migration from github to gitea",
},
},
{
"e2aa991e10ffd924a828ec149951f2f20eecead2",
[]string{
"// Gitea (git with a cup of tea) is a painless self-hosted Git Service.",
"package main // import \"code.gitea.io/gitea\"",
},
"f32b0a9dfd09a60f616f29158f772cedd89942d2",
[]string{},
},
nil,
}

for _, part := range parts {
actualPart, err := blameReader.NextPart()
if err != nil {
panic(err)
}
assert.NoError(t, err)
assert.Equal(t, part, actualPart)
}
}