emacs-aio icon indicating copy to clipboard operation
emacs-aio copied to clipboard

Usage with shell command

Open rougier opened this issue 4 years ago • 21 comments

I'm trying to use aio to run a shell command but my call seems to be blocking (cannot move cursor until stop is written). I imagine I'm doing something wrong but I'm not quite sure where:

(defun aio-shell (command)
  (let ((promise (aio-promise)))
    (prog1 promise
      (aio-resolve promise (lambda () (shell-command-to-string command))))))

(aio-defun aio-run (command)
  (interactive)
  (message "Start") (aio-await (aio-shell command)) (message "Stop"))

(aio-run "sleep 3")

Is that the right syntax?

rougier avatar Nov 17 '20 11:11 rougier

I'm guessing that won't work because shell-command-to-string boils down to call-process which is synchronous. Then again, I'm far from an expert here. Anyway, I'd suggest building a process-calling function that's async all the way down.

alphapapa avatar Nov 17 '20 11:11 alphapapa

Thanks. Would you have a pointer to a simple example that I can take inspiration from?

rougier avatar Nov 17 '20 11:11 rougier

From this email in the Emacs mailing list, I found something that could be useful. I rewrote it the following way in order to be sure no conflict would happen between several launches of async-shell-command-to-string.

(defun async-shell-command-to-string (command callback)
  (with-temp-buffer
    (lexical-let ((output-buffer (current-buffer))
                  (callback-fun callback))
      (set-process-sentinel (shell-command command output-buffer)
                            (lambda (process signal)
                              (when (memq (process-status process) '(exit signal))
                                (with-current-buffer output-buffer
                                  (funcall callback-fun (buffer-string)))
                                (kill-buffer output-buffer))))
      output-buffer)))

And it works like a charm, with this as a quick but simple test:

(progn
  (async-shell-command-to-string "sleep 3; echo World"
                                 (lambda (output) (message output)))
  (async-shell-command-to-string "echo Hello"
                                 (lambda (output) (message output))))

I would love to see an equivalent that would actually return the string and not the temporary buffer once the shell command is executed instead on relying on a callback function though. I also don’t know if getting rid of buffer-substring-no-properties from the original code was a good idea or not.

Phundrak avatar Nov 17 '20 14:11 Phundrak

Hm, well:

  1. That doesn't use aio anymore. :)
  2. You shouldn't use lexical-let but rather set lexical-binding in the buffer.

alphapapa avatar Nov 17 '20 14:11 alphapapa

  1. That doesn't use aio anymore. :)

The core issue seemed to be with shell-command-to-string not being async. Now maybe we can wrap this definition of async-shell-command-to-string in an aio function that would be able to make use of the value passed in the callback function? Right now, I don’t know how though.

  1. You shouldn't use lexical-let but rather set lexical-binding in the buffer.

True, I kept it from the original implementation since I was testing things in the *scratch* buffer, it should be a simple let instead. Thanks for noticing that!

Phundrak avatar Nov 17 '20 14:11 Phundrak

@Phundrak Thanks! I tested if on emacs 27.1 by evaluating your code in the scratch buffer but it is blocking and after 3 seconds I got an error let: Wrong type argument: processp, 1 (note that I replaced lexical-let with let and I added lexical-binding).

rougier avatar Nov 17 '20 14:11 rougier

Hi, @rougier maybe you could use async-shell-command and then populate a string with the output buffer once the promise is resolved?

sam217pa avatar Nov 17 '20 14:11 sam217pa

Yep, just found this answer on stack overflow.

rougier avatar Nov 17 '20 15:11 rougier

You must call aio-resolve in a callback, and the function you pass to aio-resolve must simply return the result (or signal an error), not have any side effects since it's potentially called more than once. If the action you want to run asynchronously doesn't support a callback, then it's probably not asynchronous, which is the case here.

Here's one way to write this:

(defun aio-call-process (program buffer &rest args)
  (let ((process (apply #'start-process program buffer program args))
        (promise (aio-promise)))
    (prog1 promise
      (setf (process-sentinel process)
            (lambda (_ status) (aio-resolve promise (lambda () status)))))))

An asynchronous function can aio-await this to receive the sentinel status directly, without any explicit callbacks and without blocking Emacs.

skeeto avatar Nov 18 '20 01:11 skeeto

Thanks! Based on your code, I tried to adapt my below without much success:

(defun aio-call-process (program buffer &rest args)
  (let ((process (apply #'start-process program buffer program args))
        (promise (aio-promise)))
    (prog1 promise
      (setf (process-sentinel process)
            (lambda (_ status) (aio-resolve promise (lambda () status)))))))

(aio-defun aio-run (command &rest args)
  (interactive)
  (message "Start")
  (aio-await (aio-call-process command (current-buffer) args))
  (message "Stop"))

(aio-run "sleep" 3)

rougier avatar Nov 18 '20 06:11 rougier

Thanks! Based on your code, I tried to adapt my below without much success:

What exactly happens?

alphapapa avatar Nov 18 '20 08:11 alphapapa

This is the log in *Messages* (that I get without any delay):

Start
#s(aio-promise (closure ((cps-state-atom-100 closure #2 nil ...) (cps-state-atom-99 closure #2 nil ...) (cps-state-iter-yield-98 closure #2 nil ...) (cps-state-let*-97 closure #2 nil ...) (cps-state-atom-96 closure #2 nil ...) (cps-binding-cps-argument-94-95) (cps-state-atom-93 closure #2 nil ...) (cps-state-let*-92 closure #2 nil ...) (cps-state-atom-91 closure #2 nil ...) (cps-binding-cps-binding-result-89-90) (cps-binding-result-89) (cps-state-atom-88 closure #2 nil ...) ...) nil (signal (car cps-binding-condition-case-error-86) (cdr cps-binding-condition-case-error-86))) nil)

rougier avatar Nov 18 '20 09:11 rougier

What are you expecting to happen?

alphapapa avatar Nov 18 '20 10:11 alphapapa

Start in the log, then non blocking pause of 3 second and then Stop in the log. Did I miss something obvious ? In the meantime, I managed to implement what I wanted but the code is ugly

rougier avatar Nov 18 '20 10:11 rougier

Start in the log, then non blocking pause of 3 second and then Stop in the log. Did I miss something obvious ?

The output you showed doesn't indicate whether there was a non-blocking, 3-second pause, and it doesn't say Stop at all. So I didn't understand what was going on. I'm still not sure that I do understand, for that reason. :)

In the meantime, I managed to implement what I wanted but the code is ugly

That code looks nice to me. :shrug:

alphapapa avatar Nov 18 '20 10:11 alphapapa

Oh wait, I'm totally wrong here. Since the call is non-blocking, I should have start/stop immediately in the log and then a background sleep without any output.

rougier avatar Nov 18 '20 10:11 rougier

There are three issues here. First you're not seeing any errors since you're running the code asynchronously, and there's nobody to receive the error signal. When you're debugging use aio-wait-for at the top-level, which is like aio-await but blocks the thread until the promise is resolved, receiving any errors.

My aio-call-process doesn't take a list of arguments, so if you want to pass a list you must use apply. Using aio-wait-for reveals this:

(aio-wait-for (aio-run "sleep" 3))
;; error: Wrong type argument: stringp, (3)

Correcting for this:

(aio-defun aio-run (command &rest args)
  (interactive)
  (message "Start")
  (aio-await (apply #'aio-call-process command (current-buffer) args))
  (message "Stop"))

Though there's still another issue: process arguments must be strings:

(aio-wait-for (aio-run "sleep" 3))
;; error: Wrong type argument: stringp, 3

So finally this achieves the desired effect, and runs it all in the background:

(aio-run "sleep" "3")

skeeto avatar Nov 18 '20 12:11 skeeto

Thanks, Chris, even though this isn't my code, your explanations are very helpful for my understanding of how aio works.

Do you think this aio-call-process function would be a good addition to the library?

alphapapa avatar Nov 18 '20 13:11 alphapapa

Thanks for you explanation, now it makes sense. I support the idea of having an aio-call-process.

rougier avatar Nov 18 '20 15:11 rougier

would be a good addition to the library?

Yeah, it would! Revisiting this now, I was surprised I hadn't added one before. I think my intention was for users to build their own when they need it using aio-make-callback rather than add wrappers around Emacs' many process creation functions, forwarding all their options through a new aio-based API.

Note that my example aio-call-process is incomplete. Resolution should happen when the status is "finished\n" (yeah, I hate this Emacs API), not just when the sentinel is called.

skeeto avatar Nov 18 '20 18:11 skeeto

I think I like the idea of having a simple aio-call-process to solve the easy cases and then encourage aio-make-callback for the more complex cases. Though, I am still curious how aio-make-callback works or improves interacting with processes or other callback-based tasks.

justinbarclay avatar Jul 19 '23 16:07 justinbarclay