Skip to content

Commit

Permalink
execext.RunCommand: fix: do not pass a cancellable context to mvdan.c…
Browse files Browse the repository at this point in the history
…c/sh

We used to pass to mvdan.cc/sh/interp.Runner a context that was cancelled on
reception of a OS signal. This caused the Runner to terminate the subprocess
abruptly.

The correct behavior instead is for us to completely ignore the signal and let
the subprocess deal with it. If the subprocess doesn't handle the signal, it
will be terminated. If the subprocess does handle the signal, it knows better
than us wether it wants to cleanup and terminate or do something different.

So now we pass an empty context just to make the API of interp.Runner happy

Fixes go-task/task/#458
  • Loading branch information
marco-m authored and ghostsquad committed May 14, 2022
1 parent bffb6e1 commit 22dfc1e
Showing 1 changed file with 10 additions and 1 deletion.
11 changes: 10 additions & 1 deletion internal/execext/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,16 @@ func RunCommand(ctx context.Context, opts *RunCommandOptions) error {
if err != nil {
return err
}
return r.Run(ctx, p)

// We used to pass to interp.Runner a context that was cancelled on reception of a
// OS signal. This caused the Runner to terminate the subprocess abruptly.
// The correct behavior instead is for us to completely ignore the signal and let
// the subprocess deal with it. If the subprocess doesn't handle the signal, it will
// be terminated. If the subprocess does handle the signal, it knows better than us
// wether it wants to cleanup and terminate or do something different.
// See https://github.com/go-task/task/issues/458 for details.
// So now we pass an empty context just to make the API of interp.Runner happy
return r.Run(context.Background(), p)
}

// IsExitError returns true the given error is an exis status error
Expand Down

0 comments on commit 22dfc1e

Please sign in to comment.