diff --git a/assets/go-licenses.json b/assets/go-licenses.json index b105757683..5ee65f2c8a 100644 --- a/assets/go-licenses.json +++ b/assets/go-licenses.json @@ -409,16 +409,6 @@ "path": "github.com/dimiro1/reply/LICENSE", "licenseText": "MIT License\n\nCopyright (c) Discourse\nCopyright (c) Claudemiro\n\nPermission is hereby granted, free of charge, to any person obtaining a copy\nof this software and associated documentation files (the \"Software\"), to deal\nin the Software without restriction, including without limitation the rights\nto use, copy, modify, merge, publish, distribute, sublicense, and/or sell\ncopies of the Software, and to permit persons to whom the Software is\nfurnished to do so, subject to the following conditions:\n\nThe above copyright notice and this permission notice shall be included in all\ncopies or substantial portions of the Software.\n\nTHE SOFTWARE IS PROVIDED \"AS IS\", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR\nIMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,\nFITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE\nAUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER\nLIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,\nOUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE\nSOFTWARE.\n" }, - { - "name": "github.com/djherbis/buffer", - "path": "github.com/djherbis/buffer/LICENSE.txt", - "licenseText": "The MIT License (MIT)\n\nCopyright (c) 2015 Dustin H\n\nPermission is hereby granted, free of charge, to any person obtaining a copy of\nthis software and associated documentation files (the \"Software\"), to deal in\nthe Software without restriction, including without limitation the rights to\nuse, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of\nthe Software, and to permit persons to whom the Software is furnished to do so,\nsubject to the following conditions:\n\nThe above copyright notice and this permission notice shall be included in all\ncopies or substantial portions of the Software.\n\nTHE SOFTWARE IS PROVIDED \"AS IS\", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR\nIMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS\nFOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR\nCOPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER\nIN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN\nCONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.\n" - }, - { - "name": "github.com/djherbis/nio/v3", - "path": "github.com/djherbis/nio/v3/LICENSE.txt", - "licenseText": "The MIT License (MIT)\n\nCopyright (c) 2015 Dustin H\n\nPermission is hereby granted, free of charge, to any person obtaining a copy of\nthis software and associated documentation files (the \"Software\"), to deal in\nthe Software without restriction, including without limitation the rights to\nuse, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of\nthe Software, and to permit persons to whom the Software is furnished to do so,\nsubject to the following conditions:\n\nThe above copyright notice and this permission notice shall be included in all\ncopies or substantial portions of the Software.\n\nTHE SOFTWARE IS PROVIDED \"AS IS\", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR\nIMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS\nFOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR\nCOPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER\nIN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN\nCONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.\n" - }, { "name": "github.com/dlclark/regexp2", "path": "github.com/dlclark/regexp2/LICENSE", diff --git a/go.mod b/go.mod index 5843630970..eabfb39a61 100644 --- a/go.mod +++ b/go.mod @@ -39,8 +39,6 @@ require ( github.com/charmbracelet/git-lfs-transfer v0.1.1-0.20251013092601-6327009efd21 github.com/chi-middleware/proxy v1.1.1 github.com/dimiro1/reply v0.0.0-20200315094148-d0136a4c9e21 - github.com/djherbis/buffer v1.2.0 - github.com/djherbis/nio/v3 v3.0.1 github.com/dsnet/compress v0.0.2-0.20230904184137-39efe44ab707 github.com/dustin/go-humanize v1.0.1 github.com/editorconfig/editorconfig-core-go/v2 v2.6.3 diff --git a/go.sum b/go.sum index 814766e79b..acd5d8912e 100644 --- a/go.sum +++ b/go.sum @@ -260,11 +260,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/dimiro1/reply v0.0.0-20200315094148-d0136a4c9e21 h1:PdsjTl0Cg+ZJgOx/CFV5NNgO1ThTreqdgKYiDCMHJwA= github.com/dimiro1/reply v0.0.0-20200315094148-d0136a4c9e21/go.mod h1:xJvkyD6Y2rZapGvPJLYo9dyx1s5dxBEDPa8T3YTuOk0= -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.2.0/go.mod h1:2pZnwuY/m+8K6iRw6wQdMtk+rH5tNGR1i55kozfMjCc= github.com/dlclark/regexp2 v1.4.0/go.mod h1:2pZnwuY/m+8K6iRw6wQdMtk+rH5tNGR1i55kozfMjCc= github.com/dlclark/regexp2 v1.7.0/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8= diff --git a/modules/git/attribute/batch.go b/modules/git/attribute/batch.go index 184d9027ee..b1e6387ade 100644 --- a/modules/git/attribute/batch.go +++ b/modules/git/attribute/batch.go @@ -7,7 +7,7 @@ import ( "bytes" "context" "fmt" - "os" + "io" "path/filepath" "time" @@ -20,7 +20,7 @@ import ( type BatchChecker struct { attributesNum int repo *git.Repository - stdinWriter *os.File + stdinWriter io.WriteCloser stdOut *nulSeparatedAttributeWriter ctx context.Context cancel context.CancelFunc @@ -60,10 +60,7 @@ func NewBatchChecker(repo *git.Repository, treeish string, attributes []string) }, } - stdinReader, stdinWriter, err := os.Pipe() - if err != nil { - return nil, err - } + stdinWriter, stdinWriterClose := cmd.MakeStdinPipe() checker.stdinWriter = stdinWriter lw := new(nulSeparatedAttributeWriter) @@ -71,21 +68,19 @@ func NewBatchChecker(repo *git.Repository, treeish string, attributes []string) lw.closed = make(chan struct{}) checker.stdOut = lw - go func() { - defer func() { - _ = stdinReader.Close() - _ = lw.Close() - }() - err := cmd.WithEnv(envs). - WithDir(repo.Path). - WithStdin(stdinReader). - WithStdout(lw). - RunWithStderr(ctx) + cmd.WithEnv(envs). + WithDir(repo.Path). + WithStdoutCopy(lw) + go func() { + defer stdinWriterClose() + defer checker.cancel() + defer lw.Close() + + err := cmd.RunWithStderr(ctx) if err != nil && !gitcmd.IsErrorCanceledOrKilled(err) { log.Error("Attribute checker for commit %s exits with error: %v", treeish, err) } - checker.cancel() }() return checker, nil diff --git a/modules/git/attribute/checker.go b/modules/git/attribute/checker.go index 4b86d7d9ba..3eea31e813 100644 --- a/modules/git/attribute/checker.go +++ b/modules/git/attribute/checker.go @@ -68,15 +68,14 @@ func CheckAttributes(ctx context.Context, gitRepo *git.Repository, treeish strin } defer cancel() - stdOut := new(bytes.Buffer) - if err := cmd.WithEnv(append(os.Environ(), envs...)). + stdout, _, err := cmd.WithEnv(append(os.Environ(), envs...)). WithDir(gitRepo.Path). - WithStdout(stdOut). - RunWithStderr(ctx); err != nil { + RunStdBytes(ctx) + if err != nil { return nil, fmt.Errorf("failed to run check-attr: %w", err) } - fields := bytes.Split(stdOut.Bytes(), []byte{'\000'}) + fields := bytes.Split(stdout, []byte{'\000'}) if len(fields)%3 != 1 { return nil, errors.New("wrong number of fields in return from check-attr") } diff --git a/modules/git/catfile_batch_command.go b/modules/git/catfile_batch_command.go index f5f0110195..710561f045 100644 --- a/modules/git/catfile_batch_command.go +++ b/modules/git/catfile_batch_command.go @@ -39,23 +39,23 @@ func (b *catFileBatchCommand) getBatch() *catFileBatchCommunicator { } func (b *catFileBatchCommand) QueryContent(obj string) (*CatFileObject, BufferedReader, error) { - _, err := b.getBatch().writer.Write([]byte("contents " + obj + "\n")) + _, err := b.getBatch().reqWriter.Write([]byte("contents " + obj + "\n")) if err != nil { return nil, nil, err } - info, err := catFileBatchParseInfoLine(b.getBatch().reader) + info, err := catFileBatchParseInfoLine(b.getBatch().respReader) if err != nil { return nil, nil, err } - return info, b.getBatch().reader, nil + return info, b.getBatch().respReader, nil } func (b *catFileBatchCommand) QueryInfo(obj string) (*CatFileObject, error) { - _, err := b.getBatch().writer.Write([]byte("info " + obj + "\n")) + _, err := b.getBatch().reqWriter.Write([]byte("info " + obj + "\n")) if err != nil { return nil, err } - return catFileBatchParseInfoLine(b.getBatch().reader) + return catFileBatchParseInfoLine(b.getBatch().respReader) } func (b *catFileBatchCommand) Close() { diff --git a/modules/git/catfile_batch_legacy.go b/modules/git/catfile_batch_legacy.go index 07b875778a..795fc4ce3d 100644 --- a/modules/git/catfile_batch_legacy.go +++ b/modules/git/catfile_batch_legacy.go @@ -50,23 +50,23 @@ func (b *catFileBatchLegacy) getBatchCheck() *catFileBatchCommunicator { } func (b *catFileBatchLegacy) QueryContent(obj string) (*CatFileObject, BufferedReader, error) { - _, err := io.WriteString(b.getBatchContent().writer, obj+"\n") + _, err := io.WriteString(b.getBatchContent().reqWriter, obj+"\n") if err != nil { return nil, nil, err } - info, err := catFileBatchParseInfoLine(b.getBatchContent().reader) + info, err := catFileBatchParseInfoLine(b.getBatchContent().respReader) if err != nil { return nil, nil, err } - return info, b.getBatchContent().reader, nil + return info, b.getBatchContent().respReader, nil } func (b *catFileBatchLegacy) QueryInfo(obj string) (*CatFileObject, error) { - _, err := io.WriteString(b.getBatchCheck().writer, obj+"\n") + _, err := io.WriteString(b.getBatchCheck().reqWriter, obj+"\n") if err != nil { return nil, err } - return catFileBatchParseInfoLine(b.getBatchCheck().reader) + return catFileBatchParseInfoLine(b.getBatchCheck().respReader) } func (b *catFileBatchLegacy) Close() { diff --git a/modules/git/catfile_batch_reader.go b/modules/git/catfile_batch_reader.go index 9039edcd20..7d2e496ace 100644 --- a/modules/git/catfile_batch_reader.go +++ b/modules/git/catfile_batch_reader.go @@ -18,46 +18,40 @@ import ( ) type catFileBatchCommunicator struct { - cancel context.CancelFunc - reader *bufio.Reader - writer io.Writer - + cancel context.CancelFunc + reqWriter io.Writer + respReader *bufio.Reader debugGitCmd *gitcmd.Command } func (b *catFileBatchCommunicator) Close() { if b.cancel != nil { b.cancel() - b.reader = nil - b.writer = nil b.cancel = nil } } // newCatFileBatch opens git cat-file --batch in the provided repo and returns a stdin pipe, a stdout reader and cancel function -func newCatFileBatch(ctx context.Context, repoPath string, cmdCatFile *gitcmd.Command) *catFileBatchCommunicator { - // We often want to feed the commits in order into cat-file --batch, followed by their trees and subtrees as necessary. +func newCatFileBatch(ctx context.Context, repoPath string, cmdCatFile *gitcmd.Command) (ret *catFileBatchCommunicator) { ctx, ctxCancel := context.WithCancelCause(ctx) - var batchStdinWriter io.WriteCloser - var batchStdoutReader io.ReadCloser - cmdCatFile = cmdCatFile. - WithDir(repoPath). - WithStdinWriter(&batchStdinWriter). - WithStdoutReader(&batchStdoutReader) + // We often want to feed the commits in order into cat-file --batch, followed by their trees and subtrees as necessary. + stdinWriter, stdoutReader, pipeClose := cmdCatFile.MakeStdinStdoutPipe() + ret = &catFileBatchCommunicator{ + debugGitCmd: cmdCatFile, + cancel: func() { ctxCancel(nil) }, + reqWriter: stdinWriter, + respReader: bufio.NewReaderSize(stdoutReader, 32*1024), // use a buffered reader for rich operations + } - err := cmdCatFile.StartWithStderr(ctx) + err := cmdCatFile.WithDir(repoPath).StartWithStderr(ctx) if err != nil { log.Error("Unable to start git command %v: %v", cmdCatFile.LogString(), err) // ideally here it should return the error, but it would require refactoring all callers // so just return a dummy communicator that does nothing, almost the same behavior as before, not bad - return &catFileBatchCommunicator{ - writer: io.Discard, - reader: bufio.NewReader(bytes.NewReader(nil)), - cancel: func() { - ctxCancel(err) - }, - } + ctxCancel(err) + pipeClose() + return ret } go func() { @@ -66,19 +60,10 @@ func newCatFileBatch(ctx context.Context, repoPath string, cmdCatFile *gitcmd.Co log.Error("cat-file --batch command failed in repo %s, error: %v", repoPath, err) } ctxCancel(err) + pipeClose() }() - // use a buffered reader to read from the cat-file --batch (StringReader.ReadString) - batchReader := bufio.NewReaderSize(batchStdoutReader, 32*1024) - - return &catFileBatchCommunicator{ - writer: batchStdinWriter, - reader: batchReader, - cancel: func() { - ctxCancel(nil) - }, - debugGitCmd: cmdCatFile, - } + return ret } // catFileBatchParseInfoLine reads the header line from cat-file --batch diff --git a/modules/git/catfile_batch_test.go b/modules/git/catfile_batch_test.go index 355e8865e5..8f6b1f5eff 100644 --- a/modules/git/catfile_batch_test.go +++ b/modules/git/catfile_batch_test.go @@ -66,10 +66,10 @@ func testCatFileBatch(t *testing.T) { switch b := batch.(type) { case *catFileBatchLegacy: c = b.batchCheck - _, _ = c.writer.Write([]byte("in-complete-line-")) + _, _ = c.reqWriter.Write([]byte("in-complete-line-")) case *catFileBatchCommand: c = b.batch - _, _ = c.writer.Write([]byte("info")) + _, _ = c.reqWriter.Write([]byte("info")) default: t.FailNow() return @@ -78,8 +78,8 @@ func testCatFileBatch(t *testing.T) { wg := sync.WaitGroup{} wg.Go(func() { buf := make([]byte, 100) - _, _ = c.reader.Read(buf) - n, errRead := c.reader.Read(buf) + _, _ = c.respReader.Read(buf) + n, errRead := c.respReader.Read(buf) assert.Zero(t, n) assert.ErrorIs(t, errRead, io.EOF) // the pipe is closed due to command being killed }) diff --git a/modules/git/diff.go b/modules/git/diff.go index e4229c07ee..7f805478d5 100644 --- a/modules/git/diff.go +++ b/modules/git/diff.go @@ -78,7 +78,7 @@ func GetRepoRawDiffForFile(repo *Repository, startCommit, endCommit string, diff } return cmd.WithDir(repo.Path). - WithStdout(writer). + WithStdoutCopy(writer). RunWithStderr(repo.Ctx) } @@ -286,11 +286,10 @@ func GetAffectedFiles(repo *Repository, branchName, oldCommitID, newCommitID str affectedFiles := make([]string, 0, 32) // Run `git diff --name-only` to get the names of the changed files - var stdoutReader io.ReadCloser - err := gitcmd.NewCommand("diff", "--name-only").AddDynamicArguments(oldCommitID, newCommitID). - WithEnv(env). - WithDir(repo.Path). - WithStdoutReader(&stdoutReader). + cmd := gitcmd.NewCommand("diff", "--name-only").AddDynamicArguments(oldCommitID, newCommitID) + stdoutReader, stdoutReaderClose := cmd.MakeStdoutPipe() + defer stdoutReaderClose() + err := cmd.WithEnv(env).WithDir(repo.Path). WithPipelineFunc(func(ctx gitcmd.Context) error { // Now scan the output from the command scanner := bufio.NewScanner(stdoutReader) diff --git a/modules/git/git.go b/modules/git/git.go index 2412e7ea33..25b1955002 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -21,7 +21,7 @@ import ( "github.com/hashicorp/go-version" ) -const RequiredVersion = "2.0.0" // the minimum Git version required +const RequiredVersion = "2.6.0" // the minimum Git version required type Features struct { gitVersion *version.Version diff --git a/modules/git/gitcmd/command.go b/modules/git/gitcmd/command.go index 0f3abeb568..413b53299a 100644 --- a/modules/git/gitcmd/command.go +++ b/modules/git/gitcmd/command.go @@ -28,9 +28,6 @@ import ( // In most cases, it shouldn't be used. Use AddXxx function instead type TrustedCmdArgs []internal.CmdArg -// DefaultLocale is the default LC_ALL to run git commands in. -const DefaultLocale = "C" - // Command represents a command with its subcommands or arguments. type Command struct { callerInfo string @@ -47,9 +44,14 @@ type Command struct { cmdFinished process.FinishedFunc cmdStartTime time.Time - cmdStdinWriter *io.WriteCloser - cmdStdoutReader *io.ReadCloser - cmdStderrReader *io.ReadCloser + parentPipeFiles []*os.File + childrenPipeFiles []*os.File + + // only os.Pipe and in-memory buffers can work with Stdin safely, see https://github.com/golang/go/issues/77227 if the command would exit unexpectedly + cmdStdin io.Reader + cmdStdout io.Writer + cmdStderr io.Writer + cmdManagedStderr *bytes.Buffer } @@ -215,19 +217,6 @@ type runOpts struct { // The correct approach is to use `--git-dir" global argument Dir string - Stdout io.Writer - - // Stdin is used for passing input to the command - // The caller must make sure the Stdin writer is closed properly to finish the Run function. - // Otherwise, the Run function may hang for long time or forever, especially when the Git's context deadline is not the same as the caller's. - // Some common mistakes: - // * `defer stdinWriter.Close()` then call `cmd.Run()`: the Run() would never return if the command is killed by timeout - // * `go { case <- parentContext.Done(): stdinWriter.Close() }` with `cmd.Run(DefaultTimeout)`: the command would have been killed by timeout but the Run doesn't return until stdinWriter.Close() - // * `go { if stdoutReader.Read() err != nil: stdinWriter.Close() }` with `cmd.Run()`: the stdoutReader may never return error if the command is killed by timeout - // In the future, ideally the git module itself should have full control of the stdin, to avoid such problems and make it easier to refactor to a better architecture. - // Use new functions like WithStdinWriter to avoid such problems. - Stdin io.Reader - PipelineFunc func(Context) error } @@ -259,7 +248,7 @@ func commonBaseEnvs() []string { // CommonGitCmdEnvs returns the common environment variables for a "git" command. func CommonGitCmdEnvs() []string { return append(commonBaseEnvs(), []string{ - "LC_ALL=" + DefaultLocale, + "LC_ALL=C", // ensure git output is in English, error messages are parsed in English "GIT_TERMINAL_PROMPT=0", // avoid prompting for credentials interactively, supported since git v2.3 }...) } @@ -286,30 +275,76 @@ func (c *Command) WithTimeout(timeout time.Duration) *Command { return c } -func (c *Command) WithStdoutReader(r *io.ReadCloser) *Command { - c.cmdStdoutReader = r +func (c *Command) makeStdoutStderr(w *io.Writer) (PipeReader, func()) { + pr, pw, err := os.Pipe() + if err != nil { + c.preErrors = append(c.preErrors, err) + return &pipeNull{err}, func() {} + } + c.childrenPipeFiles = append(c.childrenPipeFiles, pw) + c.parentPipeFiles = append(c.parentPipeFiles, pr) + *w /* stdout, stderr */ = pw + return &pipeReader{f: pr}, func() { pr.Close() } +} + +// MakeStdinPipe creates a writer for the command's stdin. +// The returned closer function must be called by the caller to close the pipe. +func (c *Command) MakeStdinPipe() (writer PipeWriter, closer func()) { + pr, pw, err := os.Pipe() + if err != nil { + c.preErrors = append(c.preErrors, err) + return &pipeNull{err}, func() {} + } + c.childrenPipeFiles = append(c.childrenPipeFiles, pr) + c.parentPipeFiles = append(c.parentPipeFiles, pw) + c.cmdStdin = pr + return &pipeWriter{pw}, func() { pw.Close() } +} + +// MakeStdoutPipe creates a reader for the command's stdout. +// The returned closer function must be called by the caller to close the pipe. +// After the pipe reader is closed, the unread data will be discarded. +func (c *Command) MakeStdoutPipe() (reader PipeReader, closer func()) { + return c.makeStdoutStderr(&c.cmdStdout) +} + +// MakeStderrPipe is like MakeStdoutPipe, but for stderr. +func (c *Command) MakeStderrPipe() (reader PipeReader, closer func()) { + return c.makeStdoutStderr(&c.cmdStderr) +} + +func (c *Command) MakeStdinStdoutPipe() (stdin PipeWriter, stdout PipeReader, closer func()) { + stdin, stdinClose := c.MakeStdinPipe() + stdout, stdoutClose := c.MakeStdoutPipe() + return stdin, stdout, func() { + stdinClose() + stdoutClose() + } +} + +func (c *Command) WithStdinBytes(stdin []byte) *Command { + c.cmdStdin = bytes.NewReader(stdin) return c } -// WithStdout is deprecated, use WithStdoutReader instead -func (c *Command) WithStdout(stdout io.Writer) *Command { - c.opts.Stdout = stdout +func (c *Command) WithStdoutBuffer(w PipeBufferWriter) *Command { + c.cmdStdout = w return c } -func (c *Command) WithStderrReader(r *io.ReadCloser) *Command { - c.cmdStderrReader = r +// WithStdinCopy and WithStdoutCopy are general functions that accept any io.Reader / io.Writer. +// In this case, Golang exec.Cmd will start new internal goroutines to do io.Copy between pipes and provided Reader/Writer. +// If the reader or writer is blocked and never returns, then the io.Copy won't finish, then exec.Cmd.Wait won't return, which may cause deadlocks. +// A typical deadlock example is: +// * `r,w:=io.Pipe(); cmd.Stdin=r; defer w.Close(); cmd.Run()`: the Run() will never return because stdin reader is blocked forever and w.Close() will never be called. +// If the reader/writer won't block forever (for example: read from a file or buffer), then these functions are safe to use. +func (c *Command) WithStdinCopy(w io.Reader) *Command { + c.cmdStdin = w return c } -func (c *Command) WithStdinWriter(w *io.WriteCloser) *Command { - c.cmdStdinWriter = w - return c -} - -// WithStdin is deprecated, use WithStdinWriter instead -func (c *Command) WithStdin(stdin io.Reader) *Command { - c.opts.Stdin = stdin +func (c *Command) WithStdoutCopy(w io.Writer) *Command { + c.cmdStdout = w return c } @@ -348,9 +383,10 @@ func (c *Command) Start(ctx context.Context) (retErr error) { } defer func() { + c.closePipeFiles(c.childrenPipeFiles) if retErr != nil { - // release the pipes to avoid resource leak - c.closeStdioPipes() + // release the pipes to avoid resource leak since the command failed to start + c.closePipeFiles(c.parentPipeFiles) // if error occurs, we must also finish the task, otherwise, cmdFinished will be called in "Wait" function if c.cmdFinished != nil { c.cmdFinished() @@ -359,7 +395,7 @@ func (c *Command) Start(ctx context.Context) (retErr error) { }() if len(c.preErrors) != 0 { - // In most cases, such error shouldn't happen. If it happens, it must be a programming error, so we log it as error level with more details + // In most cases, such error shouldn't happen. If it happens, log it as error level with more details err := errors.Join(c.preErrors...) log.Error("git command: %s, error: %s", c.LogString(), err) return err @@ -386,7 +422,7 @@ func (c *Command) Start(ctx context.Context) (retErr error) { c.cmdStartTime = time.Now() - c.cmd = exec.CommandContext(ctx, c.prog, append(c.configArgs, c.args...)...) + c.cmd = exec.CommandContext(c.cmdCtx, c.prog, append(c.configArgs, c.args...)...) if c.opts.Env == nil { c.cmd.Env = os.Environ() } else { @@ -396,52 +432,38 @@ func (c *Command) Start(ctx context.Context) (retErr error) { process.SetSysProcAttribute(c.cmd) c.cmd.Env = append(c.cmd.Env, CommonGitCmdEnvs()...) c.cmd.Dir = c.opts.Dir - c.cmd.Stdout = c.opts.Stdout - c.cmd.Stdin = c.opts.Stdin - - if _, err := safeAssignPipe(c.cmdStdinWriter, c.cmd.StdinPipe); err != nil { - return err - } - if _, err := safeAssignPipe(c.cmdStdoutReader, c.cmd.StdoutPipe); err != nil { - return err - } - if _, err := safeAssignPipe(c.cmdStderrReader, c.cmd.StderrPipe); err != nil { - return err - } - - if c.cmdManagedStderr != nil { - if c.cmd.Stderr != nil { - panic("CombineStderr needs managed (but not caller-provided) stderr pipe") - } - c.cmd.Stderr = c.cmdManagedStderr - } + c.cmd.Stdout = c.cmdStdout + c.cmd.Stdin = c.cmdStdin + c.cmd.Stderr = c.cmdStderr return c.cmd.Start() } -func (c *Command) closeStdioPipes() { - safeClosePtrCloser(c.cmdStdoutReader) - safeClosePtrCloser(c.cmdStderrReader) - safeClosePtrCloser(c.cmdStdinWriter) +func (c *Command) closePipeFiles(files []*os.File) { + for _, f := range files { + _ = f.Close() + } } func (c *Command) Wait() error { defer func() { - c.closeStdioPipes() + // The reader in another goroutine might be still reading the stdout, so we shouldn't close the pipes here + // MakeStdoutPipe returns a closer function to force callers to close the pipe correctly + // Here we only need to mark the command as finished c.cmdFinished() }() if c.opts.PipelineFunc != nil { - errCallback := c.opts.PipelineFunc(&cmdContext{Context: c.cmdCtx, cmd: c}) - // after the pipeline function returns, we can safely cancel the command context and close the stdio pipes - c.cmdCancel(errCallback) - c.closeStdioPipes() + errPipeline := c.opts.PipelineFunc(&cmdContext{Context: c.cmdCtx, cmd: c}) + // after the pipeline function returns, we can safely cancel the command context and close the pipes, the data in pipes should have been consumed + c.cmdCancel(errPipeline) + c.closePipeFiles(c.parentPipeFiles) errWait := c.cmd.Wait() errCause := context.Cause(c.cmdCtx) // the pipeline function should be able to know whether it succeeds or fails - if errCallback == nil && (errCause == nil || errors.Is(errCause, context.Canceled)) { + if errPipeline == nil && (errCause == nil || errors.Is(errCause, context.Canceled)) { return nil } - return errors.Join(errCallback, errCause, errWait) + return errors.Join(wrapPipelineError(errPipeline), errCause, errWait) } // there might be other goroutines using the context or pipes, so we just wait for the command to finish @@ -460,7 +482,11 @@ func (c *Command) Wait() error { } func (c *Command) StartWithStderr(ctx context.Context) RunStdError { + if c.cmdStderr != nil { + panic("caller-provided stderr receiver doesn't work with managed stderr buffer") + } c.cmdManagedStderr = &bytes.Buffer{} + c.cmdStderr = c.cmdManagedStderr err := c.Start(ctx) if err != nil { return &runStdError{err: err} @@ -470,7 +496,7 @@ func (c *Command) StartWithStderr(ctx context.Context) RunStdError { func (c *Command) WaitWithStderr() RunStdError { if c.cmdManagedStderr == nil { - panic("CombineStderr needs managed (but not caller-provided) stderr pipe") + panic("managed stderr buffer is not initialized") } errWait := c.Wait() if errWait == nil { @@ -506,14 +532,12 @@ func (c *Command) RunStdBytes(ctx context.Context) (stdout, stderr []byte, runEr } func (c *Command) runStdBytes(ctx context.Context) ([]byte, []byte, RunStdError) { - if c.opts.Stdout != nil || c.cmdStdoutReader != nil || c.cmdStderrReader != nil { - // we must panic here, otherwise there would be bugs if developers set Stdin/Stderr by mistake, and it would be very difficult to debug + if c.cmdStdout != nil || c.cmdStderr != nil { + // it must panic here, otherwise there would be bugs if developers set other Stdin/Stderr by mistake, and it would be very difficult to debug panic("stdout and stderr field must be nil when using RunStdBytes") } stdoutBuf := &bytes.Buffer{} - err := c.WithParentCallerInfo(). - WithStdout(stdoutBuf). - RunWithStderr(ctx) + err := c.WithParentCallerInfo().WithStdoutBuffer(stdoutBuf).RunWithStderr(ctx) return stdoutBuf.Bytes(), c.cmdManagedStderr.Bytes(), err } diff --git a/modules/git/gitcmd/command_test.go b/modules/git/gitcmd/command_test.go index 63e82635be..86771f499f 100644 --- a/modules/git/gitcmd/command_test.go +++ b/modules/git/gitcmd/command_test.go @@ -121,7 +121,10 @@ func TestRunWithContextTimeout(t *testing.T) { require.NoError(t, err) }) t.Run("WithTimeout", func(t *testing.T) { - err := NewCommand("hash-object", "--stdin").WithTimeout(1 * time.Millisecond).Run(t.Context()) + cmd := NewCommand("hash-object", "--stdin") + _, _, pipeClose := cmd.MakeStdinStdoutPipe() + defer pipeClose() + err := cmd.WithTimeout(1 * time.Millisecond).Run(t.Context()) require.ErrorIs(t, err, context.DeadlineExceeded) }) } diff --git a/modules/git/gitcmd/error.go b/modules/git/gitcmd/error.go index 8f5397b96d..30d2164b2b 100644 --- a/modules/git/gitcmd/error.go +++ b/modules/git/gitcmd/error.go @@ -76,3 +76,26 @@ func IsErrorCanceledOrKilled(err error) bool { // TODO: in the future, we need to use unified error type from gitcmd.Run to check whether it is manually canceled return errors.Is(err, context.Canceled) || IsErrorSignalKilled(err) } + +type pipelineError struct { + error +} + +func (e pipelineError) Unwrap() error { + return e.error +} + +func wrapPipelineError(err error) error { + if err == nil { + return nil + } + return pipelineError{err} +} + +func ErrorAsPipeline(err error) error { + var pipelineErr pipelineError + if errors.As(err, &pipelineErr) { + return pipelineErr.error + } + return nil +} diff --git a/modules/git/gitcmd/pipe.go b/modules/git/gitcmd/pipe.go new file mode 100644 index 0000000000..a1e91fdf8e --- /dev/null +++ b/modules/git/gitcmd/pipe.go @@ -0,0 +1,79 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package gitcmd + +import ( + "io" + "os" +) + +type PipeBufferReader interface { + Read(p []byte) (n int, err error) + Bytes() []byte +} + +type PipeBufferWriter interface { + Write(p []byte) (n int, err error) + Bytes() []byte +} + +type PipeReader interface { + io.ReadCloser + internalOnly() +} + +type pipeReader struct { + f *os.File +} + +func (r *pipeReader) internalOnly() {} + +func (r *pipeReader) Read(p []byte) (n int, err error) { + return r.f.Read(p) +} + +func (r *pipeReader) Close() error { + return r.f.Close() +} + +type PipeWriter interface { + io.WriteCloser + internalOnly() +} + +type pipeWriter struct { + f *os.File +} + +func (w *pipeWriter) internalOnly() {} + +func (w *pipeWriter) Close() error { + return w.f.Close() +} + +func (w *pipeWriter) Write(p []byte) (n int, err error) { + return w.f.Write(p) +} + +func (w *pipeWriter) DrainBeforeClose() error { + return nil +} + +type pipeNull struct { + err error +} + +func (p *pipeNull) internalOnly() {} + +func (p *pipeNull) Read([]byte) (n int, err error) { + return 0, p.err +} + +func (p *pipeNull) Write([]byte) (n int, err error) { + return 0, p.err +} + +func (p *pipeNull) Close() error { + return nil +} diff --git a/modules/git/gitcmd/utils.go b/modules/git/gitcmd/utils.go deleted file mode 100644 index 0df3fc2c4b..0000000000 --- a/modules/git/gitcmd/utils.go +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright 2025 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package gitcmd - -import ( - "io" -) - -func safeClosePtrCloser[T *io.ReadCloser | *io.WriteCloser](c T) { - switch v := any(c).(type) { - case *io.ReadCloser: - if v != nil && *v != nil { - _ = (*v).Close() - } - case *io.WriteCloser: - if v != nil && *v != nil { - _ = (*v).Close() - } - default: - panic("unsupported type") - } -} - -func safeAssignPipe[T any](p *T, fn func() (T, error)) (bool, error) { - if p == nil { - return false, nil - } - v, err := fn() - if err != nil { - return false, err - } - *p = v - return true, nil -} diff --git a/modules/git/grep.go b/modules/git/grep.go index bf348387e7..aac7b6b51d 100644 --- a/modules/git/grep.go +++ b/modules/git/grep.go @@ -8,7 +8,6 @@ import ( "context" "errors" "fmt" - "io" "slices" "strconv" "strings" @@ -74,9 +73,9 @@ func GrepSearch(ctx context.Context, repo *Repository, search string, opts GrepO cmd.AddDashesAndList(opts.PathspecList...) opts.MaxResultLimit = util.IfZero(opts.MaxResultLimit, 50) - var stdoutReader io.ReadCloser + stdoutReader, stdoutReaderClose := cmd.MakeStdoutPipe() + defer stdoutReaderClose() err := cmd.WithDir(repo.Path). - WithStdoutReader(&stdoutReader). WithPipelineFunc(func(ctx gitcmd.Context) error { isInBlock := false rd := bufio.NewReaderSize(stdoutReader, util.IfZero(opts.MaxLineLength, 16*1024)) diff --git a/modules/git/log_name_status.go b/modules/git/log_name_status.go index 129f3c6c05..8acfc96f26 100644 --- a/modules/git/log_name_status.go +++ b/modules/git/log_name_status.go @@ -15,25 +15,12 @@ import ( "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git/gitcmd" - - "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)) - // 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) - - cancel := func() { - ctxCancel() - _ = stdoutReader.Close() - _ = stdoutWriter.Close() - } cmd := gitcmd.NewCommand() cmd.AddArguments("log", "--name-status", "-c", "--format=commit%x00%H %P%x00", "--parents", "--no-renames", "-t", "-z").AddDynamicArguments(head) @@ -63,16 +50,21 @@ func LogNameStatusRepo(ctx context.Context, repository, head, treepath string, p } cmd.AddDashesAndList(files...) + stdoutReader, stdoutReaderClose := cmd.MakeStdoutPipe() + ctx, ctxCancel := context.WithCancel(ctx) go func() { - err := cmd.WithDir(repository). - WithStdout(stdoutWriter). - RunWithStderr(ctx) - _ = stdoutWriter.CloseWithError(err) + err := cmd.WithDir(repository).RunWithStderr(ctx) + if err != nil && !errors.Is(err, context.Canceled) { + log.Error("Unable to run git command %v: %v", cmd.LogString(), err) + } }() bufReader := bufio.NewReaderSize(stdoutReader, 32*1024) - return bufReader, cancel + return bufReader, func() { + ctxCancel() + stdoutReaderClose() + } } // LogNameStatusRepoParser parses a git log raw output from LogRawRepo diff --git a/modules/git/pipeline/catfile.go b/modules/git/pipeline/catfile.go index ccd6884a6a..3d005e28f1 100644 --- a/modules/git/pipeline/catfile.go +++ b/modules/git/pipeline/catfile.go @@ -6,67 +6,33 @@ package pipeline import ( "bufio" "context" - "fmt" "io" "strconv" "strings" - "sync" "code.gitea.io/gitea/modules/git/gitcmd" ) // CatFileBatchCheck runs cat-file with --batch-check -func CatFileBatchCheck(ctx context.Context, shasToCheckReader *io.PipeReader, catFileCheckWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath string) { - defer wg.Done() - defer shasToCheckReader.Close() - defer catFileCheckWriter.Close() - - cmd := gitcmd.NewCommand("cat-file", "--batch-check") - if err := cmd.WithDir(tmpBasePath). - WithStdin(shasToCheckReader). - WithStdout(catFileCheckWriter). - RunWithStderr(ctx); err != nil { - _ = catFileCheckWriter.CloseWithError(fmt.Errorf("git cat-file --batch-check [%s]: %w", tmpBasePath, err)) - } +func CatFileBatchCheck(ctx context.Context, cmd *gitcmd.Command, tmpBasePath string) error { + cmd.AddArguments("cat-file", "--batch-check") + return cmd.WithDir(tmpBasePath).RunWithStderr(ctx) } // 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) { - defer wg.Done() - defer catFileCheckWriter.Close() - - cmd := gitcmd.NewCommand("cat-file", "--batch-check", "--batch-all-objects") - if err := cmd.WithDir(tmpBasePath). - WithStdout(catFileCheckWriter). - RunWithStderr(ctx); err != nil { - _ = catFileCheckWriter.CloseWithError(fmt.Errorf("git cat-file --batch-check --batch-all-object [%s]: %w", tmpBasePath, err)) - errChan <- err - } +func CatFileBatchCheckAllObjects(ctx context.Context, cmd *gitcmd.Command, tmpBasePath string) error { + return cmd.AddArguments("cat-file", "--batch-check", "--batch-all-objects").WithDir(tmpBasePath).RunWithStderr(ctx) } // CatFileBatch runs cat-file --batch -func CatFileBatch(ctx context.Context, shasToBatchReader *io.PipeReader, catFileBatchWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath string) { - defer wg.Done() - defer shasToBatchReader.Close() - defer catFileBatchWriter.Close() - - if err := gitcmd.NewCommand("cat-file", "--batch"). - WithDir(tmpBasePath). - WithStdin(shasToBatchReader). - WithStdout(catFileBatchWriter). - RunWithStderr(ctx); err != nil { - _ = shasToBatchReader.CloseWithError(fmt.Errorf("git rev-list [%s]: %w", tmpBasePath, err)) - } +func CatFileBatch(ctx context.Context, cmd *gitcmd.Command, tmpBasePath string) error { + return cmd.AddArguments("cat-file", "--batch").WithDir(tmpBasePath).RunWithStderr(ctx) } // 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) { - defer wg.Done() - defer catFileCheckReader.Close() - scanner := bufio.NewScanner(catFileCheckReader) - defer func() { - _ = shasToBatchWriter.CloseWithError(scanner.Err()) - }() +func BlobsLessThan1024FromCatFileBatchCheck(in io.ReadCloser, out io.WriteCloser) error { + defer out.Close() + scanner := bufio.NewScanner(in) for scanner.Scan() { line := scanner.Text() if len(line) == 0 { @@ -82,12 +48,12 @@ func BlobsLessThan1024FromCatFileBatchCheck(catFileCheckReader *io.PipeReader, s } toWrite := []byte(fields[0] + "\n") for len(toWrite) > 0 { - n, err := shasToBatchWriter.Write(toWrite) + n, err := out.Write(toWrite) if err != nil { - _ = catFileCheckReader.CloseWithError(err) - break + return err } toWrite = toWrite[n:] } } + return scanner.Err() } diff --git a/modules/git/pipeline/lfs_common.go b/modules/git/pipeline/lfs_common.go index 188e7d4d65..914aefbeaf 100644 --- a/modules/git/pipeline/lfs_common.go +++ b/modules/git/pipeline/lfs_common.go @@ -4,7 +4,6 @@ package pipeline import ( - "fmt" "time" "code.gitea.io/gitea/modules/git" @@ -26,7 +25,3 @@ type lfsResultSlice []*LFSResult func (a lfsResultSlice) Len() int { return len(a) } func (a lfsResultSlice) Swap(i, j int) { a[i], a[j] = a[j], a[i] } func (a lfsResultSlice) Less(i, j int) bool { return a[j].When.After(a[i].When) } - -func lfsError(msg string, err error) error { - return fmt.Errorf("LFS error occurred, %s: err: %w", msg, err) -} diff --git a/modules/git/pipeline/lfs_gogit.go b/modules/git/pipeline/lfs_gogit.go index adcf8ed09c..c12397569c 100644 --- a/modules/git/pipeline/lfs_gogit.go +++ b/modules/git/pipeline/lfs_gogit.go @@ -6,11 +6,10 @@ package pipeline import ( - "bufio" + "fmt" "io" "sort" "strings" - "sync" "code.gitea.io/gitea/modules/git" @@ -24,7 +23,6 @@ func FindLFSFile(repo *git.Repository, objectID git.ObjectID) ([]*LFSResult, err resultsMap := map[string]*LFSResult{} results := make([]*LFSResult, 0) - basePath := repo.Path gogitRepo := repo.GoGitRepo() commitsIter, err := gogitRepo.Log(&gogit.LogOptions{ @@ -32,7 +30,7 @@ func FindLFSFile(repo *git.Repository, objectID git.ObjectID) ([]*LFSResult, err All: true, }) if err != nil { - return nil, lfsError("failed to get GoGit CommitsIter", err) + return nil, fmt.Errorf("LFS error occurred, failed to get GoGit CommitsIter: err: %w", err) } err = commitsIter.ForEach(func(gitCommit *object.Commit) error { @@ -66,7 +64,7 @@ func FindLFSFile(repo *git.Repository, objectID git.ObjectID) ([]*LFSResult, err return nil }) if err != nil && err != io.EOF { - return nil, lfsError("failure in CommitIter.ForEach", err) + return nil, fmt.Errorf("LFS error occurred, failure in CommitIter.ForEach: %w", err) } for _, result := range resultsMap { @@ -82,65 +80,6 @@ func FindLFSFile(repo *git.Repository, objectID git.ObjectID) ([]*LFSResult, err } 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() - errChan := make(chan error, 1) - wg := sync.WaitGroup{} - wg.Add(3) - - go func() { - defer wg.Done() - scanner := bufio.NewScanner(nameRevStdinReader) - i := 0 - for scanner.Scan() { - line := scanner.Text() - if len(line) == 0 { - continue - } - result := results[i] - result.FullCommitName = line - result.BranchName = strings.Split(line, "~")[0] - i++ - } - }() - go NameRevStdin(repo.Ctx, shasToNameReader, nameRevStdinWriter, &wg, basePath) - go func() { - defer wg.Done() - defer shasToNameWriter.Close() - for _, result := range results { - i := 0 - if i < len(result.SHA) { - n, err := shasToNameWriter.Write([]byte(result.SHA)[i:]) - if err != nil { - errChan <- err - break - } - i += n - } - n := 0 - for n < 1 { - n, err = shasToNameWriter.Write([]byte{'\n'}) - if err != nil { - errChan <- err - break - } - - } - - } - }() - - wg.Wait() - - select { - case err, has := <-errChan: - if has { - return nil, lfsError("unable to obtain name for LFS files", err) - } - default: - } - - return results, nil + err = fillResultNameRev(repo.Ctx, repo.Path, results) + return results, err } diff --git a/modules/git/pipeline/lfs_nogogit.go b/modules/git/pipeline/lfs_nogogit.go index 41282d29a9..91bda0d0e5 100644 --- a/modules/git/pipeline/lfs_nogogit.go +++ b/modules/git/pipeline/lfs_nogogit.go @@ -12,34 +12,27 @@ import ( "io" "sort" "strings" - "sync" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git/gitcmd" ) // FindLFSFile finds commits that contain a provided pointer file hash -func FindLFSFile(repo *git.Repository, objectID git.ObjectID) ([]*LFSResult, error) { +func FindLFSFile(repo *git.Repository, objectID git.ObjectID) (results []*LFSResult, _ error) { + cmd := gitcmd.NewCommand("rev-list", "--all") + revListReader, revListReaderClose := cmd.MakeStdoutPipe() + defer revListReaderClose() + err := cmd.WithDir(repo.Path). + WithPipelineFunc(func(context gitcmd.Context) (err error) { + results, err = findLFSFileFunc(repo, objectID, revListReader) + return err + }).RunWithStderr(repo.Ctx) + return results, err +} + +func findLFSFileFunc(repo *git.Repository, objectID git.ObjectID, revListReader io.Reader) ([]*LFSResult, error) { resultsMap := map[string]*LFSResult{} results := make([]*LFSResult, 0) - - basePath := repo.Path - - // Use rev-list to provide us with all commits in order - revListReader, revListWriter := io.Pipe() - defer func() { - _ = revListWriter.Close() - _ = revListReader.Close() - }() - - go func() { - err := gitcmd.NewCommand("rev-list", "--all"). - WithDir(repo.Path). - WithStdout(revListWriter). - RunWithStderr(repo.Ctx) - _ = revListWriter.CloseWithError(err) - }() - // Next 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 batch, cancel, err := repo.CatFileBatch(repo.Ctx) @@ -158,56 +151,6 @@ func FindLFSFile(repo *git.Repository, objectID git.ObjectID) ([]*LFSResult, err } 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() - errChan := make(chan error, 1) - wg := sync.WaitGroup{} - wg.Add(3) - - go func() { - defer wg.Done() - scanner := bufio.NewScanner(nameRevStdinReader) - i := 0 - for scanner.Scan() { - line := scanner.Text() - if len(line) == 0 { - continue - } - result := results[i] - result.FullCommitName = line - result.BranchName = strings.Split(line, "~")[0] - i++ - } - }() - go NameRevStdin(repo.Ctx, shasToNameReader, nameRevStdinWriter, &wg, basePath) - go func() { - defer wg.Done() - defer shasToNameWriter.Close() - for _, result := range results { - _, err := shasToNameWriter.Write([]byte(result.SHA)) - if err != nil { - errChan <- err - break - } - _, err = shasToNameWriter.Write([]byte{'\n'}) - if err != nil { - errChan <- err - break - } - } - }() - - wg.Wait() - - select { - case err, has := <-errChan: - if has { - return nil, lfsError("unable to obtain name for LFS files", err) - } - default: - } - - return results, nil + err = fillResultNameRev(repo.Ctx, repo.Path, results) + return results, err } diff --git a/modules/git/pipeline/namerev.go b/modules/git/pipeline/namerev.go index 39f7f31900..24de442940 100644 --- a/modules/git/pipeline/namerev.go +++ b/modules/git/pipeline/namerev.go @@ -4,25 +4,54 @@ package pipeline import ( + "bufio" "context" - "fmt" - "io" - "sync" + "errors" + "strings" "code.gitea.io/gitea/modules/git/gitcmd" + + "golang.org/x/sync/errgroup" ) -// NameRevStdin runs name-rev --stdin -func NameRevStdin(ctx context.Context, shasToNameReader *io.PipeReader, nameRevStdinWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath string) { - defer wg.Done() - defer shasToNameReader.Close() - defer nameRevStdinWriter.Close() +func fillResultNameRev(ctx context.Context, basePath string, results []*LFSResult) error { + // Should really use a go-git function here but name-rev is not completed and recapitulating it is not simple + wg := errgroup.Group{} + cmd := gitcmd.NewCommand("name-rev", "--stdin", "--name-only", "--always").WithDir(basePath) + stdin, stdinClose := cmd.MakeStdinPipe() + stdout, stdoutClose := cmd.MakeStdoutPipe() + defer stdinClose() + defer stdoutClose() - if err := gitcmd.NewCommand("name-rev", "--stdin", "--name-only", "--always"). - WithDir(tmpBasePath). - WithStdin(shasToNameReader). - WithStdout(nameRevStdinWriter). - RunWithStderr(ctx); err != nil { - _ = shasToNameReader.CloseWithError(fmt.Errorf("git name-rev [%s]: %w", tmpBasePath, err)) - } + wg.Go(func() error { + scanner := bufio.NewScanner(stdout) + i := 0 + for scanner.Scan() { + line := scanner.Text() + if len(line) == 0 { + continue + } + result := results[i] + result.FullCommitName = line + result.BranchName = strings.Split(line, "~")[0] + i++ + } + return scanner.Err() + }) + wg.Go(func() error { + defer stdinClose() + for _, result := range results { + _, err := stdin.Write([]byte(result.SHA)) + if err != nil { + return err + } + _, err = stdin.Write([]byte{'\n'}) + if err != nil { + return err + } + } + return nil + }) + err := cmd.RunWithStderr(ctx) + return errors.Join(err, wg.Wait()) } diff --git a/modules/git/pipeline/revlist.go b/modules/git/pipeline/revlist.go index ead562c9e9..28d4751bd8 100644 --- a/modules/git/pipeline/revlist.go +++ b/modules/git/pipeline/revlist.go @@ -6,52 +6,25 @@ package pipeline import ( "bufio" "context" - "fmt" "io" "strings" - "sync" "code.gitea.io/gitea/modules/git/gitcmd" ) -// 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) { - defer wg.Done() - defer revListWriter.Close() - - cmd := gitcmd.NewCommand("rev-list", "--objects", "--all") - if err := cmd.WithDir(basePath). - WithStdout(revListWriter). - RunWithStderr(ctx); err != nil { - _ = revListWriter.CloseWithError(fmt.Errorf("git rev-list --objects --all [%s]: %w", basePath, err)) - errChan <- err - } -} - // 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) { - defer wg.Done() - defer revListWriter.Close() - - cmd := gitcmd.NewCommand("rev-list", "--objects").AddDynamicArguments(headSHA) +func RevListObjects(ctx context.Context, cmd *gitcmd.Command, tmpBasePath, headSHA, baseSHA string) error { + cmd.AddArguments("rev-list", "--objects").AddDynamicArguments(headSHA) if baseSHA != "" { cmd = cmd.AddArguments("--not").AddDynamicArguments(baseSHA) } - if err := cmd.WithDir(tmpBasePath). - WithStdout(revListWriter). - RunWithStderr(ctx); err != nil { - errChan <- fmt.Errorf("git rev-list [%s]: %w", tmpBasePath, err) - } + return cmd.WithDir(tmpBasePath).RunWithStderr(ctx) } // BlobsFromRevListObjects reads a RevListAllObjects and only selects blobs -func BlobsFromRevListObjects(revListReader *io.PipeReader, shasToCheckWriter *io.PipeWriter, wg *sync.WaitGroup) { - defer wg.Done() - defer revListReader.Close() - scanner := bufio.NewScanner(revListReader) - defer func() { - _ = shasToCheckWriter.CloseWithError(scanner.Err()) - }() +func BlobsFromRevListObjects(in io.ReadCloser, out io.WriteCloser) error { + defer out.Close() + scanner := bufio.NewScanner(in) for scanner.Scan() { line := scanner.Text() if len(line) == 0 { @@ -63,12 +36,12 @@ func BlobsFromRevListObjects(revListReader *io.PipeReader, shasToCheckWriter *io } toWrite := []byte(fields[0] + "\n") for len(toWrite) > 0 { - n, err := shasToCheckWriter.Write(toWrite) + n, err := out.Write(toWrite) if err != nil { - _ = revListReader.CloseWithError(err) - break + return err } toWrite = toWrite[n:] } } + return scanner.Err() } diff --git a/modules/git/repo_archive.go b/modules/git/repo_archive.go deleted file mode 100644 index 2bbe3f0bb3..0000000000 --- a/modules/git/repo_archive.go +++ /dev/null @@ -1,68 +0,0 @@ -// Copyright 2015 The Gogs Authors. All rights reserved. -// Copyright 2020 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package git - -import ( - "context" - "fmt" - "io" - "path/filepath" - "strings" - - "code.gitea.io/gitea/modules/git/gitcmd" -) - -// ArchiveType archive types -type ArchiveType int - -const ( - ArchiveUnknown ArchiveType = iota - ArchiveZip // 1 - ArchiveTarGz // 2 - ArchiveBundle // 3 -) - -// String converts an ArchiveType to string: the extension of the archive file without prefix dot -func (a ArchiveType) String() string { - switch a { - case ArchiveZip: - return "zip" - case ArchiveTarGz: - return "tar.gz" - case ArchiveBundle: - return "bundle" - } - return "unknown" -} - -func SplitArchiveNameType(s string) (string, ArchiveType) { - switch { - case strings.HasSuffix(s, ".zip"): - return strings.TrimSuffix(s, ".zip"), ArchiveZip - case strings.HasSuffix(s, ".tar.gz"): - return strings.TrimSuffix(s, ".tar.gz"), ArchiveTarGz - case strings.HasSuffix(s, ".bundle"): - return strings.TrimSuffix(s, ".bundle"), ArchiveBundle - } - return s, ArchiveUnknown -} - -// CreateArchive create archive content to the target path -func (repo *Repository) CreateArchive(ctx context.Context, format ArchiveType, target io.Writer, usePrefix bool, commitID string) error { - if format.String() == "unknown" { - return fmt.Errorf("unknown format: %v", format) - } - - cmd := gitcmd.NewCommand("archive") - if usePrefix { - cmd.AddOptionFormat("--prefix=%s", filepath.Base(strings.TrimSuffix(repo.Path, ".git"))+"/") - } - cmd.AddOptionFormat("--format=%s", format.String()) - cmd.AddDynamicArguments(commitID) - - return cmd.WithDir(repo.Path). - WithStdout(target). - RunWithStderr(ctx) -} diff --git a/modules/git/repo_branch_nogogit.go b/modules/git/repo_branch_nogogit.go index f8a27455d5..dee6d9d3e8 100644 --- a/modules/git/repo_branch_nogogit.go +++ b/modules/git/repo_branch_nogogit.go @@ -94,84 +94,81 @@ func callShowRef(ctx context.Context, repoPath, trimPrefix string, extraArgs git } func WalkShowRef(ctx context.Context, repoPath string, extraArgs gitcmd.TrustedCmdArgs, skip, limit int, walkfn func(sha1, refname string) error) (countAll int, err error) { - stdoutReader, stdoutWriter := io.Pipe() - defer func() { - _ = stdoutReader.Close() - _ = stdoutWriter.Close() - }() - - go func() { - args := gitcmd.TrustedCmdArgs{"for-each-ref", "--format=%(objectname) %(refname)"} - args = append(args, extraArgs...) - err := gitcmd.NewCommand(args...). - WithDir(repoPath). - WithStdout(stdoutWriter). - RunWithStderr(ctx) - _ = stdoutWriter.CloseWithError(err) - }() - i := 0 - bufReader := bufio.NewReader(stdoutReader) - for i < skip { - _, isPrefix, err := bufReader.ReadLine() - if err == io.EOF { - return i, nil - } - if err != nil { - return 0, err - } - if !isPrefix { - i++ - } + args := gitcmd.TrustedCmdArgs{"for-each-ref", "--format=%(objectname) %(refname)"} + args = append(args, extraArgs...) + cmd := gitcmd.NewCommand(args...) + stdoutReader, stdoutReaderClose := cmd.MakeStdoutPipe() + defer stdoutReaderClose() + cmd.WithDir(repoPath). + WithPipelineFunc(func(c gitcmd.Context) error { + bufReader := bufio.NewReader(stdoutReader) + for i < skip { + _, isPrefix, err := bufReader.ReadLine() + if err == io.EOF { + return nil + } + if err != nil { + return err + } + if !isPrefix { + i++ + } + } + for limit == 0 || i < skip+limit { + // The output of show-ref is simply a list: + // SP LF + sha, err := bufReader.ReadString(' ') + if err == io.EOF { + return nil + } + if err != nil { + return err + } + + branchName, err := bufReader.ReadString('\n') + if err == io.EOF { + // This shouldn't happen... but we'll tolerate it for the sake of peace + return nil + } + if err != nil { + return err + } + + if len(branchName) > 0 { + branchName = branchName[:len(branchName)-1] + } + + if len(sha) > 0 { + sha = sha[:len(sha)-1] + } + + err = walkfn(sha, branchName) + if err != nil { + return err + } + i++ + } + // count all refs + for limit != 0 { + _, isPrefix, err := bufReader.ReadLine() + if err == io.EOF { + return nil + } + if err != nil { + return err + } + if !isPrefix { + i++ + } + } + return nil + }) + err = cmd.RunWithStderr(ctx) + if errPipeline := gitcmd.ErrorAsPipeline(err); errPipeline != nil { + return i, errPipeline // keep the old behavior: return pipeline error directly } - for limit == 0 || i < skip+limit { - // The output of show-ref is simply a list: - // SP LF - sha, err := bufReader.ReadString(' ') - if err == io.EOF { - return i, nil - } - if err != nil { - return 0, err - } - - branchName, err := bufReader.ReadString('\n') - if err == io.EOF { - // This shouldn't happen... but we'll tolerate it for the sake of peace - return i, nil - } - if err != nil { - return i, err - } - - if len(branchName) > 0 { - branchName = branchName[:len(branchName)-1] - } - - if len(sha) > 0 { - sha = sha[:len(sha)-1] - } - - err = walkfn(sha, branchName) - if err != nil { - return i, err - } - i++ - } - // count all refs - for limit != 0 { - _, isPrefix, err := bufReader.ReadLine() - if err == io.EOF { - return i, nil - } - if err != nil { - return 0, err - } - if !isPrefix { - i++ - } - } - return i, nil + return i, err } // GetRefsBySha returns all references filtered with prefix that belong to a sha commit hash diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index bf3f7238d2..c10f73690c 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -226,60 +226,55 @@ type CommitsByFileAndRangeOptions struct { // CommitsByFileAndRange return the commits according revision file and the page func (repo *Repository) CommitsByFileAndRange(opts CommitsByFileAndRangeOptions) ([]*Commit, error) { - stdoutReader, stdoutWriter := io.Pipe() - defer func() { - _ = stdoutReader.Close() - _ = stdoutWriter.Close() - }() - go func() { - gitCmd := gitcmd.NewCommand("rev-list"). - AddOptionFormat("--max-count=%d", setting.Git.CommitsRangeSize). - AddOptionFormat("--skip=%d", (opts.Page-1)*setting.Git.CommitsRangeSize) - gitCmd.AddDynamicArguments(opts.Revision) + gitCmd := gitcmd.NewCommand("rev-list"). + AddOptionFormat("--max-count=%d", setting.Git.CommitsRangeSize). + AddOptionFormat("--skip=%d", (opts.Page-1)*setting.Git.CommitsRangeSize) + gitCmd.AddDynamicArguments(opts.Revision) - if opts.Not != "" { - gitCmd.AddOptionValues("--not", opts.Not) - } - if opts.Since != "" { - gitCmd.AddOptionFormat("--since=%s", opts.Since) - } - if opts.Until != "" { - gitCmd.AddOptionFormat("--until=%s", opts.Until) - } - - gitCmd.AddDashesAndList(opts.File) - err := gitCmd.WithDir(repo.Path). - WithStdout(stdoutWriter). - RunWithStderr(repo.Ctx) - _ = stdoutWriter.CloseWithError(err) - }() - - objectFormat, err := repo.GetObjectFormat() - if err != nil { - return nil, err + if opts.Not != "" { + gitCmd.AddOptionValues("--not", opts.Not) } + if opts.Since != "" { + gitCmd.AddOptionFormat("--since=%s", opts.Since) + } + if opts.Until != "" { + gitCmd.AddOptionFormat("--until=%s", opts.Until) + } + gitCmd.AddDashesAndList(opts.File) - length := objectFormat.FullLength() - commits := []*Commit{} - shaline := make([]byte, length+1) - for { - n, err := io.ReadFull(stdoutReader, shaline) - if err != nil || n < length { - if err == io.EOF { - err = nil + var commits []*Commit + stdoutReader, stdoutReaderClose := gitCmd.MakeStdoutPipe() + defer stdoutReaderClose() + err := gitCmd.WithDir(repo.Path). + WithPipelineFunc(func(context gitcmd.Context) error { + objectFormat, err := repo.GetObjectFormat() + if err != nil { + return err } - return commits, err - } - objectID, err := NewIDFromString(string(shaline[0:length])) - if err != nil { - return nil, err - } - commit, err := repo.getCommit(objectID) - if err != nil { - return nil, err - } - commits = append(commits, commit) - } + + length := objectFormat.FullLength() + shaline := make([]byte, length+1) + for { + n, err := io.ReadFull(stdoutReader, shaline) + if err != nil || n < length { + if err == io.EOF { + err = nil + } + return err + } + objectID, err := NewIDFromString(string(shaline[0:length])) + if err != nil { + return err + } + commit, err := repo.getCommit(objectID) + if err != nil { + return err + } + commits = append(commits, commit) + } + }). + RunWithStderr(repo.Ctx) + return commits, err } // FilesCountBetween return the number of files changed between two commits diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index f78d569b83..aa25e2ec20 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -45,7 +45,7 @@ func (repo *Repository) GetDiffNumChangedFiles(base, head string, directComparis AddDynamicArguments(base + separator + head). AddArguments("--"). WithDir(repo.Path). - WithStdout(w). + WithStdoutCopy(w). RunWithStderr(repo.Ctx); err != nil { if strings.Contains(err.Stderr(), "no merge base") { // git >= 2.28 now returns an error if base and head have become unrelated. @@ -55,7 +55,7 @@ func (repo *Repository) GetDiffNumChangedFiles(base, head string, directComparis AddDynamicArguments(base, head). AddArguments("--"). WithDir(repo.Path). - WithStdout(w). + WithStdoutCopy(w). RunWithStderr(repo.Ctx); err == nil { return w.numLines, nil } @@ -71,7 +71,7 @@ var patchCommits = regexp.MustCompile(`^From\s(\w+)\s`) func (repo *Repository) GetDiff(compareArg string, w io.Writer) error { return gitcmd.NewCommand("diff", "-p").AddDynamicArguments(compareArg). WithDir(repo.Path). - WithStdout(w). + WithStdoutCopy(w). Run(repo.Ctx) } @@ -80,7 +80,7 @@ func (repo *Repository) GetDiffBinary(compareArg string, w io.Writer) error { return gitcmd.NewCommand("diff", "-p", "--binary", "--histogram"). AddDynamicArguments(compareArg). WithDir(repo.Path). - WithStdout(w). + WithStdoutCopy(w). Run(repo.Ctx) } @@ -88,7 +88,7 @@ func (repo *Repository) GetDiffBinary(compareArg string, w io.Writer) error { func (repo *Repository) GetPatch(compareArg string, w io.Writer) error { return gitcmd.NewCommand("format-patch", "--binary", "--stdout").AddDynamicArguments(compareArg). WithDir(repo.Path). - WithStdout(w). + WithStdoutCopy(w). Run(repo.Ctx) } diff --git a/modules/git/repo_index.go b/modules/git/repo_index.go index 4e62198570..1d040d5e0a 100644 --- a/modules/git/repo_index.go +++ b/modules/git/repo_index.go @@ -110,7 +110,7 @@ func (repo *Repository) RemoveFilesFromIndex(filenames ...string) error { } return cmd. WithDir(repo.Path). - WithStdin(bytes.NewReader(input.Bytes())). + WithStdinBytes(input.Bytes()). RunWithStderr(repo.Ctx) } @@ -130,7 +130,7 @@ func (repo *Repository) AddObjectsToIndex(objects ...IndexObjectInfo) error { } return cmd. WithDir(repo.Path). - WithStdin(bytes.NewReader(input.Bytes())). + WithStdinBytes(input.Bytes()). RunWithStderr(repo.Ctx) } diff --git a/modules/git/repo_object.go b/modules/git/repo_object.go index fbaaa707e7..38e16b4646 100644 --- a/modules/git/repo_object.go +++ b/modules/git/repo_object.go @@ -5,7 +5,6 @@ package git import ( - "io" "strings" "code.gitea.io/gitea/modules/git/gitcmd" @@ -32,18 +31,12 @@ func (o ObjectType) Bytes() []byte { return []byte(o) } -type EmptyReader struct{} - -func (EmptyReader) Read(p []byte) (int, error) { - return 0, io.EOF -} - func (repo *Repository) GetObjectFormat() (ObjectFormat, error) { if repo != nil && repo.objectFormat != nil { return repo.objectFormat, nil } - str, err := repo.hashObject(EmptyReader{}, false) + str, err := repo.hashObjectBytes(nil, false) if err != nil { return nil, err } @@ -57,16 +50,16 @@ func (repo *Repository) GetObjectFormat() (ObjectFormat, error) { return repo.objectFormat, nil } -// HashObject takes a reader and returns hash for that reader -func (repo *Repository) HashObject(reader io.Reader) (ObjectID, error) { - idStr, err := repo.hashObject(reader, true) +// HashObjectBytes returns hash for the content +func (repo *Repository) HashObjectBytes(buf []byte) (ObjectID, error) { + idStr, err := repo.hashObjectBytes(buf, true) if err != nil { return nil, err } return NewIDFromString(idStr) } -func (repo *Repository) hashObject(reader io.Reader, save bool) (string, error) { +func (repo *Repository) hashObjectBytes(buf []byte, save bool) (string, error) { var cmd *gitcmd.Command if save { cmd = gitcmd.NewCommand("hash-object", "-w", "--stdin") @@ -75,7 +68,7 @@ func (repo *Repository) hashObject(reader io.Reader, save bool) (string, error) } stdout, _, err := cmd. WithDir(repo.Path). - WithStdin(reader). + WithStdinBytes(buf). RunStdString(repo.Ctx) if err != nil { return "", err diff --git a/modules/git/repo_ref_nogogit.go b/modules/git/repo_ref_nogogit.go index a5737fb859..c58992fa9d 100644 --- a/modules/git/repo_ref_nogogit.go +++ b/modules/git/repo_ref_nogogit.go @@ -15,69 +15,61 @@ 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() - defer func() { - _ = stdoutReader.Close() - _ = stdoutWriter.Close() - }() - - go func() { - err := gitcmd.NewCommand("for-each-ref"). - WithDir(repo.Path). - WithStdout(stdoutWriter). - Run(repo.Ctx) - _ = stdoutWriter.CloseWithError(err) - }() - refs := make([]*Reference, 0) - bufReader := bufio.NewReader(stdoutReader) - for { - // The output of for-each-ref is simply a list: - // SP TAB LF - sha, err := bufReader.ReadString(' ') - if err == io.EOF { - break - } - if err != nil { - return nil, err - } - sha = sha[:len(sha)-1] + cmd := gitcmd.NewCommand("for-each-ref") + stdoutReader, stdoutReaderClose := cmd.MakeStdoutPipe() + defer stdoutReaderClose() + err := cmd.WithDir(repo.Path). + WithPipelineFunc(func(context gitcmd.Context) error { + bufReader := bufio.NewReader(stdoutReader) + for { + // The output of for-each-ref is simply a list: + // SP TAB LF + sha, err := bufReader.ReadString(' ') + if err == io.EOF { + break + } + if err != nil { + return err + } + sha = sha[:len(sha)-1] - typ, err := bufReader.ReadString('\t') - if err == io.EOF { - // This should not happen, but we'll tolerate it - break - } - if err != nil { - return nil, err - } - typ = typ[:len(typ)-1] + typ, err := bufReader.ReadString('\t') + if err == io.EOF { + // This should not happen, but we'll tolerate it + break + } + if err != nil { + return err + } + typ = typ[:len(typ)-1] - refName, err := bufReader.ReadString('\n') - if err == io.EOF { - // This should not happen, but we'll tolerate it - break - } - if err != nil { - return nil, err - } - refName = refName[:len(refName)-1] + refName, err := bufReader.ReadString('\n') + if err == io.EOF { + // This should not happen, but we'll tolerate it + break + } + if err != nil { + return err + } + refName = refName[:len(refName)-1] - // refName cannot be HEAD but can be remotes or stash - if strings.HasPrefix(refName, RemotePrefix) || refName == "/refs/stash" { - continue - } + // refName cannot be HEAD but can be remotes or stash + if strings.HasPrefix(refName, RemotePrefix) || refName == "/refs/stash" { + continue + } - if pattern == "" || strings.HasPrefix(refName, pattern) { - r := &Reference{ - Name: refName, - Object: MustIDFromString(sha), - Type: typ, - repo: repo, + if pattern == "" || strings.HasPrefix(refName, pattern) { + r := &Reference{ + Name: refName, + Object: MustIDFromString(sha), + Type: typ, + repo: repo, + } + refs = append(refs, r) + } } - refs = append(refs, r) - } - } - - return refs, nil + return nil + }).RunWithStderr(repo.Ctx) + return refs, err } diff --git a/modules/git/repo_stats.go b/modules/git/repo_stats.go index 2232fbadf4..1dd77f05d4 100644 --- a/modules/git/repo_stats.go +++ b/modules/git/repo_stats.go @@ -6,7 +6,6 @@ package git import ( "bufio" "fmt" - "io" "sort" "strconv" "strings" @@ -62,10 +61,10 @@ func (repo *Repository) GetCodeActivityStats(fromTime time.Time, branch string) gitCmd.AddArguments("--first-parent").AddDynamicArguments(branch) } - var stdoutReader io.ReadCloser + stdoutReader, stdoutReaderClose := gitCmd.MakeStdoutPipe() + defer stdoutReaderClose() err = gitCmd. WithDir(repo.Path). - WithStdoutReader(&stdoutReader). WithPipelineFunc(func(ctx gitcmd.Context) error { scanner := bufio.NewScanner(stdoutReader) scanner.Split(bufio.ScanLines) @@ -117,7 +116,6 @@ func (repo *Repository) GetCodeActivityStats(fromTime time.Time, branch string) } } if err = scanner.Err(); err != nil { - _ = stdoutReader.Close() return fmt.Errorf("GetCodeActivityStats scan: %w", err) } a := make([]*CodeActivityAuthor, 0, len(authors)) @@ -131,7 +129,6 @@ func (repo *Repository) GetCodeActivityStats(fromTime time.Time, branch string) stats.AuthorCount = int64(len(authors)) stats.ChangedFiles = int64(len(files)) stats.Authors = a - _ = stdoutReader.Close() return nil }). RunWithStderr(repo.Ctx) diff --git a/modules/git/repo_tag.go b/modules/git/repo_tag.go index dacf9b40ec..2599236ae0 100644 --- a/modules/git/repo_tag.go +++ b/modules/git/repo_tag.go @@ -6,7 +6,6 @@ package git import ( "fmt" - "io" "strings" "code.gitea.io/gitea/modules/git/foreachref" @@ -115,45 +114,42 @@ func (repo *Repository) GetTagInfos(page, pageSize int) ([]*Tag, int, error) { // https://git-scm.com/docs/git-for-each-ref#Documentation/git-for-each-ref.txt-refname forEachRefFmt := foreachref.NewFormat("objecttype", "refname:lstrip=2", "object", "objectname", "creator", "contents", "contents:signature") - stdoutReader, stdoutWriter := io.Pipe() - defer stdoutReader.Close() - defer stdoutWriter.Close() - - go func() { - err := gitcmd.NewCommand("for-each-ref"). - AddOptionFormat("--format=%s", forEachRefFmt.Flag()). - AddArguments("--sort", "-*creatordate", "refs/tags"). - WithDir(repo.Path). - WithStdout(stdoutWriter). - RunWithStderr(repo.Ctx) - _ = stdoutWriter.CloseWithError(err) - }() - var tags []*Tag - parser := forEachRefFmt.Parser(stdoutReader) - for { - ref := parser.Next() - if ref == nil { - break - } + var tagsTotal int + cmd := gitcmd.NewCommand("for-each-ref") + stdoutReader, stdoutReaderClose := cmd.MakeStdoutPipe() + defer stdoutReaderClose() + err := cmd.AddOptionFormat("--format=%s", forEachRefFmt.Flag()). + AddArguments("--sort", "-*creatordate", "refs/tags"). + WithDir(repo.Path). + WithPipelineFunc(func(context gitcmd.Context) error { + parser := forEachRefFmt.Parser(stdoutReader) + for { + ref := parser.Next() + if ref == nil { + break + } - tag, err := parseTagRef(ref) - if err != nil { - return nil, 0, fmt.Errorf("GetTagInfos: parse tag: %w", err) - } - tags = append(tags, tag) - } - if err := parser.Err(); err != nil { - return nil, 0, fmt.Errorf("GetTagInfos: parse output: %w", err) - } + tag, err := parseTagRef(ref) + if err != nil { + return fmt.Errorf("GetTagInfos: parse tag: %w", err) + } + tags = append(tags, tag) + } + if err := parser.Err(); err != nil { + return fmt.Errorf("GetTagInfos: parse output: %w", err) + } - sortTagsByTime(tags) - tagsTotal := len(tags) - if page != 0 { - tags = util.PaginateSlice(tags, page, pageSize).([]*Tag) - } + sortTagsByTime(tags) + tagsTotal = len(tags) + if page != 0 { + tags = util.PaginateSlice(tags, page, pageSize).([]*Tag) + } + return nil + }). + RunWithStderr(repo.Ctx) - return tags, tagsTotal, nil + return tags, tagsTotal, err } // parseTagRef parses a tag from a 'git for-each-ref'-produced reference. diff --git a/modules/git/repo_tree.go b/modules/git/repo_tree.go index 9b119e8426..e65e2441ed 100644 --- a/modules/git/repo_tree.go +++ b/modules/git/repo_tree.go @@ -60,7 +60,7 @@ func (repo *Repository) CommitTree(author, committer *Signature, tree *Tree, opt stdout, _, err := cmd.WithEnv(env). WithDir(repo.Path). - WithStdin(messageBytes). + WithStdinBytes(messageBytes.Bytes()). RunStdString(repo.Ctx) if err != nil { return nil, err diff --git a/modules/git/submodule.go b/modules/git/submodule.go index b32e6e4031..ed69cbe55d 100644 --- a/modules/git/submodule.go +++ b/modules/git/submodule.go @@ -7,7 +7,6 @@ import ( "bufio" "context" "fmt" - "io" "code.gitea.io/gitea/modules/git/gitcmd" "code.gitea.io/gitea/modules/log" @@ -21,10 +20,10 @@ type TemplateSubmoduleCommit struct { // GetTemplateSubmoduleCommits returns a list of submodules paths and their commits from a repository // This function is only for generating new repos based on existing template, the template couldn't be too large. func GetTemplateSubmoduleCommits(ctx context.Context, repoPath string) (submoduleCommits []TemplateSubmoduleCommit, _ error) { - var stdoutReader io.ReadCloser - err := gitcmd.NewCommand("ls-tree", "-r", "--", "HEAD"). - WithDir(repoPath). - WithStdoutReader(&stdoutReader). + cmd := gitcmd.NewCommand("ls-tree", "-r", "--", "HEAD") + stdoutReader, stdoutReaderClose := cmd.MakeStdoutPipe() + defer stdoutReaderClose() + err := cmd.WithDir(repoPath). WithPipelineFunc(func(ctx gitcmd.Context) error { scanner := bufio.NewScanner(stdoutReader) for scanner.Scan() { diff --git a/modules/gitrepo/archive.go b/modules/gitrepo/archive.go index 138639ec47..191a1bd2c0 100644 --- a/modules/gitrepo/archive.go +++ b/modules/gitrepo/archive.go @@ -36,7 +36,7 @@ func CreateArchive(ctx context.Context, repo Repository, format string, target i paths[i] = path.Clean(paths[i]) } cmd.AddDynamicArguments(paths...) - return RunCmdWithStderr(ctx, repo, cmd.WithStdout(target)) + return RunCmdWithStderr(ctx, repo, cmd.WithStdoutCopy(target)) } // CreateBundle create bundle content to the target path diff --git a/modules/gitrepo/blame.go b/modules/gitrepo/blame.go index 192cf2c91f..2352da1760 100644 --- a/modules/gitrepo/blame.go +++ b/modules/gitrepo/blame.go @@ -8,7 +8,6 @@ import ( "bytes" "context" "io" - "os" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git/gitcmd" @@ -34,8 +33,6 @@ type BlamePart struct { // BlameReader returns part of file blame one by one type BlameReader struct { - output io.WriteCloser - reader io.ReadCloser bufferedReader *bufio.Reader done chan error lastSha *string @@ -131,34 +128,42 @@ func (r *BlameReader) Close() error { err := <-r.done r.bufferedReader = nil - _ = r.reader.Close() - _ = r.output.Close() - for _, cleanup := range r.cleanupFuncs { - if cleanup != nil { - cleanup() - } - } + r.cleanup() return err } +func (r *BlameReader) cleanup() { + for _, cleanup := range r.cleanupFuncs { + cleanup() + } +} + // CreateBlameReader creates reader for given repository, commit and file -func CreateBlameReader(ctx context.Context, objectFormat git.ObjectFormat, repo Repository, commit *git.Commit, file string, bypassBlameIgnore bool) (rd *BlameReader, err error) { - var ignoreRevsFileName string - var ignoreRevsFileCleanup func() +func CreateBlameReader(ctx context.Context, objectFormat git.ObjectFormat, repo Repository, commit *git.Commit, file string, bypassBlameIgnore bool) (rd *BlameReader, retErr error) { defer func() { - if err != nil && ignoreRevsFileCleanup != nil { - ignoreRevsFileCleanup() + if retErr != nil { + rd.cleanup() } }() + rd = &BlameReader{ + done: make(chan error, 1), + objectFormat: objectFormat, + } + cmd := gitcmd.NewCommand("blame", "--porcelain") + stdoutReader, stdoutReaderClose := cmd.MakeStdoutPipe() + rd.bufferedReader = bufio.NewReader(stdoutReader) + rd.cleanupFuncs = append(rd.cleanupFuncs, stdoutReaderClose) + if git.DefaultFeatures().CheckVersionAtLeast("2.23") && !bypassBlameIgnore { - ignoreRevsFileName, ignoreRevsFileCleanup, err = tryCreateBlameIgnoreRevsFile(commit) + ignoreRevsFileName, ignoreRevsFileCleanup, err := tryCreateBlameIgnoreRevsFile(commit) if err != nil && !git.IsErrNotExist(err) { return nil, err - } - if ignoreRevsFileName != "" { + } else if err == nil { + rd.ignoreRevsFile = ignoreRevsFileName + rd.cleanupFuncs = append(rd.cleanupFuncs, ignoreRevsFileCleanup) // Possible improvement: use --ignore-revs-file /dev/stdin on unix // There is no equivalent on Windows. May be implemented if Gitea uses an external git backend. cmd.AddOptionValues("--ignore-revs-file", ignoreRevsFileName) @@ -167,28 +172,12 @@ func CreateBlameReader(ctx context.Context, objectFormat git.ObjectFormat, repo cmd.AddDynamicArguments(commit.ID.String()).AddDashesAndList(file) - done := make(chan error, 1) - reader, stdout, err := os.Pipe() - if err != nil { - return nil, err - } go func() { // TODO: it doesn't work for directories (the directories shouldn't be "blamed"), and the "err" should be returned by "Read" but not by "Close" - err := RunCmdWithStderr(ctx, repo, cmd.WithStdout(stdout)) - done <- err - _ = stdout.Close() + rd.done <- RunCmdWithStderr(ctx, repo, cmd) }() - bufferedReader := bufio.NewReader(reader) - return &BlameReader{ - output: stdout, - reader: reader, - bufferedReader: bufferedReader, - done: done, - ignoreRevsFile: ignoreRevsFileName, - objectFormat: objectFormat, - cleanupFuncs: []func(){ignoreRevsFileCleanup}, - }, nil + return rd, nil } func tryCreateBlameIgnoreRevsFile(commit *git.Commit) (string, func(), error) { diff --git a/modules/gitrepo/commit_file.go b/modules/gitrepo/commit_file.go index 0cd4f99130..437b3b51ad 100644 --- a/modules/gitrepo/commit_file.go +++ b/modules/gitrepo/commit_file.go @@ -67,20 +67,18 @@ func parseCommitFileStatus(fileStatus *CommitFileStatus, stdout io.Reader) { // GetCommitFileStatus returns file status of commit in given repository. func GetCommitFileStatus(ctx context.Context, repo Repository, commitID string) (*CommitFileStatus, error) { - stdout, w := io.Pipe() + cmd := gitcmd.NewCommand("log", "--name-status", "-m", "--pretty=format:", "--first-parent", "--no-renames", "-z", "-1") + stdout, stdoutClose := cmd.MakeStdoutPipe() + defer stdoutClose() done := make(chan struct{}) fileStatus := NewCommitFileStatus() go func() { parseCommitFileStatus(fileStatus, stdout) close(done) }() - - err := gitcmd.NewCommand("log", "--name-status", "-m", "--pretty=format:", "--first-parent", "--no-renames", "-z", "-1"). - AddDynamicArguments(commitID). + err := cmd.AddDynamicArguments(commitID). WithDir(repoPath(repo)). - WithStdout(w). RunWithStderr(ctx) - _ = w.Close() // Close writer to exit parsing goroutine if err != nil { return nil, err } diff --git a/modules/gitrepo/diff.go b/modules/gitrepo/diff.go index e1be47904a..0092cf0bb8 100644 --- a/modules/gitrepo/diff.go +++ b/modules/gitrepo/diff.go @@ -66,6 +66,6 @@ func parseDiffStat(stdout string) (numFiles, totalAdditions, totalDeletions int, func GetReverseRawDiff(ctx context.Context, repo Repository, commitID string, writer io.Writer) error { return RunCmdWithStderr(ctx, repo, gitcmd.NewCommand("show", "--pretty=format:revert %H%n", "-R"). AddDynamicArguments(commitID). - WithStdout(writer), + WithStdoutCopy(writer), ) } diff --git a/modules/lfs/pointer_scanner_gogit.go b/modules/lfs/pointer_scanner_gogit.go index e153b8e24e..ccfb16b6c0 100644 --- a/modules/lfs/pointer_scanner_gogit.go +++ b/modules/lfs/pointer_scanner_gogit.go @@ -15,7 +15,7 @@ import ( ) // SearchPointerBlobs scans the whole repository for LFS pointer files -func SearchPointerBlobs(ctx context.Context, repo *git.Repository, pointerChan chan<- PointerBlob, errChan chan<- error) { +func SearchPointerBlobs(ctx context.Context, repo *git.Repository, pointerChan chan<- PointerBlob) error { gitRepo := repo.GoGitRepo() err := func() error { @@ -49,14 +49,7 @@ func SearchPointerBlobs(ctx context.Context, repo *git.Repository, pointerChan c return nil }) }() - if err != nil { - select { - case <-ctx.Done(): - default: - errChan <- err - } - } close(pointerChan) - close(errChan) + return err } diff --git a/modules/lfs/pointer_scanner_nogogit.go b/modules/lfs/pointer_scanner_nogogit.go index c37a93e73b..29f5d0e346 100644 --- a/modules/lfs/pointer_scanner_nogogit.go +++ b/modules/lfs/pointer_scanner_nogogit.go @@ -8,96 +8,84 @@ package lfs import ( "bufio" "context" + "errors" "io" "strconv" "strings" - "sync" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/git/gitcmd" "code.gitea.io/gitea/modules/git/pipeline" + "code.gitea.io/gitea/modules/util" + + "golang.org/x/sync/errgroup" ) // SearchPointerBlobs scans the whole repository for LFS pointer files -func SearchPointerBlobs(ctx context.Context, repo *git.Repository, pointerChan chan<- PointerBlob, errChan chan<- error) { - basePath := repo.Path +func SearchPointerBlobs(ctx context.Context, repo *git.Repository, pointerChan chan<- PointerBlob) error { + cmd1AllObjs, cmd3BatchContent := gitcmd.NewCommand(), gitcmd.NewCommand() - catFileCheckReader, catFileCheckWriter := io.Pipe() - shasToBatchReader, shasToBatchWriter := io.Pipe() - catFileBatchReader, catFileBatchWriter := io.Pipe() + cmd1AllObjsStdout, cmd1AllObjsStdoutClose := cmd1AllObjs.MakeStdoutPipe() + defer cmd1AllObjsStdoutClose() - wg := sync.WaitGroup{} - wg.Add(4) - - // Create the go-routines in reverse order. + cmd3BatchContentIn, cmd3BatchContentOut, cmd3BatchContentClose := cmd3BatchContent.MakeStdinStdoutPipe() + defer cmd3BatchContentClose() + // Create the go-routines in reverse order (update: the order is not needed any more, the pipes are properly prepared) + wg := errgroup.Group{} // 4. Take the output of cat-file --batch and check if each file in turn // to see if they're pointers to files in the LFS store - go createPointerResultsFromCatFileBatch(ctx, catFileBatchReader, &wg, pointerChan) + wg.Go(func() error { + return createPointerResultsFromCatFileBatch(cmd3BatchContentOut, pointerChan) + }) // 3. Take the shas of the blobs and batch read them - go pipeline.CatFileBatch(ctx, shasToBatchReader, catFileBatchWriter, &wg, basePath) + wg.Go(func() error { + return pipeline.CatFileBatch(ctx, cmd3BatchContent, repo.Path) + }) // 2. From the provided objects restrict to blobs <=1k - go pipeline.BlobsLessThan1024FromCatFileBatchCheck(catFileCheckReader, shasToBatchWriter, &wg) + wg.Go(func() error { + return pipeline.BlobsLessThan1024FromCatFileBatchCheck(cmd1AllObjsStdout, cmd3BatchContentIn) + }) // 1. Run batch-check on all objects in the repository - if !git.DefaultFeatures().CheckVersionAtLeast("2.6.0") { - revListReader, revListWriter := io.Pipe() - shasToCheckReader, shasToCheckWriter := io.Pipe() - wg.Add(2) - go pipeline.CatFileBatchCheck(ctx, shasToCheckReader, catFileCheckWriter, &wg, basePath) - go pipeline.BlobsFromRevListObjects(revListReader, shasToCheckWriter, &wg) - go pipeline.RevListAllObjects(ctx, revListWriter, &wg, basePath, errChan) - } else { - go pipeline.CatFileBatchCheckAllObjects(ctx, catFileCheckWriter, &wg, basePath, errChan) - } - wg.Wait() - + wg.Go(func() error { + return pipeline.CatFileBatchCheckAllObjects(ctx, cmd1AllObjs, repo.Path) + }) + err := wg.Wait() close(pointerChan) - close(errChan) + return err } -func createPointerResultsFromCatFileBatch(ctx context.Context, catFileBatchReader *io.PipeReader, wg *sync.WaitGroup, pointerChan chan<- PointerBlob) { - defer wg.Done() +func createPointerResultsFromCatFileBatch(catFileBatchReader io.ReadCloser, pointerChan chan<- PointerBlob) error { defer catFileBatchReader.Close() bufferedReader := bufio.NewReader(catFileBatchReader) buf := make([]byte, 1025) -loop: for { - select { - case <-ctx.Done(): - break loop - default: - } - // File descriptor line: sha sha, err := bufferedReader.ReadString(' ') if err != nil { - _ = catFileBatchReader.CloseWithError(err) - break + return util.Iif(errors.Is(err, io.EOF), nil, err) } sha = strings.TrimSpace(sha) // Throw away the blob if _, err := bufferedReader.ReadString(' '); err != nil { - _ = catFileBatchReader.CloseWithError(err) - break + return err } sizeStr, err := bufferedReader.ReadString('\n') if err != nil { - _ = catFileBatchReader.CloseWithError(err) - break + return err } size, err := strconv.Atoi(sizeStr[:len(sizeStr)-1]) if err != nil { - _ = catFileBatchReader.CloseWithError(err) - break + return err } pointerBuf := buf[:size+1] if _, err := io.ReadFull(bufferedReader, pointerBuf); err != nil { - _ = catFileBatchReader.CloseWithError(err) - break + return err } pointerBuf = pointerBuf[:size] // Now we need to check if the pointerBuf is an LFS pointer @@ -105,7 +93,6 @@ loop: if !pointer.IsValid() { continue } - pointerChan <- PointerBlob{Hash: sha, Pointer: pointer} } } diff --git a/modules/repository/repo.go b/modules/repository/repo.go index 9353043f91..9013590247 100644 --- a/modules/repository/repo.go +++ b/modules/repository/repo.go @@ -62,7 +62,9 @@ func StoreMissingLfsObjectsInRepository(ctx context.Context, repo *repo_model.Re pointerChan := make(chan lfs.PointerBlob) errChan := make(chan error, 1) - go lfs.SearchPointerBlobs(ctx, gitRepo, pointerChan, errChan) + go func() { + errChan <- lfs.SearchPointerBlobs(ctx, gitRepo, pointerChan) + }() downloadObjects := func(pointers []lfs.Pointer) error { err := lfsClient.Download(ctx, pointers, func(p lfs.Pointer, content io.ReadCloser, objectError error) error { @@ -150,13 +152,12 @@ func StoreMissingLfsObjectsInRepository(ctx context.Context, repo *repo_model.Re } } - err, has := <-errChan - if has { + err := <-errChan + if err != nil { log.Error("Repo[%-v]: Error enumerating LFS objects for repository: %v", repo, err) - return err } - return nil + return err } // shortRelease to reduce load memory, this struct can replace repo_model.Release diff --git a/routers/private/hook_verification.go b/routers/private/hook_verification.go index 0867028c4c..71ba9cc74c 100644 --- a/routers/private/hook_verification.go +++ b/routers/private/hook_verification.go @@ -27,10 +27,11 @@ func verifyCommits(oldCommitID, newCommitID string, repo *git.Repository, env [] command = gitcmd.NewCommand("rev-list").AddDynamicArguments(oldCommitID + "..." + newCommitID) } // This is safe as force pushes are already forbidden - var stdoutReader io.ReadCloser + stdoutReader, stdoutReaderClose := command.MakeStdoutPipe() + defer stdoutReaderClose() + err := command.WithEnv(env). WithDir(repo.Path). - WithStdoutReader(&stdoutReader). WithPipelineFunc(func(ctx gitcmd.Context) error { err := readAndVerifyCommitsFromShaReader(stdoutReader, repo, env) return ctx.CancelWithCause(err) @@ -56,11 +57,12 @@ func readAndVerifyCommitsFromShaReader(input io.ReadCloser, repo *git.Repository func readAndVerifyCommit(sha string, repo *git.Repository, env []string) error { commitID := git.MustIDFromString(sha) - var stdoutReader io.ReadCloser - return gitcmd.NewCommand("cat-file", "commit").AddDynamicArguments(sha). - WithEnv(env). + cmd := gitcmd.NewCommand("cat-file", "commit").AddDynamicArguments(sha) + stdoutReader, stdoutReaderClose := cmd.MakeStdoutPipe() + defer stdoutReaderClose() + + return cmd.WithEnv(env). WithDir(repo.Path). - WithStdoutReader(&stdoutReader). WithPipelineFunc(func(ctx gitcmd.Context) error { commit, err := git.CommitFromReader(repo, commitID, stdoutReader) if err != nil { diff --git a/routers/web/repo/githttp.go b/routers/web/repo/githttp.go index e169955832..8b3deb5a03 100644 --- a/routers/web/repo/githttp.go +++ b/routers/web/repo/githttp.go @@ -446,8 +446,8 @@ func serviceRPC(ctx *context.Context, service string) { if err := gitrepo.RunCmdWithStderr(ctx, h.getStorageRepo(), cmd.AddArguments("."). WithEnv(append(os.Environ(), h.environ...)). - WithStdin(reqBody). - WithStdout(ctx.Resp), + WithStdinCopy(reqBody). + WithStdoutCopy(ctx.Resp), ); err != nil { if !gitcmd.IsErrorCanceledOrKilled(err) { log.Error("Fail to serve RPC(%s) in %s: %v", service, h.getStorageRepo().RelativePath(), err) diff --git a/routers/web/repo/setting/lfs.go b/routers/web/repo/setting/lfs.go index c7a19062d2..8a8015035f 100644 --- a/routers/web/repo/setting/lfs.go +++ b/routers/web/repo/setting/lfs.go @@ -407,7 +407,9 @@ func LFSPointerFiles(ctx *context.Context) { err = func() error { pointerChan := make(chan lfs.PointerBlob) errChan := make(chan error, 1) - go lfs.SearchPointerBlobs(ctx, ctx.Repo.GitRepo, pointerChan, errChan) + go func() { + errChan <- lfs.SearchPointerBlobs(ctx, ctx.Repo.GitRepo, pointerChan) + }() numPointers := 0 var numAssociated, numNoExist, numAssociatable int @@ -483,11 +485,6 @@ func LFSPointerFiles(ctx *context.Context) { results = append(results, result) } - err, has := <-errChan - if has { - return err - } - ctx.Data["Pointers"] = results ctx.Data["NumPointers"] = numPointers ctx.Data["NumAssociated"] = numAssociated @@ -495,7 +492,8 @@ func LFSPointerFiles(ctx *context.Context) { ctx.Data["NumNoExist"] = numNoExist ctx.Data["NumNotAssociated"] = numPointers - numAssociated - return nil + err := <-errChan + return err }() if err != nil { ctx.ServerError("LFSPointerFiles", err) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index fb1bd7f4ef..06c19b90d9 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1263,21 +1263,14 @@ func getDiffBasic(ctx context.Context, gitRepo *git.Repository, opts *DiffOption cmdCtx, cmdCancel := context.WithCancel(ctx) defer cmdCancel() - reader, writer := io.Pipe() - defer func() { - _ = reader.Close() - _ = writer.Close() - }() - + reader, readerClose := cmdDiff.MakeStdoutPipe() + defer readerClose() go func() { if err := cmdDiff. WithDir(repoPath). - WithStdout(writer). RunWithStderr(cmdCtx); err != nil && !gitcmd.IsErrorCanceledOrKilled(err) { log.Error("error during GetDiff(git diff dir: %s): %v", repoPath, err) } - - _ = writer.Close() }() diff, err := ParsePatch(cmdCtx, opts.MaxLines, opts.MaxLineCharacters, opts.MaxFiles, reader, parsePatchSkipToFile) diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index ebe2363cf7..eeada3100d 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -896,7 +896,7 @@ func (g *GiteaLocalUploader) CreateReviews(ctx context.Context, reviews ...*base comment.TreePath = util.PathJoinRel(comment.TreePath) var patch string - reader, writer := io.Pipe() + reader, writer := io.Pipe() // FIXME: use os.Pipe to avoid deadlock defer func() { _ = reader.Close() _ = writer.Close() diff --git a/services/mirror/mirror_push.go b/services/mirror/mirror_push.go index bae189ba87..1acb227ac0 100644 --- a/services/mirror/mirror_push.go +++ b/services/mirror/mirror_push.go @@ -192,7 +192,9 @@ func pushAllLFSObjects(ctx context.Context, gitRepo *git.Repository, lfsClient l pointerChan := make(chan lfs.PointerBlob) errChan := make(chan error, 1) - go lfs.SearchPointerBlobs(ctx, gitRepo, pointerChan, errChan) + go func() { + errChan <- lfs.SearchPointerBlobs(ctx, gitRepo, pointerChan) + }() uploadObjects := func(pointers []lfs.Pointer) error { err := lfsClient.Upload(ctx, pointers, func(p lfs.Pointer, objectError error) (io.ReadCloser, error) { @@ -242,13 +244,12 @@ func pushAllLFSObjects(ctx context.Context, gitRepo *git.Repository, lfsClient l } } - err, has := <-errChan - if has { + err := <-errChan + if err != nil { log.Error("Error enumerating LFS objects for repository: %v", err) - return err } - return nil + return err } func syncPushMirrorWithSyncOnCommit(ctx context.Context, repoID int64) { diff --git a/services/pull/lfs.go b/services/pull/lfs.go index eb2a08ed8d..094b563b92 100644 --- a/services/pull/lfs.go +++ b/services/pull/lfs.go @@ -7,15 +7,19 @@ package pull import ( "bufio" "context" + "errors" "io" "strconv" - "sync" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/modules/git/gitcmd" "code.gitea.io/gitea/modules/git/pipeline" "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" + + "golang.org/x/sync/errgroup" ) // LFSPush pushes lfs objects referred to in new commits in the head repository from the base repository @@ -26,81 +30,82 @@ 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() - errChan := make(chan error, 1) - wg := sync.WaitGroup{} - wg.Add(6) - // Create the go-routines in reverse order. + + cmd1RevList, cmd3BathCheck, cmd5BatchContent := gitcmd.NewCommand(), gitcmd.NewCommand(), gitcmd.NewCommand() + cmd1RevListOut, cmd1RevListClose := cmd1RevList.MakeStdoutPipe() + defer cmd1RevListClose() + + cmd3BatchCheckIn, cmd3BatchCheckOut, cmd3BatchCheckClose := cmd3BathCheck.MakeStdinStdoutPipe() + defer cmd3BatchCheckClose() + + cmd5BatchContentIn, cmd5BatchContentOut, cmd5BatchContentClose := cmd5BatchContent.MakeStdinStdoutPipe() + defer cmd5BatchContentClose() + + // Create the go-routines in reverse order (update: the order is not needed any more, the pipes are properly prepared) + wg := &errgroup.Group{} // 6. Take the output of cat-file --batch and check if each file in turn // to see if they're pointers to files in the LFS store associated with // the head repo and add them to the base repo if so - go createLFSMetaObjectsFromCatFileBatch(ctx, catFileBatchReader, &wg, pr) + wg.Go(func() error { + return createLFSMetaObjectsFromCatFileBatch(ctx, cmd5BatchContentOut, pr) + }) // 5. Take the shas of the blobs and batch read them - go pipeline.CatFileBatch(ctx, shasToBatchReader, catFileBatchWriter, &wg, tmpBasePath) + wg.Go(func() error { + return pipeline.CatFileBatch(ctx, cmd5BatchContent, tmpBasePath) + }) // 4. From the provided objects restrict to blobs <=1k - go pipeline.BlobsLessThan1024FromCatFileBatchCheck(catFileCheckReader, shasToBatchWriter, &wg) + wg.Go(func() error { + return pipeline.BlobsLessThan1024FromCatFileBatchCheck(cmd3BatchCheckOut, cmd5BatchContentIn) + }) // 3. Run batch-check on the objects retrieved from rev-list - go pipeline.CatFileBatchCheck(ctx, shasToCheckReader, catFileCheckWriter, &wg, tmpBasePath) + wg.Go(func() error { + return pipeline.CatFileBatchCheck(ctx, cmd3BathCheck, tmpBasePath) + }) // 2. Check each object retrieved rejecting those without names as they will be commits or trees - go pipeline.BlobsFromRevListObjects(revListReader, shasToCheckWriter, &wg) + wg.Go(func() error { + return pipeline.BlobsFromRevListObjects(cmd1RevListOut, cmd3BatchCheckIn) + }) // 1. Run rev-list objects from mergeHead to mergeBase - go pipeline.RevListObjects(ctx, revListWriter, &wg, tmpBasePath, mergeHeadSHA, mergeBaseSHA, errChan) + wg.Go(func() error { + return pipeline.RevListObjects(ctx, cmd1RevList, tmpBasePath, mergeHeadSHA, mergeBaseSHA) + }) - wg.Wait() - select { - case err, has := <-errChan: - if has { - return err - } - default: - } - return nil + return wg.Wait() } -func createLFSMetaObjectsFromCatFileBatch(ctx context.Context, catFileBatchReader *io.PipeReader, wg *sync.WaitGroup, pr *issues_model.PullRequest) { - defer wg.Done() +func createLFSMetaObjectsFromCatFileBatch(ctx context.Context, catFileBatchReader io.ReadCloser, pr *issues_model.PullRequest) error { defer catFileBatchReader.Close() contentStore := lfs.NewContentStore() - bufferedReader := bufio.NewReader(catFileBatchReader) buf := make([]byte, 1025) for { // File descriptor line: sha _, err := bufferedReader.ReadString(' ') if err != nil { - _ = catFileBatchReader.CloseWithError(err) - break + return util.Iif(errors.Is(err, io.EOF), nil, err) } // Throw away the blob if _, err := bufferedReader.ReadString(' '); err != nil { - _ = catFileBatchReader.CloseWithError(err) - break + return err } sizeStr, err := bufferedReader.ReadString('\n') if err != nil { - _ = catFileBatchReader.CloseWithError(err) - break + return err } size, err := strconv.Atoi(sizeStr[:len(sizeStr)-1]) if err != nil { - _ = catFileBatchReader.CloseWithError(err) - break + return err } pointerBuf := buf[:size+1] if _, err := io.ReadFull(bufferedReader, pointerBuf); err != nil { - _ = catFileBatchReader.CloseWithError(err) - break + return err } pointerBuf = pointerBuf[:size] // Now we need to check if the pointerBuf is an LFS pointer @@ -120,15 +125,13 @@ func createLFSMetaObjectsFromCatFileBatch(ctx context.Context, catFileBatchReade log.Warn("During merge of: %d in %-v, there is a pointer to LFS Oid: %s which although present in the LFS store is not associated with the head repo %-v", pr.Index, pr.BaseRepo, pointer.Oid, pr.HeadRepo) continue } - _ = catFileBatchReader.CloseWithError(err) - break + return err } // OK we have a pointer that is associated with the head repo // and is actually a file in the LFS // Therefore it should be associated with the base repo if _, err := git_model.NewLFSMetaObject(ctx, pr.BaseRepoID, pointer); err != nil { - _ = catFileBatchReader.CloseWithError(err) - break + return err } } } diff --git a/services/pull/merge_prepare.go b/services/pull/merge_prepare.go index 2c9727df52..cee62275d3 100644 --- a/services/pull/merge_prepare.go +++ b/services/pull/merge_prepare.go @@ -41,7 +41,7 @@ func (ctx *mergeContext) PrepareGitCmd(cmd *gitcmd.Command) *gitcmd.Command { return cmd.WithEnv(ctx.env). WithDir(ctx.tmpBasePath). WithParentCallerInfo(). - WithStdout(ctx.outbuf) + WithStdoutBuffer(ctx.outbuf) } // ErrSHADoesNotMatch represents a "SHADoesNotMatch" kind of error. @@ -219,11 +219,11 @@ func getDiffTree(ctx context.Context, repoPath, baseBranch, headBranch string, o return 0, nil, nil } - var diffOutReader io.ReadCloser - err := gitcmd.NewCommand("diff-tree", "--no-commit-id", "--name-only", "-r", "-r", "-z", "--root"). - AddDynamicArguments(baseBranch, headBranch). + cmd := gitcmd.NewCommand("diff-tree", "--no-commit-id", "--name-only", "-r", "-r", "-z", "--root") + diffOutReader, diffOutReaderClose := cmd.MakeStdoutPipe() + defer diffOutReaderClose() + err := cmd.AddDynamicArguments(baseBranch, headBranch). WithDir(repoPath). - WithStdoutReader(&diffOutReader). WithPipelineFunc(func(ctx gitcmd.Context) error { // Now scan the output from the command scanner := bufio.NewScanner(diffOutReader) diff --git a/services/pull/patch.go b/services/pull/patch.go index 30b8f964de..f0429e7bec 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -414,13 +414,13 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo * // - alternatively we can do the equivalent of: // `git apply --check ... | grep ...` // meaning we don't store all the conflicts unnecessarily. - var stderrReader io.ReadCloser + stderrReader, stderrReaderClose := cmdApply.MakeStderrPipe() + defer stderrReaderClose() // 8. Run the check command conflict = false err = cmdApply. WithDir(tmpBasePath). - WithStderrReader(&stderrReader). WithPipelineFunc(func(ctx gitcmd.Context) error { const prefix = "error: patch failed:" const errorPrefix = "error: " diff --git a/services/pull/patch_unmerged.go b/services/pull/patch_unmerged.go index ca0e515616..b8416abab2 100644 --- a/services/pull/patch_unmerged.go +++ b/services/pull/patch_unmerged.go @@ -59,10 +59,10 @@ func readUnmergedLsFileLines(ctx context.Context, tmpBasePath string, outputChan close(outputChan) }() - var lsFilesReader io.ReadCloser - err := gitcmd.NewCommand("ls-files", "-u", "-z"). - WithDir(tmpBasePath). - WithStdoutReader(&lsFilesReader). + cmd := gitcmd.NewCommand("ls-files", "-u", "-z") + lsFilesReader, lsFilesReaderClose := cmd.MakeStdoutPipe() + defer lsFilesReaderClose() + err := cmd.WithDir(tmpBasePath). WithPipelineFunc(func(_ gitcmd.Context) error { bufferedReader := bufio.NewReader(lsFilesReader) diff --git a/services/pull/pull.go b/services/pull/pull.go index 490d23abca..60d0b2776b 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -526,9 +526,9 @@ func checkIfPRContentChanged(ctx context.Context, pr *issues_model.PullRequest, cmd := gitcmd.NewCommand("diff", "--name-only", "-z").AddDynamicArguments(newCommitID, oldCommitID, mergeBase) - var stdoutReader io.ReadCloser + stdoutReader, stdoutReaderClose := cmd.MakeStdoutPipe() + defer stdoutReaderClose() if err := cmd.WithDir(prCtx.tmpBasePath). - WithStdoutReader(&stdoutReader). WithPipelineFunc(func(ctx gitcmd.Context) error { return util.IsEmptyReader(stdoutReader) }). diff --git a/services/pull/review.go b/services/pull/review.go index 6e16205e70..bcd7392063 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -274,7 +274,7 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo if len(commitID) == 0 { commitID = headCommitID } - reader, writer := io.Pipe() + reader, writer := io.Pipe() // FIXME: use os.Pipe to avoid deadlock defer func() { _ = reader.Close() _ = writer.Close() diff --git a/services/pull/temp_repo.go b/services/pull/temp_repo.go index f5c24a1bd2..ec546ac997 100644 --- a/services/pull/temp_repo.go +++ b/services/pull/temp_repo.go @@ -5,11 +5,11 @@ package pull import ( + "bytes" "context" "fmt" "os" "path/filepath" - "strings" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" @@ -32,7 +32,7 @@ type prTmpRepoContext struct { context.Context tmpBasePath string pr *issues_model.PullRequest - outbuf *strings.Builder // we keep these around to help reduce needless buffer recreation, any use should be preceded by a Reset and preferably after use + outbuf *bytes.Buffer // we keep these around to help reduce needless buffer recreation, any use should be preceded by a Reset and preferably after use } // PrepareGitCmd prepares a git command with the correct directory, environment, and output buffers @@ -40,7 +40,7 @@ type prTmpRepoContext struct { // Do NOT use it with gitcmd.RunStd*() functions, otherwise it will panic func (ctx *prTmpRepoContext) PrepareGitCmd(cmd *gitcmd.Command) *gitcmd.Command { ctx.outbuf.Reset() - return cmd.WithDir(ctx.tmpBasePath).WithStdout(ctx.outbuf) + return cmd.WithDir(ctx.tmpBasePath).WithStdoutBuffer(ctx.outbuf) } // createTemporaryRepoForPR creates a temporary repo with "base" for pr.BaseBranch and "tracking" for pr.HeadBranch @@ -82,7 +82,7 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest) Context: ctx, tmpBasePath: tmpBasePath, pr: pr, - outbuf: &strings.Builder{}, + outbuf: &bytes.Buffer{}, } baseRepoPath := pr.BaseRepo.RepoPath() diff --git a/services/pull/update_rebase.go b/services/pull/update_rebase.go index c267b6978a..a107bdf376 100644 --- a/services/pull/update_rebase.go +++ b/services/pull/update_rebase.go @@ -82,7 +82,7 @@ func updateHeadByRebaseOnToBase(ctx context.Context, pr *issues_model.PullReques pr.Index, )). WithDir(mergeCtx.tmpBasePath). - WithStdout(mergeCtx.outbuf). + WithStdoutBuffer(mergeCtx.outbuf). RunWithStderr(ctx); err != nil { if strings.Contains(err.Stderr(), "non-fast-forward") { return &git.ErrPushOutOfDate{ diff --git a/services/repository/branch.go b/services/repository/branch.go index 142073eabe..b076514590 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -264,12 +264,12 @@ func checkBranchName(ctx context.Context, repo *repo_model.Repository, name stri return git_model.ErrBranchAlreadyExists{ BranchName: name, } - // If branchRefName like a/b but we want to create a branch named a then we have a conflict + // If branchRefName like "a/b" but we want to create a branch named a then we have a conflict case strings.HasPrefix(branchRefName, name+"/"): return git_model.ErrBranchNameConflict{ BranchName: branchRefName, } - // Conversely if branchRefName like a but we want to create a branch named a/b then we also have a conflict + // Conversely if branchRefName like "a" but we want to create a branch named "a/b" then we also have a conflict case strings.HasPrefix(name, branchRefName+"/"): return git_model.ErrBranchNameConflict{ BranchName: branchRefName, @@ -281,7 +281,6 @@ func checkBranchName(ctx context.Context, repo *repo_model.Repository, name stri } return nil }) - return err } diff --git a/services/repository/contributors_graph.go b/services/repository/contributors_graph.go index 952aed6d9a..c9cc0dcd0b 100644 --- a/services/repository/contributors_graph.go +++ b/services/repository/contributors_graph.go @@ -8,7 +8,6 @@ import ( "context" "errors" "fmt" - "io" "strconv" "strings" "sync" @@ -122,10 +121,11 @@ func getExtendedCommitStats(repo *git.Repository, revision string /*, limit int // AddOptionFormat("--max-count=%d", limit) gitCmd.AddDynamicArguments(baseCommit.ID.String()) - var stdoutReader io.ReadCloser + stdoutReader, stdoutReaderClose := gitCmd.MakeStdoutPipe() + defer stdoutReaderClose() + var extendedCommitStats []*ExtendedCommitStats err = gitCmd.WithDir(repo.Path). - WithStdoutReader(&stdoutReader). WithPipelineFunc(func(ctx gitcmd.Context) error { scanner := bufio.NewScanner(stdoutReader) diff --git a/services/repository/files/patch.go b/services/repository/files/patch.go index 1f6fd033ed..10f923f2e1 100644 --- a/services/repository/files/patch.go +++ b/services/repository/files/patch.go @@ -170,7 +170,7 @@ func ApplyDiffPatch(ctx context.Context, repo *repo_model.Repository, doer *user } if err := cmdApply.WithDir(t.basePath). - WithStdin(strings.NewReader(opts.Content)). + WithStdinBytes([]byte(opts.Content)). RunWithStderr(ctx); err != nil { return nil, fmt.Errorf("git apply error: %w", err) } diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go index 9e2a1ec91b..63f4f06d25 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -117,7 +117,7 @@ func (t *TemporaryUploadRepository) LsFiles(ctx context.Context, filenames ...st stdOut := new(bytes.Buffer) if err := gitcmd.NewCommand("ls-files", "-z").AddDashesAndList(filenames...). WithDir(t.basePath). - WithStdout(stdOut). + WithStdoutBuffer(stdOut). RunWithStderr(ctx); err != nil { return nil, fmt.Errorf("unable to run git ls-files for temporary repo of: %s, error: %w", t.repo.FullName(), err) } @@ -155,7 +155,7 @@ func (t *TemporaryUploadRepository) RemoveFilesFromIndex(ctx context.Context, fi if err := gitcmd.NewCommand("update-index", "--remove", "-z", "--index-info"). WithDir(t.basePath). - WithStdin(stdIn). + WithStdinBytes(stdIn.Bytes()). RunWithStderr(ctx); err != nil { return fmt.Errorf("unable to update-index for temporary repo: %q, error: %w", t.repo.FullName(), err) } @@ -167,8 +167,8 @@ func (t *TemporaryUploadRepository) HashObjectAndWrite(ctx context.Context, cont stdOut := new(bytes.Buffer) if err := gitcmd.NewCommand("hash-object", "-w", "--stdin"). WithDir(t.basePath). - WithStdout(stdOut). - WithStdin(content). + WithStdoutBuffer(stdOut). + WithStdinCopy(content). RunWithStderr(ctx); err != nil { return "", fmt.Errorf("unable to hash-object to temporary repo: %s, error: %w", t.repo.FullName(), err) } @@ -330,8 +330,8 @@ func (t *TemporaryUploadRepository) CommitTree(ctx context.Context, opts *Commit if err := cmdCommitTree. WithEnv(env). WithDir(t.basePath). - WithStdout(stdout). - WithStdin(messageBytes). + WithStdoutBuffer(stdout). + WithStdinBytes(messageBytes.Bytes()). RunWithStderr(ctx); err != nil { return "", fmt.Errorf("unable to commit-tree in temporary repo: %s Error: %w", t.repo.FullName(), err) } @@ -363,11 +363,12 @@ func (t *TemporaryUploadRepository) Push(ctx context.Context, doer *user_model.U // DiffIndex returns a Diff of the current index to the head func (t *TemporaryUploadRepository) DiffIndex(ctx context.Context) (*gitdiff.Diff, error) { var diff *gitdiff.Diff - var stdoutReader io.ReadCloser - err := gitcmd.NewCommand("diff-index", "--src-prefix=\\a/", "--dst-prefix=\\b/", "--cached", "-p", "HEAD"). - WithTimeout(30 * time.Second). + cmd := gitcmd.NewCommand("diff-index", "--src-prefix=\\a/", "--dst-prefix=\\b/", "--cached", "-p", "HEAD") + stdoutReader, stdoutReaderClose := cmd.MakeStdoutPipe() + defer stdoutReaderClose() + + err := cmd.WithTimeout(30 * time.Second). WithDir(t.basePath). - WithStdoutReader(&stdoutReader). WithPipelineFunc(func(ctx gitcmd.Context) error { var diffErr error diff, diffErr = gitdiff.ParsePatch(ctx, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdoutReader, "") diff --git a/services/repository/gitgraph/graph.go b/services/repository/gitgraph/graph.go index 565f67872e..6e114f901a 100644 --- a/services/repository/gitgraph/graph.go +++ b/services/repository/gitgraph/graph.go @@ -6,7 +6,6 @@ package gitgraph import ( "bufio" "bytes" - "io" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git/gitcmd" @@ -45,10 +44,10 @@ func GetCommitGraph(r *git.Repository, page, maxAllowedColors int, hidePRRefs bo commitsToSkip := setting.UI.GraphMaxCommitNum * (page - 1) - var stdoutReader io.ReadCloser + stdoutReader, stdoutReaderClose := graphCmd.MakeStdoutPipe() + defer stdoutReaderClose() if err := graphCmd. WithDir(r.Path). - WithStdoutReader(&stdoutReader). WithPipelineFunc(func(ctx gitcmd.Context) error { scanner := bufio.NewScanner(stdoutReader) parser := &Parser{} diff --git a/services/wiki/wiki.go b/services/wiki/wiki.go index f4115038cb..1f1e564006 100644 --- a/services/wiki/wiki.go +++ b/services/wiki/wiki.go @@ -170,7 +170,7 @@ func updateWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model // FIXME: The wiki doesn't have lfs support at present - if this changes need to check attributes here - objectHash, err := gitRepo.HashObject(strings.NewReader(content)) + objectHash, err := gitRepo.HashObjectBytes([]byte(content)) if err != nil { log.Error("HashObject failed: %v", err) return err diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index cc66c23d4a..c80f790425 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -405,16 +405,13 @@ func TestCantMergeUnrelated(t *testing.T) { err := gitcmd.NewCommand("read-tree", "--empty").WithDir(path).Run(t.Context()) assert.NoError(t, err) - stdin := strings.NewReader("Unrelated File") - var stdout strings.Builder - err = gitcmd.NewCommand("hash-object", "-w", "--stdin"). + stdout, _, err := gitcmd.NewCommand("hash-object", "-w", "--stdin"). WithDir(path). - WithStdin(stdin). - WithStdout(&stdout). - Run(t.Context()) + WithStdinBytes([]byte("Unrelated File")). + RunStdString(t.Context()) assert.NoError(t, err) - sha := strings.TrimSpace(stdout.String()) + sha := strings.TrimSpace(stdout) _, _, err = gitcmd.NewCommand("update-index", "--add", "--replace", "--cacheinfo"). AddDynamicArguments("100644", sha, "somewher-over-the-rainbow"). @@ -441,15 +438,13 @@ func TestCantMergeUnrelated(t *testing.T) { _, _ = messageBytes.WriteString("Unrelated") _, _ = messageBytes.WriteString("\n") - stdout.Reset() - err = gitcmd.NewCommand("commit-tree").AddDynamicArguments(treeSha). + stdout, _, err = gitcmd.NewCommand("commit-tree").AddDynamicArguments(treeSha). WithEnv(env). WithDir(path). - WithStdin(messageBytes). - WithStdout(&stdout). - Run(t.Context()) + WithStdinBytes(messageBytes.Bytes()). + RunStdString(t.Context()) assert.NoError(t, err) - commitSha := strings.TrimSpace(stdout.String()) + commitSha := strings.TrimSpace(stdout) _, _, err = gitcmd.NewCommand("branch", "unrelated"). AddDynamicArguments(commitSha).