diff --git a/go.mod b/go.mod index bfb87a1b37a8b..3fe86ec5816e9 100644 --- a/go.mod +++ b/go.mod @@ -18,8 +18,6 @@ require ( github.com/caddyserver/certmagic v0.15.4 github.com/chi-middleware/proxy v1.1.1 github.com/denisenkom/go-mssqldb v0.12.0 - github.com/djherbis/buffer v1.2.0 - github.com/djherbis/nio/v3 v3.0.1 github.com/duo-labs/webauthn v0.0.0-20220223184316-4d1cf2d34051 github.com/dustin/go-humanize v1.0.0 github.com/editorconfig/editorconfig-core-go/v2 v2.4.3 diff --git a/go.sum b/go.sum index d969c26bf533d..a5d6cea344595 100644 --- a/go.sum +++ b/go.sum @@ -369,11 +369,6 @@ github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/r github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc= github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no= github.com/dimchansky/utfbom v1.1.0/go.mod h1:rO41eb7gLfo8SF1jd9F8HplJm1Fewwi4mQvIirEdv+8= -github.com/djherbis/buffer v1.1.0/go.mod h1:VwN8VdFkMY0DCALdY8o00d3IZ6Amz/UNVMWcSaJT44o= -github.com/djherbis/buffer v1.2.0 h1:PH5Dd2ss0C7CRRhQCZ2u7MssF+No9ide8Ye71nPHcrQ= -github.com/djherbis/buffer v1.2.0/go.mod h1:fjnebbZjCUpPinBRD+TDwXSOeNQ7fPQWLfGQqiAiUyE= -github.com/djherbis/nio/v3 v3.0.1 h1:6wxhnuppteMa6RHA4L81Dq7ThkZH8SwnDzXDYy95vB4= -github.com/djherbis/nio/v3 v3.0.1/go.mod h1:Ng4h80pbZFMla1yKzm61cF0tqqilXZYrogmWgZxOcmg= github.com/dlclark/regexp2 v1.4.0 h1:F1rxgk7p4uKjwIQxBs9oAXe5CqrXlCduYEJvrF4u93E= github.com/dlclark/regexp2 v1.4.0/go.mod h1:2pZnwuY/m+8K6iRw6wQdMtk+rH5tNGR1i55kozfMjCc= github.com/dnaeon/go-vcr v1.2.0/go.mod h1:R4UdLID7HZT3taECzJs4YgbbH6PIGXB6W/sc5OLb6RQ= diff --git a/modules/git/batch_reader.go b/modules/git/batch_reader.go index 902fa897185f5..e30fd1a3e1eb7 100644 --- a/modules/git/batch_reader.go +++ b/modules/git/batch_reader.go @@ -9,24 +9,14 @@ import ( "bytes" "context" "fmt" - "io" "math" "runtime" "strconv" "strings" "code.gitea.io/gitea/modules/log" - - "github.com/djherbis/buffer" - "github.com/djherbis/nio/v3" ) -// WriteCloserError wraps an io.WriteCloser with an additional CloseWithError function -type WriteCloserError interface { - io.WriteCloser - CloseWithError(err error) error -} - // EnsureValidGitRepository runs git rev-parse in the repository path - thus ensuring that the repository is a valid repository. // Run before opening git cat-file. // This is needed otherwise the git cat-file will hang for invalid repositories. @@ -44,10 +34,21 @@ func EnsureValidGitRepository(ctx context.Context, repoPath string) error { return nil } +func returnClosedReaderWriters(err error) (WriteCloserError, *bufio.Reader, func()) { + wr := &ClosedReadWriteCloserError{err} + return wr, bufio.NewReader(wr), func() {} +} + // CatFileBatchCheck opens git cat-file --batch-check in the provided repo and returns a stdin pipe, a stdout reader and cancel function func CatFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError, *bufio.Reader, func()) { - batchStdinReader, batchStdinWriter := io.Pipe() - batchStdoutReader, batchStdoutWriter := io.Pipe() + pipes, err := NewPipePairs(2) + if err != nil { + log.Critical("Unable to open pipe to write to: %v", err) + return returnClosedReaderWriters(err) + } + batchStdinReader, batchStdinWriter := pipes[0].ReaderWriter() + batchStdoutReader, batchStdoutWriter := pipes[1].ReaderWriter() + ctx, ctxCancel := context.WithCancel(ctx) closed := make(chan struct{}) cancel := func() { @@ -57,12 +58,6 @@ func CatFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError, <-closed } - // Ensure cancel is called as soon as the provided context is cancelled - go func() { - <-ctx.Done() - cancel() - }() - _, filename, line, _ := runtime.Caller(2) filename = strings.TrimPrefix(filename, callerPrefix) @@ -77,8 +72,9 @@ func CatFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError, Stderr: &stderr, }) if err != nil { - _ = batchStdoutWriter.CloseWithError(ConcatenateError(err, (&stderr).String())) - _ = batchStdinReader.CloseWithError(ConcatenateError(err, (&stderr).String())) + err := ConcatenateError(err, (&stderr).String()) + _ = batchStdinReader.CloseWithError(err) + _ = batchStdoutWriter.CloseWithError(err) } else { _ = batchStdoutWriter.Close() _ = batchStdinReader.Close() @@ -88,7 +84,6 @@ func CatFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError, // For simplicities sake we'll use a buffered reader to read from the cat-file --batch-check batchReader := bufio.NewReader(batchStdoutReader) - return batchStdinWriter, batchReader, cancel } @@ -96,8 +91,14 @@ func CatFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError, func CatFileBatch(ctx context.Context, repoPath string) (WriteCloserError, *bufio.Reader, func()) { // We often want to feed the commits in order into cat-file --batch, followed by their trees and sub trees as necessary. // so let's create a batch stdin and stdout - batchStdinReader, batchStdinWriter := io.Pipe() - batchStdoutReader, batchStdoutWriter := nio.Pipe(buffer.New(32 * 1024)) + pipes, err := NewPipePairs(2) + if err != nil { + log.Critical("Unable to open pipe to write to: %v", err) + return returnClosedReaderWriters(err) + } + batchStdinReader, batchStdinWriter := pipes[0].ReaderWriter() + batchStdoutReader, batchStdoutWriter := pipes[1].ReaderWriter() + ctx, ctxCancel := context.WithCancel(ctx) closed := make(chan struct{}) cancel := func() { @@ -107,12 +108,6 @@ func CatFileBatch(ctx context.Context, repoPath string) (WriteCloserError, *bufi <-closed } - // Ensure cancel is called as soon as the provided context is cancelled - go func() { - <-ctx.Done() - cancel() - }() - _, filename, line, _ := runtime.Caller(2) filename = strings.TrimPrefix(filename, callerPrefix) @@ -127,8 +122,9 @@ func CatFileBatch(ctx context.Context, repoPath string) (WriteCloserError, *bufi Stderr: &stderr, }) if err != nil { - _ = batchStdoutWriter.CloseWithError(ConcatenateError(err, (&stderr).String())) - _ = batchStdinReader.CloseWithError(ConcatenateError(err, (&stderr).String())) + err := ConcatenateError(err, (&stderr).String()) + _ = batchStdinReader.CloseWithError(err) + _ = batchStdoutWriter.CloseWithError(err) } else { _ = batchStdoutWriter.Close() _ = batchStdinReader.Close() diff --git a/modules/git/blob.go b/modules/git/blob.go index 9567affd03745..34e5b533b94b1 100644 --- a/modules/git/blob.go +++ b/modules/git/blob.go @@ -9,6 +9,7 @@ import ( "bytes" "encoding/base64" "io" + "strings" "code.gitea.io/gitea/modules/typesniffer" "code.gitea.io/gitea/modules/util" @@ -69,25 +70,18 @@ func (b *Blob) GetBlobContentBase64() (string, error) { } defer dataRc.Close() - pr, pw := io.Pipe() - encoder := base64.NewEncoder(base64.StdEncoding, pw) + sb := &strings.Builder{} - go func() { - _, err := io.Copy(encoder, dataRc) - _ = encoder.Close() + encoder := base64.NewEncoder(base64.StdEncoding, sb) - if err != nil { - _ = pw.CloseWithError(err) - } else { - _ = pw.Close() - } - }() + _, err = io.Copy(encoder, dataRc) + _ = encoder.Close() - out, err := io.ReadAll(pr) if err != nil { return "", err } - return string(out), nil + + return sb.String(), nil } // GuessContentType guesses the content type of the blob. diff --git a/modules/git/command.go b/modules/git/command.go index 3dd12e421e409..b076e93ff679d 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -158,13 +158,50 @@ func (c *Command) Run(opts *RunOpts) error { ) cmd.Dir = opts.Dir - cmd.Stdout = opts.Stdout - cmd.Stderr = opts.Stderr - cmd.Stdin = opts.Stdin + if pipeWriter, ok := opts.Stdout.(*PipeWriter); ok { + cmd.Stdout = pipeWriter.File() + } else { + cmd.Stdout = opts.Stdout + } + if pipeWriter, ok := opts.Stderr.(*PipeWriter); ok { + cmd.Stderr = pipeWriter.File() + } else { + cmd.Stderr = opts.Stderr + } + if pipeReader, ok := opts.Stdin.(*PipeReader); ok { + cmd.Stdin = pipeReader.File() + } else { + cmd.Stdin = opts.Stdin + } if err := cmd.Start(); err != nil { return err } + // Ensure that closers are closed + closers := make([]io.Closer, 0, 3) + for _, pipe := range []interface{}{cmd.Stdout, cmd.Stdin, cmd.Stderr} { + if pipe == nil { + continue + } + if _, ok := pipe.(*os.File); ok { + continue + } + + if closer, ok := pipe.(io.Closer); ok { + closers = append(closers, closer) + } + } + + if len(closers) > 0 { + go func() { + <-ctx.Done() + cancel() + for _, closer := range closers { + _ = closer.Close() + } + }() + } + if opts.PipelineFunc != nil { err := opts.PipelineFunc(ctx, cancel) if err != nil { diff --git a/modules/git/commit.go b/modules/git/commit.go index 8337e54fef27d..a8e1c5824a1b3 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -12,6 +12,7 @@ import ( "errors" "fmt" "io" + "os" "os/exec" "strconv" "strings" @@ -475,7 +476,10 @@ func parseCommitFileStatus(fileStatus *CommitFileStatus, stdout io.Reader) { // GetCommitFileStatus returns file status of commit in given repository. func GetCommitFileStatus(ctx context.Context, repoPath, commitID string) (*CommitFileStatus, error) { - stdout, w := io.Pipe() + stdout, w, err := os.Pipe() + if err != nil { + return nil, err + } done := make(chan struct{}) fileStatus := NewCommitFileStatus() go func() { @@ -486,7 +490,7 @@ func GetCommitFileStatus(ctx context.Context, repoPath, commitID string) (*Commi stderr := new(bytes.Buffer) args := []string{"log", "--name-status", "-c", "--pretty=format:", "--parents", "--no-renames", "-z", "-1", commitID} - err := NewCommand(ctx, args...).Run(&RunOpts{ + err = NewCommand(ctx, args...).Run(&RunOpts{ Dir: repoPath, Stdout: w, Stderr: stderr, diff --git a/modules/git/log_name_status.go b/modules/git/log_name_status.go index ffd0a0991bf43..8819353bc3c1e 100644 --- a/modules/git/log_name_status.go +++ b/modules/git/log_name_status.go @@ -13,15 +13,19 @@ import ( "sort" "strings" - "github.com/djherbis/buffer" - "github.com/djherbis/nio/v3" + "code.gitea.io/gitea/modules/log" ) // LogNameStatusRepo opens git log --raw in the provided repo and returns a stdin pipe, a stdout reader and cancel function func LogNameStatusRepo(ctx context.Context, repository, head, treepath string, paths ...string) (*bufio.Reader, func()) { // We often want to feed the commits in order into cat-file --batch, followed by their trees and sub trees as necessary. // so let's create a batch stdin and stdout - stdoutReader, stdoutWriter := nio.Pipe(buffer.New(32 * 1024)) + stdoutReader, stdoutWriter, err := Pipe() + if err != nil { + log.Critical("Unable to open pipe to write to: %v", err) + rd := &ClosedReadWriteCloserError{err} + return bufio.NewReader(rd), func() {} + } // Lets also create a context so that we can absolutely ensure that the command should die when we're done ctx, ctxCancel := context.WithCancel(ctx) diff --git a/modules/git/pipeline/catfile.go b/modules/git/pipeline/catfile.go index 40dd2bca2936d..dea5fe18eb087 100644 --- a/modules/git/pipeline/catfile.go +++ b/modules/git/pipeline/catfile.go @@ -9,7 +9,6 @@ import ( "bytes" "context" "fmt" - "io" "strconv" "strings" "sync" @@ -19,7 +18,7 @@ import ( ) // CatFileBatchCheck runs cat-file with --batch-check -func CatFileBatchCheck(ctx context.Context, shasToCheckReader *io.PipeReader, catFileCheckWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath string) { +func CatFileBatchCheck(ctx context.Context, shasToCheckReader git.ReadCloserError, catFileCheckWriter git.WriteCloserError, wg *sync.WaitGroup, tmpBasePath string) { defer wg.Done() defer shasToCheckReader.Close() defer catFileCheckWriter.Close() @@ -38,7 +37,7 @@ func CatFileBatchCheck(ctx context.Context, shasToCheckReader *io.PipeReader, ca } // CatFileBatchCheckAllObjects runs cat-file with --batch-check --batch-all -func CatFileBatchCheckAllObjects(ctx context.Context, catFileCheckWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath string, errChan chan<- error) { +func CatFileBatchCheckAllObjects(ctx context.Context, catFileCheckWriter git.WriteCloserError, wg *sync.WaitGroup, tmpBasePath string, errChan chan<- error) { defer wg.Done() defer catFileCheckWriter.Close() @@ -58,7 +57,7 @@ func CatFileBatchCheckAllObjects(ctx context.Context, catFileCheckWriter *io.Pip } // CatFileBatch runs cat-file --batch -func CatFileBatch(ctx context.Context, shasToBatchReader *io.PipeReader, catFileBatchWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath string) { +func CatFileBatch(ctx context.Context, shasToBatchReader git.ReadCloserError, catFileBatchWriter git.WriteCloserError, wg *sync.WaitGroup, tmpBasePath string) { defer wg.Done() defer shasToBatchReader.Close() defer catFileBatchWriter.Close() @@ -76,7 +75,7 @@ func CatFileBatch(ctx context.Context, shasToBatchReader *io.PipeReader, catFile } // BlobsLessThan1024FromCatFileBatchCheck reads a pipeline from cat-file --batch-check and returns the blobs <1024 in size -func BlobsLessThan1024FromCatFileBatchCheck(catFileCheckReader *io.PipeReader, shasToBatchWriter *io.PipeWriter, wg *sync.WaitGroup) { +func BlobsLessThan1024FromCatFileBatchCheck(catFileCheckReader git.ReadCloserError, shasToBatchWriter git.WriteCloserError, wg *sync.WaitGroup) { defer wg.Done() defer catFileCheckReader.Close() scanner := bufio.NewScanner(catFileCheckReader) diff --git a/modules/git/pipeline/lfs.go b/modules/git/pipeline/lfs.go index 1b64b672e4582..77cb4365a454c 100644 --- a/modules/git/pipeline/lfs.go +++ b/modules/git/pipeline/lfs.go @@ -99,8 +99,15 @@ func FindLFSFile(repo *git.Repository, hash git.SHA1) ([]*LFSResult, error) { sort.Sort(lfsResultSlice(results)) // Should really use a go-git function here but name-rev is not completed and recapitulating it is not simple - shasToNameReader, shasToNameWriter := io.Pipe() - nameRevStdinReader, nameRevStdinWriter := io.Pipe() + pipes, err := git.NewPipePairs(2) + if err != nil { + return nil, err + } + defer pipes.Close() + + shasToNameReader, shasToNameWriter := pipes[0].ReaderWriter() + nameRevStdinReader, nameRevStdinWriter := pipes[1].ReaderWriter() + errChan := make(chan error, 1) wg := sync.WaitGroup{} wg.Add(3) diff --git a/modules/git/pipeline/lfs_nogogit.go b/modules/git/pipeline/lfs_nogogit.go index 31c10c6002f6f..4fac5b5c246c8 100644 --- a/modules/git/pipeline/lfs_nogogit.go +++ b/modules/git/pipeline/lfs_nogogit.go @@ -45,7 +45,10 @@ func FindLFSFile(repo *git.Repository, hash git.SHA1) ([]*LFSResult, error) { basePath := repo.Path // Use rev-list to provide us with all commits in order - revListReader, revListWriter := io.Pipe() + revListReader, revListWriter, err := git.Pipe() + if err != nil { + return nil, err + } defer func() { _ = revListWriter.Close() _ = revListReader.Close() @@ -195,8 +198,15 @@ func FindLFSFile(repo *git.Repository, hash git.SHA1) ([]*LFSResult, error) { sort.Sort(lfsResultSlice(results)) // Should really use a go-git function here but name-rev is not completed and recapitulating it is not simple - shasToNameReader, shasToNameWriter := io.Pipe() - nameRevStdinReader, nameRevStdinWriter := io.Pipe() + pipes, err := git.NewPipePairs(2) + if err != nil { + return nil, err + } + defer pipes.Close() + + shasToNameReader, shasToNameWriter := pipes[0].ReaderWriter() + nameRevStdinReader, nameRevStdinWriter := pipes[1].ReaderWriter() + errChan := make(chan error, 1) wg := sync.WaitGroup{} wg.Add(3) diff --git a/modules/git/pipeline/namerev.go b/modules/git/pipeline/namerev.go index 8356e70234459..421b80830de17 100644 --- a/modules/git/pipeline/namerev.go +++ b/modules/git/pipeline/namerev.go @@ -8,7 +8,6 @@ import ( "bytes" "context" "fmt" - "io" "strings" "sync" @@ -16,7 +15,7 @@ import ( ) // NameRevStdin runs name-rev --stdin -func NameRevStdin(ctx context.Context, shasToNameReader *io.PipeReader, nameRevStdinWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath string) { +func NameRevStdin(ctx context.Context, shasToNameReader git.ReadCloserError, nameRevStdinWriter git.WriteCloserError, wg *sync.WaitGroup, tmpBasePath string) { defer wg.Done() defer shasToNameReader.Close() defer nameRevStdinWriter.Close() diff --git a/modules/git/pipeline/revlist.go b/modules/git/pipeline/revlist.go index 02619cb58304f..c24cbb310f6af 100644 --- a/modules/git/pipeline/revlist.go +++ b/modules/git/pipeline/revlist.go @@ -9,7 +9,6 @@ import ( "bytes" "context" "fmt" - "io" "strings" "sync" @@ -18,7 +17,7 @@ import ( ) // RevListAllObjects runs rev-list --objects --all and writes to a pipewriter -func RevListAllObjects(ctx context.Context, revListWriter *io.PipeWriter, wg *sync.WaitGroup, basePath string, errChan chan<- error) { +func RevListAllObjects(ctx context.Context, revListWriter git.WriteCloserError, wg *sync.WaitGroup, basePath string, errChan chan<- error) { defer wg.Done() defer revListWriter.Close() @@ -38,7 +37,7 @@ func RevListAllObjects(ctx context.Context, revListWriter *io.PipeWriter, wg *sy } // RevListObjects run rev-list --objects from headSHA to baseSHA -func RevListObjects(ctx context.Context, revListWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath, headSHA, baseSHA string, errChan chan<- error) { +func RevListObjects(ctx context.Context, revListWriter git.WriteCloserError, wg *sync.WaitGroup, tmpBasePath, headSHA, baseSHA string, errChan chan<- error) { defer wg.Done() defer revListWriter.Close() stderr := new(bytes.Buffer) @@ -55,7 +54,8 @@ func RevListObjects(ctx context.Context, revListWriter *io.PipeWriter, wg *sync. } // BlobsFromRevListObjects reads a RevListAllObjects and only selects blobs -func BlobsFromRevListObjects(revListReader *io.PipeReader, shasToCheckWriter *io.PipeWriter, wg *sync.WaitGroup) { +// NOTE: Does not call git +func BlobsFromRevListObjects(revListReader git.ReadCloserError, shasToCheckWriter git.WriteCloserError, wg *sync.WaitGroup) { defer wg.Done() defer revListReader.Close() scanner := bufio.NewScanner(revListReader) diff --git a/modules/git/repo.go b/modules/git/repo.go index 3176e276959a0..59c02b4c0618b 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -329,18 +329,7 @@ func (repo *Repository) CreateBundle(ctx context.Context, commit string, out io. return err } - tmpFile := filepath.Join(tmp, "bundle") - _, _, err = NewCommand(ctx, "bundle", "create", tmpFile, "bundle", "HEAD").RunStdString(&RunOpts{Dir: tmp, Env: env}) - if err != nil { - return err - } - - fi, err := os.Open(tmpFile) - if err != nil { - return err - } - defer fi.Close() + _, _, err = NewCommand(ctx, "bundle", "create", "-", "bundle", "HEAD").RunStdString(&RunOpts{Dir: tmp, Env: env, Stdout: out}) - _, err = io.Copy(out, fi) return err } diff --git a/modules/git/repo_branch_nogogit.go b/modules/git/repo_branch_nogogit.go index 4393db10f9504..afc4f72682a97 100644 --- a/modules/git/repo_branch_nogogit.go +++ b/modules/git/repo_branch_nogogit.go @@ -100,7 +100,10 @@ func callShowRef(ctx context.Context, repoPath, prefix, arg string, skip, limit } func walkShowRef(ctx context.Context, repoPath, arg string, skip, limit int, walkfn func(sha1, refname string) error) (countAll int, err error) { - stdoutReader, stdoutWriter := io.Pipe() + stdoutReader, stdoutWriter, err := Pipe() + if err != nil { + return 0, err + } defer func() { _ = stdoutReader.Close() _ = stdoutWriter.Close() diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index e6fec4d1a32e2..c7153345fbc91 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -201,7 +201,10 @@ func (repo *Repository) FileCommitsCount(revision, file string) (int64, error) { func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ([]*Commit, error) { skip := (page - 1) * setting.Git.CommitsRangeSize - stdoutReader, stdoutWriter := io.Pipe() + stdoutReader, stdoutWriter, err := Pipe() + if err != nil { + return nil, err + } defer func() { _ = stdoutReader.Close() _ = stdoutWriter.Close() diff --git a/modules/git/repo_ref_nogogit.go b/modules/git/repo_ref_nogogit.go index 40e8a247c7488..58bed28c07b26 100644 --- a/modules/git/repo_ref_nogogit.go +++ b/modules/git/repo_ref_nogogit.go @@ -15,7 +15,10 @@ import ( // GetRefsFiltered returns all references of the repository that matches patterm exactly or starting with. func (repo *Repository) GetRefsFiltered(pattern string) ([]*Reference, error) { - stdoutReader, stdoutWriter := io.Pipe() + stdoutReader, stdoutWriter, err := Pipe() + if err != nil { + return nil, err + } defer func() { _ = stdoutReader.Close() _ = stdoutWriter.Close() diff --git a/modules/git/repo_tag.go b/modules/git/repo_tag.go index 8444e8d035a0b..a1bfea5e37f4a 100644 --- a/modules/git/repo_tag.go +++ b/modules/git/repo_tag.go @@ -8,7 +8,6 @@ package git import ( "context" "fmt" - "io" "strings" "code.gitea.io/gitea/modules/git/foreachref" @@ -115,7 +114,10 @@ func (repo *Repository) GetTagWithID(idStr, name string) (*Tag, error) { func (repo *Repository) GetTagInfos(page, pageSize int) ([]*Tag, int, error) { forEachRefFmt := foreachref.NewFormat("objecttype", "refname:short", "object", "objectname", "creator", "contents", "contents:signature") - stdoutReader, stdoutWriter := io.Pipe() + stdoutReader, stdoutWriter, err := Pipe() + if err != nil { + return nil, 0, err + } defer stdoutReader.Close() defer stdoutWriter.Close() stderr := strings.Builder{} diff --git a/modules/git/utils_closererror.go b/modules/git/utils_closererror.go new file mode 100644 index 0000000000000..64ef217eda0c1 --- /dev/null +++ b/modules/git/utils_closererror.go @@ -0,0 +1,48 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package git + +import ( + "io" +) + +// WriteCloserError wraps an io.WriteCloser with an additional CloseWithError function +type WriteCloserError interface { + io.WriteCloser + CloseWithError(err error) error +} + +// ReadCloserError wraps an io.ReadCloser with an additional CloseWithError function +type ReadCloserError interface { + io.ReadCloser + CloseWithError(err error) error +} + +// CloserError wraps an io.Closer with an additional CloseWithError function +type CloserError interface { + io.Closer + CloseWithError(err error) error +} + +// ClosedReadWriteCloserError is a pre-closed ReadWriteCloserError +type ClosedReadWriteCloserError struct { + err error +} + +func (c *ClosedReadWriteCloserError) Read(p []byte) (n int, err error) { + return 0, c.err +} + +func (c *ClosedReadWriteCloserError) Write(p []byte) (n int, err error) { + return 0, c.err +} + +func (c *ClosedReadWriteCloserError) Close() error { + return c.err +} + +func (c *ClosedReadWriteCloserError) CloseWithError(error) error { + return c.err +} diff --git a/modules/git/utils_pipe.go b/modules/git/utils_pipe.go new file mode 100644 index 0000000000000..1e7e79569a3a9 --- /dev/null +++ b/modules/git/utils_pipe.go @@ -0,0 +1,243 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package git + +import ( + "io" + "os" + "sync" +) + +// PipePair represents an os.Pipe() wrapped with a io.Pipe()-like PipeWriter and PipeReader +type PipePair struct { + rd *os.File + wr *os.File + + lock sync.Mutex + wrErr error + rdErr error +} + +// ReaderWriter returns the Reader and Writer ends of the Pipe +func (p *PipePair) ReaderWriter() (*PipeReader, *PipeWriter) { + return p.Reader(), p.Writer() +} + +// Reader returns the Reader end of the Pipe +func (p *PipePair) Reader() *PipeReader { + return &PipeReader{p} +} + +func (p *PipePair) read(b []byte) (n int, err error) { + n, err = p.rd.Read(b) + if err != nil { + return n, p.readCloseError(err) + } + return +} + +func (p *PipePair) closeRead(err error) error { + if err == nil { + err = io.ErrClosedPipe + } + p.lock.Lock() + defer p.lock.Unlock() + if p.rdErr != nil { + return nil + } + + p.rdErr = err + _ = p.rd.Close() + return nil +} + +// readCloseError returns the error returned on reading a closed read pipe +func (p *PipePair) readCloseError(err error) error { + p.lock.Lock() + defer p.lock.Unlock() + rdErr := p.rdErr + + if wrErr := p.wrErr; rdErr == nil && wrErr != nil { + return wrErr + } + if err != io.EOF { + return err + } + return io.ErrClosedPipe +} + +// Writer returns the Writer end of the Pipe +func (p *PipePair) Writer() *PipeWriter { + return &PipeWriter{p} +} + +func (p *PipePair) write(b []byte) (n int, err error) { + n, err = p.wr.Write(b) + if err != nil { + return n, p.writeCloseError(err) + } + return +} + +func (p *PipePair) closeWrite(err error) error { + if err == nil { + err = io.EOF + } + p.lock.Lock() + defer p.lock.Unlock() + if p.wrErr != nil { + return nil + } + p.wrErr = err + _ = p.wr.Close() + return nil +} + +// writeCloseError returns the error returned on writing to a closed write pipe +func (p *PipePair) writeCloseError(err error) error { + p.lock.Lock() + defer p.lock.Unlock() + wrErr := p.wrErr + if rdErr := p.rdErr; wrErr == nil && rdErr != nil { + return rdErr + } + if err != io.EOF { + return err + } + return io.ErrClosedPipe +} + +// Close closes the pipe pair +func (p *PipePair) Close() error { + return p.CloseWithError(nil) +} + +// CloseWithError closes the pipe pair +func (p *PipePair) CloseWithError(err error) error { + _ = p.closeRead(err) + _ = p.closeWrite(err) + return nil +} + +// PipeReader is the read half of a pipe +type PipeReader struct { + p *PipePair +} + +// Read implements the standard Read interface. +// If the write end is closed with an error, that error is +// returned as err; otherwise err is EOF. +func (r *PipeReader) Read(data []byte) (n int, err error) { + return r.p.read(data) +} + +// Close closes the reader; subsequent writes to the +// write half of the pipe will return the error ErrClosedPipe. +func (r *PipeReader) Close() error { + return r.CloseWithError(nil) +} + +// CloseWithError closes the reader; subsequent writes +// to the write half of the pipe will return the error err. +// +// CloseWithError never overwrites the previous error if it exists +// and always returns nil. +func (r *PipeReader) CloseWithError(err error) error { + return r.p.closeRead(err) +} + +// File returns the underlying os.File for this PipeReader +func (r *PipeReader) File() *os.File { + return r.p.rd +} + +// PipeWriter is the write half of a pipe. +type PipeWriter struct { + p *PipePair +} + +// Write implements the standard Write interface: +// If the read end is closed with an error, that err is +// returned as err; otherwise err is ErrClosedPipe. +func (w *PipeWriter) Write(data []byte) (n int, err error) { + return w.p.write(data) +} + +// Close closes the writer; subsequent reads from the +// read half of the pipe will return no bytes and EOF. +func (w *PipeWriter) Close() error { + return w.CloseWithError(nil) +} + +// CloseWithError closes the writer; subsequent reads from the +// read half of the pipe will return no bytes and the error err, +// or EOF if err is nil. +// +// CloseWithError never overwrites the previous error if it exists +// and always returns nil. +func (w *PipeWriter) CloseWithError(err error) error { + return w.p.closeWrite(err) +} + +// File returns the underlying os.File for this PipeWriter +func (w *PipeWriter) File() *os.File { + return w.p.wr +} + +// Pipe returns a connected pair of Files wrapped with CloserError similar to io.Pipe(). +// Reads from r return bytes written to w. It returns the files and an error, if any. +func Pipe() (*PipeReader, *PipeWriter, error) { + rd, wr, err := os.Pipe() + if err != nil { + return nil, nil, err + } + + pipe := &PipePair{rd: rd, wr: wr} + + return pipe.Reader(), pipe.Writer(), nil +} + +// NewPipePair returns a connected pair of Files wrapped in a PipePair +func NewPipePair() (*PipePair, error) { + rd, wr, err := os.Pipe() + if err != nil { + return nil, err + } + + return &PipePair{rd: rd, wr: wr}, nil +} + +// NewPipePairs will return a slice of n PipePairs or an error +func NewPipePairs(n int) (PipePairs, error) { + pipePairs := make([]*PipePair, 0, n) + for i := 0; i < n; i++ { + pipe, err := NewPipePair() + if err != nil { + for _, pipe := range pipePairs { + _ = pipe.Close() + } + return nil, err + } + + pipePairs = append(pipePairs, pipe) + } + return pipePairs, nil +} + +type PipePairs []*PipePair + +// Close closes the pipe pairs +func (pairs PipePairs) Close() error { + return pairs.CloseWithError(nil) +} + +// CloseWithError closes the pipe pairs +func (pairs PipePairs) CloseWithError(err error) error { + for _, p := range pairs { + _ = p.closeRead(err) + _ = p.closeWrite(err) + } + return nil +} diff --git a/modules/lfs/pointer_scanner_nogogit.go b/modules/lfs/pointer_scanner_nogogit.go index cdf88c51b009e..2af726469e4c0 100644 --- a/modules/lfs/pointer_scanner_nogogit.go +++ b/modules/lfs/pointer_scanner_nogogit.go @@ -23,9 +23,28 @@ import ( func SearchPointerBlobs(ctx context.Context, repo *git.Repository, pointerChan chan<- PointerBlob, errChan chan<- error) { basePath := repo.Path - catFileCheckReader, catFileCheckWriter := io.Pipe() - shasToBatchReader, shasToBatchWriter := io.Pipe() - catFileBatchReader, catFileBatchWriter := io.Pipe() + // We will need to run batch-check on all the objects in the repository + // in Git 2.6.0+ we can use `git cat-file --batch-check --batch-all-objects` + // However in earlier versions we'll need to use rev-list to get the objects. + gitHasBatchCheckAllObjects := git.CheckGitVersionAtLeast("2.6.0") == nil + + numPipesRequired := 3 + if !gitHasBatchCheckAllObjects { + numPipesRequired += 2 + } + + pipes, err := git.NewPipePairs(numPipesRequired) + if err != nil { + errChan <- err + close(pointerChan) + close(errChan) + return + } + defer pipes.Close() + + catFileCheckReader, catFileCheckWriter := pipes[0].ReaderWriter() + shasToBatchReader, shasToBatchWriter := pipes[1].ReaderWriter() + catFileBatchReader, catFileBatchWriter := pipes[2].ReaderWriter() wg := sync.WaitGroup{} wg.Add(4) @@ -43,9 +62,10 @@ func SearchPointerBlobs(ctx context.Context, repo *git.Repository, pointerChan c go pipeline.BlobsLessThan1024FromCatFileBatchCheck(catFileCheckReader, shasToBatchWriter, &wg) // 1. Run batch-check on all objects in the repository - if git.CheckGitVersionAtLeast("2.6.0") != nil { - revListReader, revListWriter := io.Pipe() - shasToCheckReader, shasToCheckWriter := io.Pipe() + if !gitHasBatchCheckAllObjects { + revListReader, revListWriter := pipes[3].ReaderWriter() + shasToCheckReader, shasToCheckWriter := pipes[4].ReaderWriter() + wg.Add(2) go pipeline.CatFileBatchCheck(ctx, shasToCheckReader, catFileCheckWriter, &wg, basePath) go pipeline.BlobsFromRevListObjects(revListReader, shasToCheckWriter, &wg) @@ -59,7 +79,8 @@ func SearchPointerBlobs(ctx context.Context, repo *git.Repository, pointerChan c close(errChan) } -func createPointerResultsFromCatFileBatch(ctx context.Context, catFileBatchReader *io.PipeReader, wg *sync.WaitGroup, pointerChan chan<- PointerBlob) { +// createPointerResultsFromCatFileBatch does not call git +func createPointerResultsFromCatFileBatch(ctx context.Context, catFileBatchReader git.ReadCloserError, wg *sync.WaitGroup, pointerChan chan<- PointerBlob) { defer wg.Done() defer catFileBatchReader.Close() diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 1df16e50167cf..b037db459867a 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1369,13 +1369,17 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff diffArgs = append(diffArgs, files...) } - reader, writer := io.Pipe() + reader, writer, err := git.Pipe() + if err != nil { + return nil, err + } + defer func() { _ = reader.Close() _ = writer.Close() }() - go func(ctx context.Context, diffArgs []string, repoPath string, writer *io.PipeWriter) { + go func(ctx context.Context, diffArgs []string, repoPath string, writer git.WriteCloserError) { cmd := git.NewCommand(ctx, diffArgs...) cmd.SetDescription(fmt.Sprintf("GetDiffRange [repo_path: %s]", repoPath)) if err := cmd.Run(&git.RunOpts{ diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index 34dd59d7fc0d5..ceb014a4a102f 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -751,21 +751,10 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error { continue } - var patch string - reader, writer := io.Pipe() - defer func() { - _ = reader.Close() - _ = writer.Close() - }() - go func(comment *base.ReviewComment) { - if err := git.GetRepoRawDiffForFile(g.gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, comment.TreePath, writer); err != nil { - // We should ignore the error since the commit maybe removed when force push to the pull request - log.Warn("GetRepoRawDiffForFile failed when migrating [%s, %s, %s, %s]: %v", g.gitRepo.Path, pr.MergeBase, headCommitID, comment.TreePath, err) - } - _ = writer.Close() - }(comment) - - patch, _ = git.CutDiffAroundLine(reader, int64((&models.Comment{Line: int64(line + comment.Position - 1)}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines) + patch, err := patchForComment(g, comment, line, pr, headCommitID) + if err != nil { + return err + } if comment.CreatedAt.IsZero() { comment.CreatedAt = review.CreatedAt @@ -799,6 +788,29 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error { return models.InsertReviews(cms) } +func patchForComment(g *GiteaLocalUploader, comment *base.ReviewComment, line int, pr *models.PullRequest, headCommitID string) (string, error) { + reader, writer, err := git.Pipe() + if err != nil { + log.Error("GetRepoRawDiffForFile failed to create pipe when migrating [%s, %s, %s, %s]: %v", g.gitRepo.Path, pr.MergeBase, headCommitID, comment.TreePath, err) + return "", err + } + defer func() { + _ = reader.Close() + _ = writer.Close() + }() + go func(comment *base.ReviewComment) { + if err := git.GetRepoRawDiffForFile(g.gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, comment.TreePath, writer); err != nil { + // We should ignore the error since the commit maybe removed when force push to the pull request + log.Warn("GetRepoRawDiffForFile failed when migrating [%s, %s, %s, %s]: %v", g.gitRepo.Path, pr.MergeBase, headCommitID, comment.TreePath, err) + } + _ = writer.Close() + }(comment) + + patch, _ := git.CutDiffAroundLine(reader, int64((&models.Comment{Line: int64(line + comment.Position - 1)}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines) + + return patch, nil +} + // Rollback when migrating failed, this will rollback all the changes. func (g *GiteaLocalUploader) Rollback() error { if g.repo != nil && g.repo.ID > 0 { diff --git a/services/pull/lfs.go b/services/pull/lfs.go index fada9b6121fa5..08741e963802d 100644 --- a/services/pull/lfs.go +++ b/services/pull/lfs.go @@ -13,6 +13,7 @@ import ( "sync" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git/pipeline" "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/log" @@ -26,11 +27,19 @@ func LFSPush(ctx context.Context, tmpBasePath, mergeHeadSHA, mergeBaseSHA string // ensure only blobs and <=1k size then pass in to git cat-file --batch // to read each sha and check each as a pointer // Then if they are lfs -> add them to the baseRepo - revListReader, revListWriter := io.Pipe() - shasToCheckReader, shasToCheckWriter := io.Pipe() - catFileCheckReader, catFileCheckWriter := io.Pipe() - shasToBatchReader, shasToBatchWriter := io.Pipe() - catFileBatchReader, catFileBatchWriter := io.Pipe() + + pipes, err := git.NewPipePairs(5) + if err != nil { + return err + } + defer pipes.Close() + + revListReader, revListWriter := pipes[0].ReaderWriter() + shasToCheckReader, shasToCheckWriter := pipes[1].ReaderWriter() + catFileCheckReader, catFileCheckWriter := pipes[2].ReaderWriter() + shasToBatchReader, shasToBatchWriter := pipes[3].ReaderWriter() + catFileBatchReader, catFileBatchWriter := pipes[4].ReaderWriter() + errChan := make(chan error, 1) wg := sync.WaitGroup{} wg.Add(6) @@ -67,7 +76,7 @@ func LFSPush(ctx context.Context, tmpBasePath, mergeHeadSHA, mergeBaseSHA string return nil } -func createLFSMetaObjectsFromCatFileBatch(catFileBatchReader *io.PipeReader, wg *sync.WaitGroup, pr *models.PullRequest) { +func createLFSMetaObjectsFromCatFileBatch(catFileBatchReader git.ReadCloserError, wg *sync.WaitGroup, pr *models.PullRequest) { defer wg.Done() defer catFileBatchReader.Close() diff --git a/services/pull/review.go b/services/pull/review.go index e7e6f3135ba91..41f99f38328d7 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -8,7 +8,6 @@ package pull import ( "context" "fmt" - "io" "regexp" "strings" @@ -183,7 +182,11 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo if len(commitID) == 0 { commitID = headCommitID } - reader, writer := io.Pipe() + reader, writer, err := git.Pipe() + if err != nil { + log.Critical("Unable to open pipe whilst generating patch: %v", err) + return nil, err + } defer func() { _ = reader.Close() _ = writer.Close() diff --git a/services/repository/archiver/archiver.go b/services/repository/archiver/archiver.go index ebd3eaf236ab9..59e9da6ee7579 100644 --- a/services/repository/archiver/archiver.go +++ b/services/repository/archiver/archiver.go @@ -8,7 +8,6 @@ import ( "context" "errors" "fmt" - "io" "os" "regexp" "strings" @@ -167,7 +166,10 @@ func doArchive(r *ArchiveRequest) (*repo_model.RepoArchiver, error) { return nil, fmt.Errorf("unable to stat archive: %v", err) } - rd, w := io.Pipe() + rd, w, err := git.Pipe() + if err != nil { + return nil, err + } defer func() { w.Close() rd.Close() @@ -184,7 +186,7 @@ func doArchive(r *ArchiveRequest) (*repo_model.RepoArchiver, error) { } defer gitRepo.Close() - go func(done chan error, w *io.PipeWriter, archiver *repo_model.RepoArchiver, gitRepo *git.Repository) { + go func(done chan error, w git.WriteCloserError, archiver *repo_model.RepoArchiver, gitRepo *git.Repository) { defer func() { if r := recover(); r != nil { done <- fmt.Errorf("%v", r)