Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding kill built-in command in order to support job control. #52

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Adding kill built-in command in order to support job control. #52

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions internal/engine/engine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package engine

import (
"fmt"
"os"
"path/filepath"
"strconv"
Expand All @@ -13,12 +14,14 @@ import (
"github.com/michaelmacinnis/oh/internal/common/interface/boolean"
"github.com/michaelmacinnis/oh/internal/common/interface/cell"
"github.com/michaelmacinnis/oh/internal/common/interface/integer"
"github.com/michaelmacinnis/oh/internal/common/interface/literal"
"github.com/michaelmacinnis/oh/internal/common/interface/scope"
"github.com/michaelmacinnis/oh/internal/common/struct/frame"
"github.com/michaelmacinnis/oh/internal/common/type/env"
"github.com/michaelmacinnis/oh/internal/common/type/list"
"github.com/michaelmacinnis/oh/internal/common/type/num"
"github.com/michaelmacinnis/oh/internal/common/type/obj"
"github.com/michaelmacinnis/oh/internal/common/type/pair"
"github.com/michaelmacinnis/oh/internal/common/type/pipe"
"github.com/michaelmacinnis/oh/internal/common/type/status"
"github.com/michaelmacinnis/oh/internal/common/type/str"
Expand Down Expand Up @@ -220,6 +223,7 @@ func init() { //nolint:gochecknoinits
scope0.Define("bg", &task.Method{Op: task.Action(bg)})
scope0.Define("fg", &task.Method{Op: task.Action(fg)})
scope0.Define("jobs", &task.Method{Op: task.Action(jobs)})
scope0.Define("kill", &task.Method{Op: task.Action(kill)})

task.Actions(scope0)

Expand All @@ -233,6 +237,50 @@ func jobs(t *task.T) task.Op {
return t.Return(status.Int(0))
}

func kill(t *task.T) task.Op {
Copy link
Owner

@michaelmacinnis michaelmacinnis Jul 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My initial thought was to make this a bit easier to write by exporting a few actions from internal/engine/task/action.go:

var (
    ExecMethod = Action(execMethod)
    External   = Action(external)
    Resume     = Action(resume)
)

so that kill could look something like this:

func kill(t *task.T) task.Op {
    if t.Code() == pair.Null {
        panic("expected at least 1 argument, passed 0")
    }

    args := []cell.I{}
    groups := []cell.I{}

    for a := t.Code(); a != pair.Null; a = pair.Cdr(a) {
        c := pair.Car(a)

        s := common.String(c)
        if strings.HasPrefix(s, "%") {
            n, err := strconv.Atoi(s[1:])
            if err != nil {
                panic("job does not exist")
            }

            group := job.GetGroup(n)
            if group == 0 {
                panic("job does not exist")
            }

            groups = append(groups, str.New("-" + strconv.Itoa(group)))
        } else {
            args = append(args, c)
        }
    }
    
    t.ReplaceOp(task.Resume)
    t.PushOp(task.External)

    t.PushResult(nil)
    t.PushResult(str.New("kill"))

    for _, c := range args {
        t.PushResult(c)
    }

    if len(groups) > 0 {
        t.PushResult(str.New("--"))

        for _, c := range groups {
            t.PushResult(c)
        }
    }

    return t.PushOp(task.ExecMethod)
}

and then update bg and fg to also support the percent syntax (by just stripping the % from arguments passed to those commands).

But the bigger problem is that jobs don't work the same way in oh as they do in other shells. In oh, the list of jobs is a list of currently suspended tasks that can be resumed. Oh's bg command removes a command from the list of jobs and returns a reference to that command that can be passed to wait. The bg command behaves like the spawn command. (Note that & in oh is rewritten as spawn).

To see what I mean try executing:

define t: spawn {
    sleep 60; echo done
}

jobs
# Should output nothing.

wait $t

The effect is the same as doing:

sleep 60; echo done

# Press Ctrl-Z to suspend the sleep.

define t: bg 1

jobs
# Should output nothing.

wait $t

Oh also avoids launching processes so not every job is part of a new process group (as they are part of the same process group as the shell - indicated with a group of 0) and as such the only signals that really make sense for those jobs are ones that terminate the job.

Because every job in oh's job list is suspended , we have to do a kill %n and then a fg n or bg n to continue the job which will then act on the signal sent by kill.

If we want a convenience wrapper to terminate a job we have suspended we should add a command called terminate (or something similar) that takes the same kind of arguments as wait so that we can do:

terminate (bg n)

jobId := false
signal := false
args := make([]cell.I, 0)

for a := t.Code(); a != pair.Null; a = pair.Cdr(a) {
v := literal.String(pair.Car(a))
if v[0:1] == "%" {
id, err := strconv.Atoi(v[1:])
if err != nil {
panic("job does not exist")
}

pgid := job.Getpgid(id)
if pgid == 0 {
panic("job does not exist")
}

args = append(args, str.New(fmt.Sprintf("-%d", pgid)))
jobId = true
} else {
if v[0:1] == "-" {
signal = true
}
args = append(args, pair.Car(a))
}
}

v := make([]cell.I, 0)
v = append(v, str.New("kill"))
if jobId && !signal {
v = append(v, str.New("--"))
}
v = append(v, args...)

c := list.New(v...)

j := job.New(0)
Evaluate(j, c)
process.RestoreForegroundGroup()

return t.Return(status.Int(0))
}

func success(c cell.I) (exitcode int) {
defer func() {
recover() //nolint:errcheck
Expand Down
16 changes: 16 additions & 0 deletions internal/system/job/job_unix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,22 @@ func Jobs(w io.Writer) {
<-r
}

func Getpgid(n int) int {
r := make(chan int)

requestq <- func() {
job, ok := jobs[n]
if ok {
r <- job.group
} else {
r <- 0
}
close(r)
}

return <-r
}

// Monitor launches the goroutine responsible for monitoring jobs/tasks.
func Monitor() {
signals := []os.Signal{unix.SIGCHLD}
Expand Down