feat: deprecate options timeout in favor of command options

CommandOptions now can take a context and a timeout. Deprecate current
options timeout property in favor of that.

The current behavior is not affected and RunWithTimeout methods should
be removed in the future.

Use *Command.WithTimeout and *Command.SetTimeout instead.

Signed-off-by: Ayman Bagabas <[email protected]>

Author: aymanbagabas

command.go

@@ -17,10 +17,11 @@ import (
 
 // Command contains the name, arguments and environment variables of a command.
 type Command struct {
-	name string
-	args []string
-	envs []string
-	ctx  context.Context
+	name    string
+	args    []string
+	envs    []string
+	timeout time.Duration
+	ctx     context.Context
 }
 
 // CommandOptions contains options for running a command.
@@ -69,17 +70,29 @@ func (c *Command) AddEnvs(envs ...string) *Command {
 	return c
 }
 
-// WithContext sets the context for the command.
-func (c *Command) WithContext(ctx context.Context) *Command {
+// WithContext returns a new Command with the given context.
+func (c Command) WithContext(ctx context.Context) *Command {
 	c.ctx = ctx
-	return c
+	return &c
+}
+
+// WithTimeout returns a new Command with given timeout.
+func (c Command) WithTimeout(timeout time.Duration) *Command {
+	c.timeout = timeout
+	return &c
+}
+
+// SetTimeout sets the timeout for the command.
+func (c *Command) SetTimeout(timeout time.Duration) {
+	c.timeout = timeout
 }
 
 // AddOptions adds options to the command.
 // Note: only the last option will take effect if there are duplicated options.
 func (c *Command) AddOptions(opts ...CommandOptions) *Command {
 	for _, opt := range opts {
-		c = c.WithContext(opt.Context)
+		c.timeout = opt.Timeout
+		c.ctx = opt.Context
 		c.AddArgs(opt.Args...)
 		c.AddEnvs(opt.Envs...)
 	}
@@ -93,7 +106,7 @@ func (c *Command) AddCommitter(committer *Signature) *Command {
 }
 
 // DefaultTimeout is the default timeout duration for all commands.
-const DefaultTimeout = time.Minute
+const DefaultTimeout = time.Second
 
 // A limitDualWriter writes to W but limits the amount of data written to just N
 // bytes. On the other hand, it passes everything to w.
@@ -132,6 +145,8 @@ type RunInDirOptions struct {
 	// Stderr is the error output from the command.
 	Stderr io.Writer
 	// Timeout is the duration to wait before timing out.
+	//
+	// Deprecated: Use CommandOptions.Timeout or *Command.WithTimeout instead.
 	Timeout time.Duration
 }
 
@@ -145,10 +160,15 @@ func (c *Command) RunInDirWithOptions(dir string, opts ...RunInDirOptions) (err
 	if len(opts) > 0 {
 		opt = opts[0]
 	}
-	if opt.Timeout < 0 {
-		opt.Timeout = -1
-	} else if opt.Timeout == 0 {
-		opt.Timeout = DefaultTimeout
+
+	timeout := c.timeout
+	// TODO: remove this in newer version
+	if opt.Timeout > 0 {
+		timeout = opt.Timeout
+	}
+
+	if timeout == 0 {
+		timeout = DefaultTimeout
 	}
 
 	buf := new(bytes.Buffer)
@@ -164,9 +184,9 @@ func (c *Command) RunInDirWithOptions(dir string, opts ...RunInDirOptions) (err
 
 	defer func() {
 		if len(dir) == 0 {
-			log("[timeout: %v] %s\n%s", opt.Timeout, c, buf.Bytes())
+			log("[timeout: %v] %s\n%s", timeout, c, buf.Bytes())
 		} else {
-			log("[timeout: %v] %s: %s\n%s", opt.Timeout, dir, c, buf.Bytes())
+			log("[timeout: %v] %s: %s\n%s", timeout, dir, c, buf.Bytes())
 		}
 	}()
 
@@ -175,9 +195,9 @@ func (c *Command) RunInDirWithOptions(dir string, opts ...RunInDirOptions) (err
 		ctx = c.ctx
 	}
 
-	if opt.Timeout > 0 {
+	if timeout > 0 {
 		var cancel context.CancelFunc
-		ctx, cancel = context.WithTimeout(ctx, opt.Timeout)
+		ctx, cancel = context.WithTimeout(ctx, timeout)
 		defer func() {
 			cancel()
 			if err == context.DeadlineExceeded {
@@ -219,55 +239,72 @@ func (c *Command) RunInDirWithOptions(dir string, opts ...RunInDirOptions) (err
 
 }
 
+// RunInDirPipeline executes the command in given directory and default timeout
+// duration. It pipes stdout and stderr to supplied io.Writer.
+func (c *Command) RunInDirPipeline(stdout, stderr io.Writer, dir string) error {
+	return c.RunInDirWithOptions(dir, RunInDirOptions{
+		Stdin:  nil,
+		Stdout: stdout,
+		Stderr: stderr,
+	})
+}
+
 // RunInDirPipelineWithTimeout executes the command in given directory and
 // timeout duration. It pipes stdout and stderr to supplied io.Writer.
 // DefaultTimeout will be used if the timeout duration is less than
 // time.Nanosecond (i.e. less than or equal to 0). It returns an ErrExecTimeout
 // if the execution was timed out.
+//
+// Deprecated: Use RunInDirPipeline and CommandOptions instead.
+// TODO: remove this in the next major version
 func (c *Command) RunInDirPipelineWithTimeout(timeout time.Duration, stdout, stderr io.Writer, dir string) (err error) {
-	return c.RunInDirWithOptions(dir, RunInDirOptions{
-		Stdin:   nil,
-		Stdout:  stdout,
-		Stderr:  stderr,
-		Timeout: timeout,
-	})
-}
-
-// RunInDirPipeline executes the command in given directory and default timeout
-// duration. It pipes stdout and stderr to supplied io.Writer.
-func (c *Command) RunInDirPipeline(stdout, stderr io.Writer, dir string) error {
-	return c.RunInDirPipelineWithTimeout(DefaultTimeout, stdout, stderr, dir)
+	if timeout != 0 {
+		c = c.WithTimeout(timeout)
+	}
+	return c.RunInDirPipeline(stdout, stderr, dir)
 }
 
 // RunInDirWithTimeout executes the command in given directory and timeout
 // duration. It returns stdout in []byte and error (combined with stderr).
+//
+// Deprecated: Use RunInDir and CommandOptions instead.
+// TODO: remove this in the next major version
 func (c *Command) RunInDirWithTimeout(timeout time.Duration, dir string) ([]byte, error) {
-	stdout := new(bytes.Buffer)
-	stderr := new(bytes.Buffer)
-	if err := c.RunInDirPipelineWithTimeout(timeout, stdout, stderr, dir); err != nil {
-		return nil, concatenateError(err, stderr.String())
+	if timeout != 0 {
+		c = c.WithTimeout(timeout)
 	}
-	return stdout.Bytes(), nil
+	return c.RunInDir(dir)
 }
 
 // RunInDir executes the command in given directory and default timeout
 // duration. It returns stdout and error (combined with stderr).
 func (c *Command) RunInDir(dir string) ([]byte, error) {
-	return c.RunInDirWithTimeout(DefaultTimeout, dir)
+	stdout := new(bytes.Buffer)
+	stderr := new(bytes.Buffer)
+	if err := c.RunInDirPipeline(stdout, stderr, dir); err != nil {
+		return nil, concatenateError(err, stderr.String())
+	}
+	return stdout.Bytes(), nil
 }
 
 // RunWithTimeout executes the command in working directory and given timeout
 // duration. It returns stdout in string and error (combined with stderr).
+//
+// Deprecated: Use RunInDir and CommandOptions instead.
+// TODO: remove this in the next major version
 func (c *Command) RunWithTimeout(timeout time.Duration) ([]byte, error) {
-	stdout, err := c.RunInDirWithTimeout(timeout, "")
-	if err != nil {
-		return nil, err
+	if timeout != 0 {
+		c = c.WithTimeout(timeout)
 	}
-	return stdout, nil
+	return c.Run()
 }
 
 // Run executes the command in working directory and default timeout duration.
 // It returns stdout in string and error (combined with stderr).
 func (c *Command) Run() ([]byte, error) {
-	return c.RunWithTimeout(DefaultTimeout)
+	stdout, err := c.RunInDir("")
+	if err != nil {
+		return nil, err
+	}
+	return stdout, nil
 }

command_test.go

@@ -60,6 +60,6 @@ func TestCommand_AddEnvs(t *testing.T) {
 }
 
 func TestCommand_RunWithTimeout(t *testing.T) {
-	_, err := NewCommand("version").RunWithTimeout(time.Nanosecond)
+	_, err := NewCommand("version").WithTimeout(time.Nanosecond).Run()
 	assert.Equal(t, ErrExecTimeout, err)
 }

repo.go

@@ -60,6 +60,8 @@ type InitOptions struct {
 	Bare bool
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions
@@ -119,6 +121,8 @@ type CloneOptions struct {
 	Depth uint64
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions
@@ -165,6 +169,8 @@ type FetchOptions struct {
 	Prune bool
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions
@@ -200,6 +206,8 @@ type PullOptions struct {
 	Branch string
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions
@@ -236,6 +244,8 @@ func (r *Repository) Pull(opts ...PullOptions) error {
 type PushOptions struct {
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions
@@ -272,6 +282,8 @@ type CheckoutOptions struct {
 	BaseBranch string
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions
@@ -315,6 +327,8 @@ type ResetOptions struct {
 	Hard bool
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions
@@ -353,6 +367,8 @@ func (r *Repository) Reset(rev string, opts ...ResetOptions) error {
 type MoveOptions struct {
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions
@@ -391,6 +407,8 @@ type AddOptions struct {
 	Pathspecs []string
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions
@@ -433,6 +451,8 @@ type CommitOptions struct {
 	Author *Signature
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions
@@ -488,6 +508,8 @@ type NameStatus struct {
 type ShowNameStatusOptions struct {
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions
@@ -554,6 +576,8 @@ func (r *Repository) ShowNameStatus(rev string, opts ...ShowNameStatusOptions) (
 type RevParseOptions struct {
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions
@@ -598,6 +622,8 @@ type CountObject struct {
 type CountObjectsOptions struct {
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions
@@ -663,6 +689,8 @@ func (r *Repository) CountObjects(opts ...CountObjectsOptions) (*CountObject, er
 type FsckOptions struct {
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions

repo_blame.go

@@ -14,6 +14,8 @@ import (
 type BlameOptions struct {
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions

repo_blob.go

@@ -12,6 +12,8 @@ import "time"
 type CatFileBlobOptions struct {
 	// The timeout duration before giving up for each shell command execution.
 	// The default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions

repo_commit.go

@@ -72,6 +72,8 @@ loop:
 type CatFileCommitOptions struct {
 	// The timeout duration before giving up for each shell command execution.
 	// The default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions

repo_diff.go

@@ -20,6 +20,8 @@ type DiffOptions struct {
 	Base string
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions

repo_grep.go

@@ -27,6 +27,8 @@ type GrepOptions struct {
 	ExtendedRegexp bool
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions

repo_pull.go

@@ -15,6 +15,8 @@ import (
 type MergeBaseOptions struct {
 	// The timeout duration before giving up for each shell command execution. The
 	// default timeout duration will be used when not supplied.
+	//
+	// Deprecated: Use CommandOptions.Timeout instead.
 	Timeout time.Duration
 	// The additional options to be passed to the underlying git.
 	CommandOptions