#shell interpreter builtins

1 messages · Page 1 of 1 (latest)

fallen rampart
#

@misty echo, one last question 🙏 Is it possible to call a builtin from an exec handler? Would be helpful for example to have a Dagger .wait command add some convenience around the interpreter's builtin wait.

misty echo
#

this is currently not possible; builtins kick in before the exec handler:

    if isBuiltin(name) {
        r.exit = r.builtinCode(ctx, pos, name, args[1:])
        return
    }
    r.exec(ctx, args)

have you looked into the call handler? https://pkg.go.dev/mvdan.cc/sh/v3/interp#CallHandler

#

it might be enough for you but it depends on what you mean by "add some convenience".

fallen rampart
# misty echo this is currently not possible; builtins kick in before the exec handler: ``` ...

I was wondering if it was possible to add support for it. Including making isBuiltin() public since right now we’ve duplicated it on our side, but not maintainable.

have you looked into the call handler? https://pkg.go.dev/mvdan.cc/sh/v3/interp#CallHandler

Yeah, and we use it, but it’s only as an alias so we can expose some builtins and hide others, along with a help page (e.g., .help .wait). Most of our CallHandler is working around interpreter builtins running first. For context, basically we hide interpreter builtins behind a _ prefix in order to allow having a Dagger function named “echo” for example. It also allows us to only expose a subset through our own builtins prefix (.).

What I’m after here though is a way to call builtins from within an exec handler. I don’t mean that’s the only thing the exec handler does, I mean an exec handler that does other things and at some point may relay some logic to an interpreter builtin.

fallen rampart
misty echo
#

gotcha. I think exposing the list of builtins, and exposing the builtins themselves, is reasonable - I just wanted to check you were aware of CallHandler first.

I'll need to think about what the right API is here, because I'm not sure that simply exposing them as an ExecHandler API is the right thing to do.

misty echo
#

I don't think the ExecHandler API will work for builtins, because many of them are closely tied to the internal Runner state, such as cd, set, alias, etc.

So it will have to be something a bit bespoke. More tomorrow.

misty echo
#

I've exported interp.IsBuiltin now, FYI. I'm still working through an API to expose the builtins themselves.

fallen rampart
misty echo
#

trial and error, currently.

misty echo
#

after a bit more refactoring, and three failed attempts, here's where I landed: https://github.com/mvdan/sh/commit/6b98b83b673469da62330f807fd989eb434acb96

what do you think? see the added tests.

I initially added Runner.Builtin, but quickly realised it sounded like it could be called from anywhere, which is not the intent. I think anyone wanting to run a builtin from the top level API should do it the usual way, via Runner.Run.

hanging it off of HandlerContext is a bit weird but I think it works. I added a hidden Runner field there so that you don't have to keep and pass the runner pointer yourself.

#

I did not push to master because I wanted to run it past you before people it lands. It wouldn't be stable at master yet, but I also want to avoid experimenting in master too much.

fallen rampart
misty echo
#

đź‘Ť I will polish this up and push to master this evening

misty echo
#

I have some time this weekend, so let me know if you need any other adjustments or investigation

fallen rampart