interp: move KillTimeout to DefaultOpen

KillTimeout is specific to DefaultExec,
other exec implementations, like those written in go,
don't benefit from it (you can't kill a goroutine).

Also updated DefaultOpen for consistency.
This also makes Default* functions group under their
respective type in godoc.

Fixes #420.

interp/example_test.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"os"
 	"strings"
+	"time"
 
 	"mvdan.cc/sh/v3/expand"
 	"mvdan.cc/sh/v3/interp"
@@ -52,7 +53,7 @@ func ExampleExecModule() {
 			return interp.ExitStatus(1)
 		}
 
-		return interp.DefaultExec(ctx, args)
+		return interp.DefaultExec(2*time.Second)(ctx, args)
 	}
 	runner, _ := interp.New(
 		interp.StdIO(nil, os.Stdout, os.Stdout),

interp/interp.go

@@ -40,8 +40,8 @@ type RunnerOption func(*Runner) error
 func New(opts ...RunnerOption) (*Runner, error) {
 	r := &Runner{
 		usedNew:    true,
-		execModule: DefaultExec,
-		openModule: DefaultOpen,
+		execModule: DefaultExec(2 * time.Second),
+		openModule: DefaultOpen(),
 	}
 	r.dirStack = r.dirBootstrap[:0]
 	for _, opt := range opts {
@@ -406,19 +406,6 @@ type Runner struct {
 	// apply to the current shell, and not just the command.
 	keepRedirs bool
 
-	// KillTimeout holds how much time the interpreter will wait for a
-	// program to stop after being sent an interrupt signal, after
-	// which a kill signal will be sent. This process will happen when the
-	// interpreter's context is cancelled.
-	//
-	// The zero value will default to 2 seconds.
-	//
-	// A negative value means that a kill signal will be sent immediately.
-	//
-	// On Windows, the kill signal is always sent immediately,
-	// because Go doesn't currently support sending Interrupt on Windows.
-	KillTimeout time.Duration
-
 	// So that we can get io.Copy to reuse the same buffer within a runner.
 	// For example, this saves an allocation for every shell pipe, since
 	// io.PipeReader does not implement io.WriterTo.
@@ -517,10 +504,9 @@ func (r *Runner) Reset() {
 	}
 	// reset the internal state
 	*r = Runner{
-		Env:         r.Env,
-		execModule:  r.execModule,
-		openModule:  r.openModule,
-		KillTimeout: r.KillTimeout,
+		Env:        r.Env,
+		execModule: r.execModule,
+		openModule: r.openModule,
 
 		// These can be set by functions like Dir or Params, but
 		// builtins can overwrite them; reset the fields to whatever the
@@ -581,20 +567,16 @@ func (r *Runner) Reset() {
 	}
 
 	r.dirStack = append(r.dirStack, r.Dir)
-	if r.KillTimeout == 0 {
-		r.KillTimeout = 2 * time.Second
-	}
 	r.didReset = true
 	r.bufCopier.Reader = nil
 }
 
 func (r *Runner) modCtx(ctx context.Context) context.Context {
 	mc := ModuleCtx{
-		Dir:         r.Dir,
-		Stdin:       r.stdin,
-		Stdout:      r.stdout,
-		Stderr:      r.stderr,
-		KillTimeout: r.KillTimeout,
+		Dir:    r.Dir,
+		Stdin:  r.stdin,
+		Stdout: r.stdout,
+		Stderr: r.stderr,
 	}
 	oenv := overlayEnviron{
 		parent: r.Env,
@@ -744,18 +726,17 @@ func (r *Runner) sub() *Runner {
 	// Keep in sync with the Runner type. Manually copy fields, to not copy
 	// sensitive ones like errgroup.Group, and to do deep copies of slices.
 	r2 := &Runner{
-		Env:         r.Env,
-		Dir:         r.Dir,
-		Params:      r.Params,
-		Funcs:       r.Funcs,
-		execModule:  r.execModule,
-		openModule:  r.openModule,
-		stdin:       r.stdin,
-		stdout:      r.stdout,
-		stderr:      r.stderr,
-		KillTimeout: r.KillTimeout,
-		filename:    r.filename,
-		opts:        r.opts,
+		Env:        r.Env,
+		Dir:        r.Dir,
+		Params:     r.Params,
+		Funcs:      r.Funcs,
+		execModule: r.execModule,
+		openModule: r.openModule,
+		stdin:      r.stdin,
+		stdout:     r.stdout,
+		stderr:     r.stderr,
+		filename:   r.filename,
+		opts:       r.opts,
 	}
 	r2.Vars = make(map[string]expand.Variable, len(r.Vars))
 	for k, v := range r.Vars {

interp/interp_test.go

@@ -2670,15 +2670,15 @@ func testExecModule(ctx context.Context, args []string) error {
 		mc, _ := FromModuleContext(ctx)
 		return fn(mc, args[1:])
 	}
-	return DefaultExec(ctx, args)
+	return DefaultExec(2*time.Second)(ctx, args)
 }
 
 func testOpenModule(ctx context.Context, path string, flag int, perm os.FileMode) (io.ReadWriteCloser, error) {
 	if runtime.GOOS == "windows" && path == "/dev/null" {
 		path = "NUL"
 	}
 
-	return DefaultOpen(ctx, path, flag, perm)
+	return DefaultOpen()(ctx, path, flag, perm)
 }
 
 func TestRunnerRunConfirm(t *testing.T) {

interp/module.go

@@ -44,10 +44,6 @@ type ModuleCtx struct {
 	Stdout io.Writer
 	// Stderr is the interpreter's current standard error writer.
 	Stderr io.Writer
-
-	// KillTimeout is the duration configured by Runner.KillTimeout; refer
-	// to its docs for its purpose. It is needed to implement DefaultExec.
-	KillTimeout time.Duration
 }
 
 // ExecModuleFunc is the module responsible for executing a simple command. It is
@@ -59,65 +55,75 @@ type ModuleCtx struct {
 // interpreter will come to a stop.
 type ExecModuleFunc func(ctx context.Context, args []string) error
 
-func DefaultExec(ctx context.Context, args []string) error {
-	mc, _ := FromModuleContext(ctx)
-	path, err := LookPath(mc.Env, args[0])
-	if err != nil {
-		fmt.Fprintln(mc.Stderr, err)
-		return ExitStatus(127)
-	}
-	cmd := exec.Cmd{
-		Path:   path,
-		Args:   args,
-		Env:    execEnv(mc.Env),
-		Dir:    mc.Dir,
-		Stdin:  mc.Stdin,
-		Stdout: mc.Stdout,
-		Stderr: mc.Stderr,
-	}
+// DefaultExec returns an ExecModuleFunc used by default.
+// It finds binaries in PATH and executes them.
+// When context is cancelled, interrupt signal is sent to running processes.
+// KillTimeout is a duration to wait before sending kill signal.
+// A negative value means that a kill signal will be sent immediately.
+// On Windows, the kill signal is always sent immediately,
+// because Go doesn't currently support sending Interrupt on Windows.
+// Runner.New() sets killTimeout to 2 seconds by default.
+func DefaultExec(killTimeout time.Duration) ExecModuleFunc {
+	return func(ctx context.Context, args []string) error {
+		mc, _ := FromModuleContext(ctx)
+		path, err := LookPath(mc.Env, args[0])
+		if err != nil {
+			fmt.Fprintln(mc.Stderr, err)
+			return ExitStatus(127)
+		}
+		cmd := exec.Cmd{
+			Path:   path,
+			Args:   args,
+			Env:    execEnv(mc.Env),
+			Dir:    mc.Dir,
+			Stdin:  mc.Stdin,
+			Stdout: mc.Stdout,
+			Stderr: mc.Stderr,
+		}
 
-	err = cmd.Start()
-	if err == nil {
-		if done := ctx.Done(); done != nil {
-			go func() {
-				<-done
+		err = cmd.Start()
+		if err == nil {
+			if done := ctx.Done(); done != nil {
+				go func() {
+					<-done
 
-				if mc.KillTimeout <= 0 || runtime.GOOS == "windows" {
-					_ = cmd.Process.Signal(os.Kill)
-					return
-				}
+					if killTimeout <= 0 || runtime.GOOS == "windows" {
+						_ = cmd.Process.Signal(os.Kill)
+						return
+					}
 
-				// TODO: don't temporarily leak this goroutine
-				// if the program stops itself with the
-				// interrupt.
-				go func() {
-					time.Sleep(mc.KillTimeout)
-					_ = cmd.Process.Signal(os.Kill)
+					// TODO: don't temporarily leak this goroutine
+					// if the program stops itself with the
+					// interrupt.
+					go func() {
+						time.Sleep(killTimeout)
+						_ = cmd.Process.Signal(os.Kill)
+					}()
+					_ = cmd.Process.Signal(os.Interrupt)
 				}()
-				_ = cmd.Process.Signal(os.Interrupt)
-			}()
-		}
+			}
 
-		err = cmd.Wait()
-	}
+			err = cmd.Wait()
+		}
 
-	switch x := err.(type) {
-	case *exec.ExitError:
-		// started, but errored - default to 1 if OS
-		// doesn't have exit statuses
-		if status, ok := x.Sys().(syscall.WaitStatus); ok {
-			if status.Signaled() && ctx.Err() != nil {
-				return ctx.Err()
+		switch x := err.(type) {
+		case *exec.ExitError:
+			// started, but errored - default to 1 if OS
+			// doesn't have exit statuses
+			if status, ok := x.Sys().(syscall.WaitStatus); ok {
+				if status.Signaled() && ctx.Err() != nil {
+					return ctx.Err()
+				}
+				return ExitStatus(status.ExitStatus())
 			}
-			return ExitStatus(status.ExitStatus())
+			return ExitStatus(1)
+		case *exec.Error:
+			// did not start
+			fmt.Fprintf(mc.Stderr, "%v\n", err)
+			return ExitStatus(127)
+		default:
+			return err
 		}
-		return ExitStatus(1)
-	case *exec.Error:
-		// did not start
-		fmt.Fprintf(mc.Stderr, "%v\n", err)
-		return ExitStatus(127)
-	default:
-		return err
 	}
 }
 
@@ -268,10 +274,13 @@ func pathExts(env expand.Environ) []string {
 // interpreter will come to a stop.
 type OpenModuleFunc func(ctx context.Context, path string, flag int, perm os.FileMode) (io.ReadWriteCloser, error)
 
-func DefaultOpen(ctx context.Context, path string, flag int, perm os.FileMode) (io.ReadWriteCloser, error) {
-	mc, _ := FromModuleContext(ctx)
-	if !filepath.IsAbs(path) {
-		path = filepath.Join(mc.Dir, path)
+// DefaultOpen returns an OpenModuleFunc used by default. It uses os.OpenFile to open files.
+func DefaultOpen() OpenModuleFunc {
+	return func(ctx context.Context, path string, flag int, perm os.FileMode) (io.ReadWriteCloser, error) {
+		mc, _ := FromModuleContext(ctx)
+		if !filepath.IsAbs(path) {
+			path = filepath.Join(mc.Dir, path)
+		}
+		return os.OpenFile(path, flag, perm)
 	}
-	return os.OpenFile(path, flag, perm)
 }

interp/module_test.go

@@ -177,11 +177,13 @@ func TestKillTimeout(t *testing.T) {
 				var rbuf readyBuffer
 				rbuf.seenReady.Add(1)
 				ctx, cancel := context.WithCancel(context.Background())
-				r, err := New(StdIO(nil, &rbuf, &rbuf))
+				r, err := New(
+					StdIO(nil, &rbuf, &rbuf),
+					ExecModule(DefaultExec(test.killTimeout)),
+				)
 				if err != nil {
 					t.Fatal(err)
 				}
-				r.KillTimeout = test.killTimeout
 				go func() {
 					rbuf.seenReady.Wait()
 					cancel()