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.

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

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

Merged
merged 1 commit into from
Jun 25, 2017
Merged

Conversation

stnutt
Copy link
Contributor

@stnutt stnutt commented Jun 20, 2017

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
Copy link
Owner

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
Copy link
Contributor Author

stnutt commented Jun 21, 2017

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
Copy link
Contributor Author

stnutt commented Jun 21, 2017

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
Copy link
Owner

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
Copy link
Contributor Author

stnutt commented Jun 24, 2017

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 astahlman merged commit fc0020e into astahlman:master Jun 25, 2017
@astahlman
Copy link
Owner

Merged. Thanks for the change!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Execute asynchronous processes on remote host
2 participants