Repository: mvdan/sh
Pull request #666 (2021-02-16 17:16:06)

interp: implement trap of ERR and EXIT signals
(see commit message)


Patch:
diff --git a/interp/api.go b/interp/api.go
index 92a1ee08..e17baed3 100644
--- a/interp/api.go
+++ b/interp/api.go
@@ -102,8 +102,9 @@ type Runner struct {
 	// track if a sourced script set positional parameters
 	sourceSetParams bool
 
-	err       error // current shell exit code or fatal error
-	exitShell bool  // whether the shell needs to exit
+	err          error // current shell exit code or fatal error
+	handlingTrap bool  // whether we're currently in a trap callback
+	shellExited  bool  // whether the shell needs to exit
 
 	// The current and last exit status code. They can only be different if
 	// the interpreter is in the middle of running a statement. In that
@@ -133,6 +134,10 @@ type Runner struct {
 	// keepRedirs is used so that "exec" can make any redirections
 	// apply to the current shell, and not just the command.
 	keepRedirs bool
+
+	// Fake signal callbacks
+	callbackErr  string
+	callbackExit string
 }
 
 type alias struct {
@@ -474,24 +479,30 @@ func IsExitStatus(err error) (status uint8, ok bool) {
 }
 
 // Run interprets a node, which can be a *File, *Stmt, or Command. If a non-nil
-// error is returned, it will typically contain commands exit status,
-// which can be retrieved with IsExitStatus.
+// error is returned, it will typically contain a command's exit status, which
+// can be retrieved with IsExitStatus.
 //
 // Run can be called multiple times synchronously to interpret programs
 // incrementally. To reuse a Runner without keeping the internal shell state,
 // call Reset.
+//
+// Calling Run on an entire *File implies an exit, meaning that an exit trap may
+// run.
 func (r *Runner) Run(ctx context.Context, node syntax.Node) error {
 	if !r.didReset {
 		r.Reset()
 	}
 	r.fillExpandConfig(ctx)
 	r.err = nil
-	r.exitShell = false
+	r.shellExited = false
 	r.filename = ""
 	switch x := node.(type) {
 	case *syntax.File:
 		r.filename = x.Name
 		r.stmts(ctx, x.Stmts)
+		if !r.shellExited {
+			r.exitShell(ctx, r.exit)
+		}
 	case *syntax.Stmt:
 		r.stmt(ctx, x)
 	case syntax.Command:
@@ -517,7 +528,7 @@ func (r *Runner) Run(ctx context.Context, node syntax.Node) error {
 // Note that this state is overwritten at every Run call, so it should be
 // checked immediately after each Run call.
 func (r *Runner) Exited() bool {
-	return r.exitShell
+	return r.shellExited
 }
 
 // Subshell makes a copy of the given Runner, suitable for use concurrently
diff --git a/interp/builtin.go b/interp/builtin.go
index 61b48366..dcbe26ba 100644
--- a/interp/builtin.go
+++ b/interp/builtin.go
@@ -51,21 +51,23 @@ func (r *Runner) builtinCode(ctx context.Context, pos syntax.Pos, name string, a
 	case "false":
 		return 1
 	case "exit":
-		r.exitShell = true
+		exit := 0
 		switch len(args) {
 		case 0:
-			return r.lastExit
+			exit = r.lastExit
 		case 1:
 			n, err := strconv.Atoi(args[0])
 			if err != nil {
 				r.errf("invalid exit status code: %q\n", args[0])
 				return 2
 			}
-			return n
+			exit = n
 		default:
 			r.errf("exit cannot take multiple arguments\n")
 			return 1
 		}
+		r.exitShell(ctx, exit)
+		return exit
 	case "set":
 		if err := Params(args...)(r); err != nil {
 			r.errf("set: %v\n", err)
@@ -385,7 +387,7 @@ func (r *Runner) builtinCode(ctx context.Context, pos syntax.Pos, name string, a
 			r.keepRedirs = true
 			break
 		}
-		r.exitShell = true
+		r.exitShell(ctx, 1)
 		r.exec(ctx, args)
 		return r.exit
 	case "command":
@@ -702,8 +704,56 @@ func (r *Runner) builtinCode(ctx context.Context, pos syntax.Pos, name string, a
 			delete(r.alias, name)
 		}
 
+	case "trap":
+		fp := flagParser{remaining: args}
+		callback := "-"
+		for fp.more() {
+			switch flag := fp.flag(); flag {
+			case "-l", "-p":
+				r.errf("trap: %q: NOT IMPLEMENTED flag\n", flag)
+				return 2
+			case "-":
+				// default signal
+			default:
+				r.errf("trap: %q: invalid option\n", flag)
+				r.errf("trap: usage: trap [-lp] [[arg] signal_spec ...]\n")
+				return 2
+			}
+		}
+		args := fp.args()
+		switch len(args) {
+		case 0:
+			// Print non-default signals
+			if r.callbackExit != "" {
+				r.outf("trap -- %q EXIT\n", r.callbackExit)
+			}
+			if r.callbackErr != "" {
+				r.outf("trap -- %q ERR\n", r.callbackErr)
+			}
+		case 1:
+			// assume it's a signal, the default will be restored
+		default:
+			callback = args[0]
+			args = args[1:]
+		}
+		// For now, treat both empty and - the same since ERR and EXIT have no
+		// default callback.
+		if callback == "-" {
+			callback = ""
+		}
+		for _, arg := range args {
+			switch arg {
+			case "ERR":
+				r.callbackErr = callback
+			case "EXIT":
+				r.callbackExit = callback
+			default:
+				r.errf("trap: %s: invalid signal specification\n", arg)
+				return 2
+			}
+		}
 	default:
-		// "trap", "umask", "fg", "bg",
+		// "umask", "fg", "bg",
 		panic(fmt.Sprintf("unhandled builtin: %s", name))
 	}
 	return 0
diff --git a/interp/runner.go b/interp/runner.go
index 7b91711f..d4052be1 100644
--- a/interp/runner.go
+++ b/interp/runner.go
@@ -136,8 +136,7 @@ func (r *Runner) updateExpandOpts() {
 func (r *Runner) expandErr(err error) {
 	if err != nil {
 		r.errf("%v\n", err)
-		r.exit = 1
-		r.exitShell = true
+		r.exitShell(context.TODO(), 1)
 	}
 }
 
@@ -221,7 +220,7 @@ func (r *Runner) errf(format string, a ...interface{}) {
 }
 
 func (r *Runner) stop(ctx context.Context) bool {
-	if r.err != nil || r.exitShell {
+	if r.err != nil || r.Exited() {
 		return true
 	}
 	if err := ctx.Err(); err != nil {
@@ -278,7 +277,9 @@ func (r *Runner) stmtSync(ctx context.Context, st *syntax.Stmt) {
 		//   conditions (if <cond>, while <cond>, etc)
 		//   part of && or || lists
 		//   preceded by !
-		r.exitShell = true
+		r.exitShell(ctx, r.exit)
+	} else if r.exit != 0 {
+		r.trapCallback(ctx, r.callbackErr, "error")
 	}
 	if !r.keepRedirs {
 		r.stdin, r.stdout, r.stderr = oldIn, oldOut, oldErr
@@ -556,6 +557,40 @@ func (r *Runner) cmd(ctx context.Context, cm syntax.Command) {
 	}
 }
 
+func (r *Runner) trapCallback(ctx context.Context, callback, name string) {
+	if callback == "" {
+		return // nothing to do
+	}
+	if r.handlingTrap {
+		return // don't recurse, as that could lead to cycles
+	}
+	r.handlingTrap = true
+
+	p := syntax.NewParser()
+	// TODO: do this parsing when "trap" is called?
+	file, err := p.Parse(strings.NewReader(callback), name+" trap")
+	if err != nil {
+		r.errf(name+"trap: %v\n", err)
+		// ignore errors in the callback
+		return
+	}
+	r.stmts(ctx, file.Stmts)
+
+	r.handlingTrap = false
+}
+
+// setExit call this function to exit the shell with status
+func (r *Runner) exitShell(ctx context.Context, status int) {
+	if status != 0 {
+		r.trapCallback(ctx, r.callbackErr, "error")
+	}
+	r.trapCallback(ctx, r.callbackExit, "exit")
+
+	r.shellExited = true
+	// Restore the original exit status. We ignore the callbacks.
+	r.exit = status
+}
+
 func (r *Runner) flattenAssign(as *syntax.Assign) []*syntax.Assign {
 	// Convert "declare $x" into "declare value".
 	// Don't use syntax.Parser here, as we only want the basic


Test patch:
diff --git a/interp/interp_test.go b/interp/interp_test.go
index f73585ff..4f20d115 100644
--- a/interp/interp_test.go
+++ b/interp/interp_test.go
@@ -1956,6 +1956,16 @@ set +o pipefail
 	{"type -t $PATH_PROG", "file\n"},
 	{"type -t inexisting_dfgsdgfds", "exit status 1"},
 
+	// trap
+	{"trap 'echo at_exit' EXIT; true", "at_exit\n"},
+	{"trap 'echo on_err' ERR; false; echo FAIL", "on_err\nFAIL\n"},
+	{"trap 'echo at_exit' EXIT; trap - EXIT; echo OK", "OK\n"},
+	{"set -e; trap 'echo A' ERR EXIT; false; echo FAIL", "A\nA\nexit status 1"},
+	{"trap 'foobar' UNKNOWN", "trap: UNKNOWN: invalid signal specification\nexit status 2 #JUSTERR"},
+	// TODO: our builtin appears to not receive the piped bytes?
+	// {"trap 'echo on_err' ERR; trap | grep -q '.*echo on_err.*'", "trap -- \"echo on_err\" ERR\n"},
+	{"trap 'false' ERR EXIT; false", "exit status 1"},
+
 	// eval
 	{"eval", ""},
 	{"eval ''", ""},
@@ -2918,6 +2928,8 @@ var testBuiltinsMap = map[string]func(HandlerContext, []string) error{
 			} else if arg == "-E" {
 			} else if rx == nil {
 				rx = regexp.MustCompile(arg)
+			} else {
+				return fmt.Errorf("unexpected arg: %q", arg)
 			}
 		}
 		lines, err := readLines(hc)