Unable to get prior interp.ExecHandler behaviour with ExecHandlers

[dtrudg]

While looking to update our project to the latest release of the excellent sh, I've tried to replace the use of the deprecated ExecHandler with ExecHandlers per the changes from #964

We have a custom exec handler function execHandler that disables execution for everything except a __stop__ literal string which we hook some stuff onto.

	execHandler := func(ctx context.Context, args []string) error {
		if args[0] == stopBuiltin {
			env = GetEnv(interp.HandlerCtx(ctx))
			return nil
		}
		c := strings.Join(args, " ")
		return fmt.Errorf("could not execute %q: execution is disabled", c)
	}

Previously we were setting this as the sole ExecHandler for an interpreter like so:

opts := []interp.RunnerOption{
		interp.ExecHandler(execHandler),
<snip>

With sh v3.7.0 I'm attempting to use ExecHandlers instead:

opts := []interp.RunnerOption{
	interp.ExecHandlers(
			func(next interp.ExecHandlerFunc) interp.ExecHandlerFunc {
				return execHandler
			},
		),
<snip>

My understanding is that this should work? We have exactly one custom handler in ExecHandlers and we never call next.

However, this change seems to lead instead to the default exec handler being run, instead of our execHandler. I see output like:

"__stop__": executable file not found in $PATH

I'm going to dig further here... but wondering if I'm short-sightedly completely missing something obvious about #964 .... and shouldn't be expecting the above change to preserve behaviour?

(ref.. sylabs/singularity@2c8c20b#diff-ff1889935425f7e87f989c893c9a2922b783f7933d871ce096dfc066d7f50380)

[mvdan]

What you say makes sense - that's how the new API is meant to work. However, note that your handler falls back to defaultExecHandler, which will actually try to execute a command. So at least the code you have doesn't seem to agree with "disable execution for everything except __stop__".

[dtrudg]

Apologies for the noise. I'll look into this further on our end. Thanks again.