search

astahlman / ob-async

Asynchronous src_block execution for org-babel
343 stars 32 forks source link

Inherit :dir header arg in async function for remote execution #9

Closed stnutt closed 7 years ago

stnutt commented 7 years ago

This fixes #8. The default directory wasn't being set (from the :dir header arg) inside of the async function, so that meant you couldn't execute blocks asynchronously on remote hosts or using sudo.

astahlman commented 7 years ago

Ahhh, I see. Good catch, and thanks for the PR!

The change looks good, but can you make it in ob-async.org and tangle it to ob-async.el? The source is here: https://github.com/astahlman/ob-async/blob/master/ob-async.org#definition. Sorry for the runaround.

I was trying to think of a good way to write an acceptance test for this, but I think the problem only surfaces when using TRAMP, right? Unfortunately I can't think of anything that doesn't involve actually making some kind of remote connection.

stnutt commented 7 years ago

Sure, I'll modify the Org file instead and tangle it. As far as writing a test, I've written one that just sets :dir to / and echos the pwd, but that passes without my change since default-directory is already set around the call to async-start, so it's tramp or nothing. Unfortunately, I'm not aware of a tramp method that could be used in a testing situation.

stnutt commented 7 years ago

Actually, I just potentially thought of a test. If the :dir argument is /sudo:user@localhost:/ (where user is user-login-name), then it shouldn't require any kind of setup or password, just that sudo is installed (not sure if it's automatically installed in the Travis CI environment, but it should be on most systems that have /bin/sh). The block can then otuput the SUDO_USER and PWD variables and the test can verify them.

Here's the kind of block I'm talking about:

#+BEGIN_SRC sh :async :dir /sudo:user@localhost:/
  echo $SUDO_USER $PWD
#+END_SRC

Does this seem like a good test to you?

astahlman commented 7 years ago

Ah, very clever! Just tested with $USER, and confirmed that the following works. Yeah, I think that would be a great test.

#+BEGIN_SRC sh :async :dir /sudo:$USER@localhost:/
  echo "$SUDO_USER $PWD"
#+END_SRC
stnutt commented 7 years ago

My commit is updated with the test case and with all changes made in the org file and also tangled. I had to change the travis config to have sudo installed, and the CI is passing with the new test case.

astahlman commented 7 years ago

Merged. Thanks for the change!