From b877504b03a43dc5ed19109f1a00f08a343ebaba Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 31 Mar 2022 19:56:22 +0800 Subject: Refactor `git.Command.Run*`, introduce `RunWithContextString` and `RunWithContextBytes` (#19266) This follows * https://github.com/go-gitea/gitea/issues/18553 Introduce `RunWithContextString` and `RunWithContextBytes` to help the refactoring. Add related unit tests. They keep the same behavior to save stderr into err.Error() as `RunInXxx` before. Remove `RunInDirTimeoutPipeline` `RunInDirTimeoutFullPipeline` `RunInDirTimeout` `RunInDirTimeoutEnv` `RunInDirPipeline` `RunInDirFullPipeline` `RunTimeout`, `RunInDirTimeoutEnvPipeline`, `RunInDirTimeoutEnvFullPipeline`, `RunInDirTimeoutEnvFullPipelineFunc`. Then remaining `RunInDir` `RunInDirBytes` `RunInDirWithEnv` can be easily refactored in next PR with a simple search & replace: * before: `stdout, err := RunInDir(path)` * next: `stdout, _, err := RunWithContextString(&git.RunContext{Dir:path})` Other changes: 1. When `timeout <= 0`, use default. Because `timeout==0` is meaningless and could cause bugs. And now many functions becomes more simple, eg: `GitGcRepos` 9 lines to 1 line. `Fsck` 6 lines to 1 line. 2. Only set defaultCommandExecutionTimeout when the option `setting.Git.Timeout.Default > 0` --- modules/process/manager.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'modules/process') diff --git a/modules/process/manager.go b/modules/process/manager.go index 50dbbbe6c8..26dd6d535f 100644 --- a/modules/process/manager.go +++ b/modules/process/manager.go @@ -86,6 +86,10 @@ func (pm *Manager) AddContext(parent context.Context, description string) (ctx c // Most processes will not need to use the cancel function but there will be cases whereby you want to cancel the process but not immediately remove it from the // process table. func (pm *Manager) AddContextTimeout(parent context.Context, timeout time.Duration, description string) (ctx context.Context, cancel context.CancelFunc, finshed FinishedFunc) { + if timeout <= 0 { + // it's meaningless to use timeout <= 0, and it must be a bug! so we must panic here to tell developers to make the timeout correct + panic("the timeout must be greater than zero, otherwise the context will be cancelled immediately") + } ctx, cancel = context.WithTimeout(parent, timeout) ctx, pid, finshed := pm.Add(ctx, description, cancel) @@ -239,7 +243,7 @@ func (pm *Manager) ExecDirEnv(ctx context.Context, timeout time.Duration, dir, d // Returns its complete stdout and stderr // outputs and an error, if any (including timeout) func (pm *Manager) ExecDirEnvStdIn(ctx context.Context, timeout time.Duration, dir, desc string, env []string, stdIn io.Reader, cmdName string, args ...string) (string, string, error) { - if timeout == -1 { + if timeout <= 0 { timeout = 60 * time.Second } -- cgit v1.2.3