Hi Michael, We have a problem understanding how TRAMP deals with finding remote shell. See below.
If you have any insight, it would be appreciated. Best, Ihor Morgan Smith <[email protected]> writes: > So let me preface this email with the fact that I really don't > understand tramp. It seems very complicated and hard to debug. While > I've tried to read and understand tramp, my solutions mostly come from > trail and error. > > Since we're talking about tramp now I might as well include the other > tramp patch I have locally as patch #6. It refactors the ob-shell logic > to fix a test failure I was having with > "ob-shell/remote-with-stdin-or-cmdline". > > The branch for dealing with "stdin" gets the shell path by doing > "(with-connection-local-variables shell-file-name)". According to the info > manual (info-lookup-symbol 'shell-file-name 'emacs-lisp-mode) this will be > "/bin/sh" on remote systems. > > The branch for dealing with "shebang" gets the shell path by calling > `org-babel--get-shell-file-name'. Guix patches that function to point > to the right shell when they package org-mode. Although looking at the > logic now I can see that guix is very much in the wrong for doing that. > Oh dear. I'm going to have go tell them to stop doing that. > > Regardless I think the refactor might be good? I'm really not good with tramp > so I'm not sure. Feel free to skip this patch if you don't like it. We can > come back to it at a later date. > Subject: [PATCH 1/6] Testing: TRAMP integration: Make work in guix build > environment > > * testing/org-test.el (org-test-with-tramp-remote-dir--worker): Set > `tramp-encoding-shell' and `tramp-remote-path' so that tramp is able > to find a shell. > Set `org-babel-remote-temporary-directory' and `tramp-tmpdir' to > `temporary-file-directory' as the current tests assume these are all > equal. > --- > testing/org-test.el | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/testing/org-test.el b/testing/org-test.el > index ea2c0c8fb..fa295775f 100644 > --- a/testing/org-test.el > +++ b/testing/org-test.el > @@ -291,8 +291,22 @@ org-test-with-tramp-remote-dir--worker > (t (require 'tramp) > (defvar tramp-methods) > (defvar tramp-default-host-alist) > - (let ((tramp-methods > - (cons '("mock" > + (defvar tramp-remote-path) > + (defvar tramp-encoding-shell) > + (defvar org-babel-remote-temporary-directory) > + (let (;; In the guix build environment /bin/sh doesn't exist > + ;; so these two configurations are necessary. > + (tramp-encoding-shell "sh") > + (tramp-remote-path (cons 'tramp-own-remote-path > + tramp-remote-path)) > + ;; FIXME: If `temporary-file-directory' is not "/tmp/" > + ;; then we have to tell both org-babel and tramp for our > + ;; tests to pass. org-babel should probably be able to > + ;; figure this out without any configuration. > + (org-babel-remote-temporary-directory temporary-file-directory) > + (tramp-methods > + (cons `("mock" > + (tramp-tmpdir ,temporary-file-directory) > (tramp-login-program "sh") > (tramp-login-args (("-i"))) > (tramp-remote-shell "/bin/sh") > ... > Subject: [PATCH 6/6] ob-shell.el: Refactor > > * lisp/ob-shell.el (org-babel-sh-evaluate): Combine the cases for a > shell script together. > --- > lisp/ob-shell.el | 54 +++++++++++++----------------------------------- > 1 file changed, 14 insertions(+), 40 deletions(-) > > diff --git a/lisp/ob-shell.el b/lisp/ob-shell.el > index e3767ac06..c3e6027c0 100644 > --- a/lisp/ob-shell.el > +++ b/lisp/ob-shell.el > @@ -338,6 +338,7 @@ org-babel-sh-evaluate > of the statements in BODY, if RESULT-TYPE equals `value' then > return the value of the last statement in BODY." > (let* ((shebang (cdr (assq :shebang params))) > + (shebang (and (org-string-nw-p shebang) shebang)) > (async (org-babel-comint-use-async params)) > (results-params (cdr (assq :result-params params))) > (value-is-exit-status > @@ -347,34 +348,24 @@ org-babel-sh-evaluate > (member "value" results-params))) > (results > (cond > - ((or stdin cmdline) ; external shell script w/STDIN > - (let ((script-file (org-babel-temp-file "sh-script-")) > - (stdin-file (org-babel-temp-file "sh-stdin-")) > - (padline (not (string= "no" (cdr (assq :padline params)))))) > - (with-temp-file script-file > - (if shebang > + ((or stdin cmdline shebang) ; external shell script > + (let ((script-file (org-babel-temp-file "sh-script-")) > + (padline (not (string= "no" (cdr (assq :padline params))))) > + (stdin (or stdin ""))) > + (with-temp-file script-file > + (if shebang > (insert shebang "\n") > ;; Provide shell name explicitly. > ;; This is necessary because running, for example, > ;; dash script-for-dash.sh will use /bin/sh. > (insert (format "#!/usr/bin/env %s" shell-file-name) "\n")) > - (when padline (insert "\n")) > - (insert body)) > - (set-file-modes script-file #o755) > - (with-temp-file stdin-file (insert (or stdin ""))) > - (with-temp-buffer > - (with-connection-local-variables > - ;; `with-connection-local-variables' will override > - ;; `shell-file-name' and `shell-command-swtich' as > - ;; needed for the remote connection. > - (apply #'process-file > - shell-file-name > - stdin-file > - (current-buffer) > - nil > - (list shell-command-switch > - (concat (file-local-name script-file) " " > (format "%s" cmdline))))) > - (buffer-string)))) > + (when padline (insert "\n")) > + (insert body)) > + (set-file-modes script-file #o755) > + (org-babel-eval > + (concat (file-local-name script-file) > + (if cmdline (format " %s" cmdline) "")) > + stdin))) > (session ; session evaluation > (if async > (progn > @@ -407,23 +398,6 @@ org-babel-sh-evaluate > ;; Remove `org-babel-sh-eoe-indicator' output line. > 1)) > "\n"))) > - ;; External shell script, with or without a predefined > - ;; shebang. > - ((org-string-nw-p shebang) > - (let ((script-file (org-babel-temp-file "sh-script-")) > - (padline (not (equal "no" (cdr (assq :padline params)))))) > - (with-temp-file script-file > - (insert shebang "\n") > - (when padline (insert "\n")) > - (insert body)) > - (set-file-modes script-file #o755) > - (if (file-remote-p script-file) > - ;; Run remote script using its local path as COMMAND. > - ;; The remote execution is ensured by setting > - ;; correct `default-directory'. > - (let ((default-directory (file-name-directory > script-file))) > - (org-babel-eval (file-local-name script-file) "")) > - (org-babel-eval script-file "")))) > (t (org-babel-eval shell-file-name (org-trim body)))))) > (when (and results value-is-exit-status) > (setq results (car (reverse (split-string results "\n" t))))) > -- > 2.54.0 > -- Ihor Radchenko // yantar92, Org mode maintainer, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92>
