[enhancement] make process exit status available to user

[grrtrr]

Thank you for this useful package, I have been looking for something like this for a long time;
having used tcl/expect in the past, the other go-based packages were not what I expected (pun
not intended).

Problem

The expect package provides a Kill function, which can be used to stop a process after interacting with it through the pty.

For the use case of a normally terminating process (e.g. running an ssh process with password prompt, which then executes a single remote command), there is no way of querying the exit status.

This information can be very useful to evaluate, in addition to the command-stdout/stderr buffer.

Requested enhancement

Expose, in some form, the return value (*os.ProcessState, error) of cmd.Process.Wait.

[leemcloughlin]

I've added Result to Expect which is filled in asynchronously with the Wait() result after the command exits

For an example of its use see the func showWaitResult() in expect_test.go

[grrtrr]

Thank you.

With apologies, I think what I wrote above was misleading. Initially I thought all that is needed is to somehow capture the result, but did not realize my suggestion had these drawbacks:

• leaving the process to run in its own goroutine makes it hard to synchronize,
• in particular when the timing of when the process finishes is important or needs to be controlled.

I am facing this use case:

• we run processes with scripted interaction in their goroutines,
• the calling goroutines are to block until the child processes finish,
• at that time the return status is of interest.

Would you be open to a solution that exposes exp.cmd.Process, to allow the caller to take full control over the running command, i.e. execute the expectReaper() from its (calling) goroutine?

This would make it much simpler. I am not a fan of forking things just because 1 feature is missing, this is something I need to make work in the application.

[grrtrr]

I have added a proof-of-concept in my current branch, could you please have a look?

I was thinking you did not like to expose internals, so I have made it a separate exp.Wait() call.
This makes it possible to run all the ProcessState functions defined in os, such as Success(), Sys() (can be use to extract the exit code via syscall), SystemTime() and the like.

There were a few other changes: I made the package context.Context aware, since this is now officially supported in Go >= 1.7, i.e. the os/exec package will terminate the process via SIGKILL if the context is canceled.

I hope we can collaborate on this, if necessary I can open a separate issue.

[leemcloughlin]

Hi

I'd just pushed my own changes before seeing this.

I've added NewExpectProc that returns the Cmd to allow the caller to take control of it.

I'm looking at your version now but I'm not that familiar with Contexts so it may take a while for me to understand it

[grrtrr]

I will have a more detailed look after work. Likely we can put this on hold until these things have been sorted out, at which time I can send more clearly separated patches. At the moment I am working on integrating the expect package into the use case I am facing (interfacing with python/ansible scripts).

I have found context very useful, in particular when using many concurrent goroutines. It is essentially just a wrapper around a chan struct{}, with some utility functions added.

When properly integrated, it allows to cancel all (nested) goroutines on program or call termination, from a single point. One area I have found this very useful in is to set up a signal handler for a program, which handles a signal by canceling the context.

[grrtrr]

I had a look at 39b84f8, liked the progress. The recent 2 changes are somewhat complementary,

• one fills in the Result asynchronously,
• the other (exposing exec.Cmd) allows to Wait(), in this case the command exit status is only indirectly available, by doing a typecast of the result (error) to exec.ExitError.

I found that there is a happy combination of both approaches - by simply using exp.cmd.Process.Wait() in place of exp.cmd.Wait().

Perhaps we can return to this later, I am facing another problem, will document that separately.

[grrtrr]

@leemcloughlin - over a year has passed since this issue was opened.

When I just reconsolidated my fork at https://github.com/grrtrr/expect/commits/master, I saw that there are a few commits that to me seem very useful.

The most important one of them is to add a cancellation context, which the above referred to. Without the ability to
cancel / put a timeout, things can run ad infinitum, i.e. "hang".

Since I needed those features, I have put them into my forked master branch. Would they make sense for you -- I would be happy to work on a new PR?

[leemcloughlin]

Hi Gerrit Sorry I've been so late on investigating your commits - just been so swamped since I changed jobs. It wont be till the end of the month before I can properly reply. I know that contexts are now required and expect needs them, and thanks for your work on adding them, but I really dont like contexts. They sort of infect func's and spread... sigh Best regards Lee McLoughlin CTO LMMR Technologies <http://uk.linkedin.com/pub/lee-mcloughlin/1/973/481>

…

On 6 November 2017 at 21:40, Gerrit Renker ***@***.***> wrote: @leemcloughlin <https://github.com/leemcloughlin> - over a year has passed since this issue was opened. When I just reconsolidated my fork at https://github.com/grrtrr/ expect/commits/master, I saw that there are a few commits that to me seem very useful. The most important one of them is to add a cancellation context <https://blog.golang.org/context>, which the above referred to. Since I needed those features, I have put them into my forked master branch. Would they make sense for you -- I would be happy to work on a new PR? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKxT74_4d5QK0_QlH1ok0AaUoV0-bVVyks5sz3zcgaJpZM4KkRBN> .

[grrtrr]

Lee,

please do take your time - a few weeks more or less will not matter.

When it comes to context.Context, I meant cancellation context, and please allow me a few words on that; since after 2 years of Go programming experience it has proven so useful.

People have abused the untyped key/value interface of the context package to pass values around, and
unsurprisingly this has led to complex bugs. That is not what I was suggesting.

The problem that context solves becomes apparent when really using the concurrency support that
the language provides: Go does not have a pthread_cancel.

Once an autonomous goroutine is started, the only way to stop it is to pass a "channel" in that tells it when to stop. Doing this ad-hoc, in hundreds of functions and goroutines, leads to clutter.

With the context package, all that is required is to pass the ctx as first argument; instead of having to allocate/close/maintain chan variables, which is very repetitive and tedious.

Several low-level packages (os/exec, net/http, net) come with built-in support for a cancellation context.

For concurrent batch-processing, I have found sync/errgroup to be very helpful. When it is used with a context (errgroup.WithContext(parentCtx)), it cancels all other running requests, should one in the herd fail to complete.