From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp11.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id ICxFLTDbFGMziAAAbAwnHQ (envelope-from ) for ; Sun, 04 Sep 2022 19:06:56 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp11.migadu.com with LMTPS id OBBJLTDbFGNFSAAA9RJhRA (envelope-from ) for ; Sun, 04 Sep 2022 19:06:56 +0200 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 4839294FE for ; Sun, 4 Sep 2022 19:06:56 +0200 (CEST) Received: from localhost ([::1]:42594 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oUt5D-0001QS-1m for larch@yhetil.org; Sun, 04 Sep 2022 13:06:55 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:43742) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oUt3X-0001OI-88 for emacs-orgmode@gnu.org; Sun, 04 Sep 2022 13:05:11 -0400 Received: from smtp6-g21.free.fr ([2a01:e0c:1:1599::15]:52238) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oUt3U-00014s-6J for emacs-orgmode@gnu.org; Sun, 04 Sep 2022 13:05:10 -0400 Received: from tosh-laptop (unknown [IPv6:2a01:e0a:505:3460:1a16:a0c4:3f89:c0d9]) by smtp6-g21.free.fr (Postfix) with ESMTPS id 86FC378050B; Sun, 4 Sep 2022 19:05:01 +0200 (CEST) Received: by tosh-laptop (sSMTP sendmail emulation); Sun, 04 Sep 2022 19:08:29 +0200 From: Bruno Barbier To: Ihor Radchenko Cc: Felix Freeman , emacs-orgmode@gnu.org Subject: Re: [BUG] ob-shell: cmdline and stdin broken when used with TRAMP In-Reply-To: <87h71nxyej.fsf@localhost> References: <87k097a6on.fsf@localhost> <87h71nxyej.fsf@localhost> Date: Sun, 04 Sep 2022 19:08:28 +0200 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Received-SPF: softfail client-ip=2a01:e0c:1:1599::15; envelope-from=brubar.cs@gmail.com; helo=smtp6-g21.free.fr X-Spam_score_int: 12 X-Spam_score: 1.2 X-Spam_bar: + X-Spam_report: (1.2 / 5.0 requ) BAYES_00=-1.9, DKIM_ADSP_CUSTOM_MED=0.001, FORGED_GMAIL_RCVD=1, FREEMAIL_FROM=0.001, MISSING_MID=0.497, NML_ADSP_CUSTOM_MED=0.9, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_SOFTFAIL=0.665, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=no autolearn_force=no X-Spam_action: no action Message-Id: X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: "Emacs-orgmode" X-Migadu-Flow: FLOW_IN X-Migadu-To: larch@yhetil.org X-Migadu-Country: US ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1662311216; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post; bh=0KMJwGYahDUU/AMj9qn7y+HYnJ9t7egBIecncd8eecc=; b=XfV/eeGIpSSp0hH4a4xKgNTehSRorp3Gl+oSsbaS7yb5EwTXMkdOIxJ+/XX48+rL98pl/6 CdFRihYFo30HmveANVd5ULPp6lArTPeMg0ICDJSvcm7qOaHF0Dppy9YEfPXnJJxj2n67oQ +TKNG362NOOIT3qif/Wici4Mowi8254QP+RxIlaE4QQ/qIJ8QHMc84KOVdgh85gF6MAFrQ BQZwRQHkqR1diOXZsbg/1cKUOveeXa4RgasmUqBA3AhBOxavYg8qLMPDhn2k5lhHz0Z6wk vDcouh1DnWvkVEkOGfmzDURzD9J/tnapnmTEXrSfCDsBlBlVsp+I9v+NBh6Y3w== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1662311216; a=rsa-sha256; cv=none; b=srYONjyuDOEQ4qX/9pmIy0AUE22k5YDVYx81nR6XCLeeKj29hoZPY5UQudrW+jxDwP/mEb tTpwfSNWhm6UA+xXXB64EOpTu2nCUmBQn/hNLxXGYmtyqV284qhq7TAsjIinx/vkMS+6Ic wU70JDfe5ZewCN0Am4ZlkS76ukpawJ1olGcLYlS98YMM0Rd+zfLUeCDp3vayn71EwOESjN O1UvDN7VYyDe6IV/KbtvsopbDJyLQwk3REvg6DUYq/Dw772pw2+v1+loQnwsWfFawHyPGf GjDPypKH39ExFeXpxlXGkkpkEXtnQsNnW4Ekq54OZVyJhoBMV9uFR50HwkXVpw== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=gmail.com (policy=none); spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org" X-Migadu-Spam-Score: 2.52 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=gmail.com (policy=none); spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org" X-Migadu-Queue-Id: 4839294FE X-Spam-Score: 2.52 X-Migadu-Scanner: scn1.migadu.com X-TUID: 4wKqmw7Vfn9P --=-=-= Content-Type: text/plain Ihor Radchenko writes: > > '->` :: `process-file' > Sorry: fixed. > > lib/ob-shell.el (org-babel-sh-evaluate): Use `process-file' (instead > > lisp/ob-shell.el Thanks. > >> + (with-connection-local-variables > > I tried to test your patch on various Emacs versions. > `with-connection-local-variables' is not yet available in Emacs 26. > Emacs 26 has `with-connection-local-profiles' though I am not sure if it > is a full equivalent. If not, you may need to create a compatibility > wrapper for Emacs 26. Indeed. It's been renamed/changed in Emacs 27. https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=21f54feee8c83e2c5fd8eeb6741cbd479a7b19eb I'm now adding a version of `with-connection-local-variables', in org-compat.el: that should work for 26. I didn't test it as I currently don't have an Emacs 26. But I should be able to compile one, if you think it's needed. Thank you for spotting this and pointing me to `with-connection-local-profiles'. Thank you again, Ihor, for the review and the testing. Let me know if you see further improvements, Bruno --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-ob-shell-Use-process-file-when-stdin-or-cmdline.patch Content-Description: patch >From 9ed2f6d9e2c8bd7dd219e98ff7983fa53fd0686f Mon Sep 17 00:00:00 2001 From: Bruno BARBIER Date: Sat, 18 Jun 2022 09:48:01 +0200 Subject: [PATCH] ob-shell: Use `process-file' when stdin or cmdline lisp/ob-shell.el (org-babel-sh-evaluate): Use `process-file' (instead of `call-process-shell-command') so that `org-babel-sh-evaluate' will invoke file name handlers based on `default-directory', if needed, like when using a remote directory. lisp/org-compat.el (with-connection-local-variables): New compatibility macro. testing/lisp/test-ob-shell.el (ob-shell/remote-with-stdin-or-cmdline): New test. testing/org-test.el (org-test-with-tramp-remote-dir): New macro. Fixes https://list.orgmode.org/CKMOBWBK709F.1RUN69SRWB64U@laptop/. --- lisp/ob-shell.el | 16 +++++++----- lisp/org-compat.el | 10 +++++++ testing/lisp/test-ob-shell.el | 49 +++++++++++++++++++++++++++++++++++ testing/org-test.el | 29 +++++++++++++++++++++ 4 files changed, 98 insertions(+), 6 deletions(-) diff --git a/lisp/ob-shell.el b/lisp/ob-shell.el index 44efb4ea1..51071d40a 100644 --- a/lisp/ob-shell.el +++ b/lisp/ob-shell.el @@ -279,12 +279,16 @@ (defun org-babel-sh-evaluate (session body &optional params stdin cmdline) (set-file-modes script-file #o755) (with-temp-file stdin-file (insert (or stdin ""))) (with-temp-buffer - (call-process-shell-command - (concat (if shebang script-file - (format "%s %s" shell-file-name script-file)) - (and cmdline (concat " " cmdline))) - stdin-file - (current-buffer)) + (with-connection-local-variables + (apply #'process-file + (if shebang (file-local-name script-file) + shell-file-name) + stdin-file + (current-buffer) + nil + (if shebang (when cmdline (list cmdline)) + (list shell-command-switch + (concat (file-local-name script-file) " " cmdline))))) (buffer-string)))) (session ; session evaluation (mapconcat diff --git a/lisp/org-compat.el b/lisp/org-compat.el index 91972ef9c..501e64bdb 100644 --- a/lisp/org-compat.el +++ b/lisp/org-compat.el @@ -221,6 +221,16 @@ (if (fboundp 'string-distance) (define-obsolete-function-alias 'org-babel-edit-distance 'org-string-distance "9.5") +(unless (fboundp 'with-connection-local-variables) + ;; Added in Emacs 27: commit:21f54feee8, 2019-03-09. + ;; Redefining it using the old function `with-connection-local-profiles'. + (defmacro with-connection-local-variables (&rest body) + "Apply connection-local variables according to `default-directory'. +Execute BODY, and unwind connection-local variables." + (declare (debug t)) + `(with-connection-local-profiles (connection-local-get-profiles) + ,@body))) + ;;; Emacs < 26.1 compatibility diff --git a/testing/lisp/test-ob-shell.el b/testing/lisp/test-ob-shell.el index 2f346f699..442e70372 100644 --- a/testing/lisp/test-ob-shell.el +++ b/testing/lisp/test-ob-shell.el @@ -106,6 +106,55 @@ (ert-deftest ob-shell/simple-list () "#+BEGIN_SRC sh :results output :var l='(1 2)\necho ${l}\n#+END_SRC" (org-trim (org-babel-execute-src-block)))))) +(ert-deftest ob-shell/remote-with-stdin-or-cmdline () + "Test :stdin and :cmdline with a remote directory." + ;; We assume 'default-directory' is a local directory. + (skip-unless (not (memq system-type '(ms-dos windows-nt)))) + (org-test-with-tramp-remote-dir remote-dir + (dolist (spec `( () + (:dir ,remote-dir) + (:dir ,remote-dir :cmdline t) + (:dir ,remote-dir :stdin t) + (:dir ,remote-dir :cmdline t :shebang t) + (:dir ,remote-dir :stdin t :shebang t) + (:dir ,remote-dir :cmdline t :stdin t :shebang t) + (:cmdline t) + (:stdin t) + (:cmdline t :shebang t) + (:stdin t :shebang t) + (:cmdline t :stdin t :shebang t))) + (let ((default-directory (or (plist-get spec :dir) default-directory)) + (org-confirm-babel-evaluate nil) + (params-line "") + (who-line " export who=tramp") + (args-line " echo ARGS: --verbose 23 71")) + (when-let ((dir (plist-get spec :dir))) + (setq params-line (concat params-line " " ":dir " dir))) + (when (plist-get spec :stdin) + (setq who-line " read -r who") + (setq params-line (concat params-line " :stdin input"))) + (when (plist-get spec :cmdline) + (setq args-line " echo \"ARGS: $*\"") + (setq params-line (concat params-line " :cmdline \"--verbose 23 71\""))) + (when (plist-get spec :shebang) + (setq params-line (concat params-line " :shebang \"#!/bin/sh\""))) + (let* ((result (org-test-with-temp-text + (mapconcat #'identity + (list "#+name: input" + "tramp" + "" + (concat "" + "#+begin_src sh :results output " params-line) + args-line + who-line + " echo \"hello $who from $(pwd)/\"" + "#+end_src") + "\n") + (org-trim (org-babel-execute-src-block)))) + (expected (concat "ARGS: --verbose 23 71" + "\nhello tramp from " (file-local-name default-directory)))) + (should (equal result expected))))))) + (provide 'test-ob-shell) ;;; test-ob-shell.el ends here diff --git a/testing/org-test.el b/testing/org-test.el index 0520e82f9..7e5d60e63 100644 --- a/testing/org-test.el +++ b/testing/org-test.el @@ -284,6 +284,35 @@ (defun org-test-table-target-expect (target &optional expect laps ;; on multiple lines in the ERT results buffer. (setq pp-escape-newlines back))))) +(defun org-test-with-tramp-remote-dir--worker (body) + "Worker for 'org-test-with-tramp-remote-dir'." + (let ((env-def (getenv "REMOTE_TEMPORARY_FILE_DIRECTORY"))) + (cond + (env-def (funcall body env-def)) + ((eq system-type 'windows-nt) (funcall body null-device)) + (t (require 'tramp) + (let ((tramp-methods + (cons '("mock" + (tramp-login-program "sh") + (tramp-login-args (("-i"))) + (tramp-remote-shell "/bin/sh") + (tramp-remote-shell-args ("-c")) + (tramp-connection-timeout 10)) + tramp-methods)) + (tramp-default-host-alist + `(("\\`mock\\'" nil ,(system-name))))) + (funcall body (format "/mock::%s" temporary-file-directory))))))) + +(defmacro org-test-with-tramp-remote-dir (dir &rest body) + "Bind the symbol DIR to a remote directory and execute BODY. +Return the value of the last form in BODY. The directory DIR +will be something like \"/mock::/tmp/\", which allows to test +Tramp related features. We mostly follow +`tramp-test-temporary-file-directory' from GNU Emacs tests." + (declare (debug (sexp body)) (indent 2)) + `(org-test-with-tramp-remote-dir--worker (lambda (,dir) ,@body))) + + ;;; Navigation Functions -- 2.35.1 --=-=-=--