[PATCH] ob-python.el: Fix issue with sessions on remote machines

[PATCH] ob-python.el: Fix issue with sessions on remote machines

[Christian Vanderwall]

[-- Attachment #1: Type: text/plain, Size: 518 bytes --]

The function `org-babel-python-evaluate-session' doesn't process temp
file names with `org-babel-process-file-name' before inserting them into
the Python code blocks. This causes a 'No such file' error when the
executing the code blocks on a remote directory.

The attached patch fixes this issue, allowing compilation of Python
source blocks with a remote directory, such as :dir /ssh:user@server:/.

This is my first patch ever, so please let me know if there's ways I can
improve.

Thanks!
-- 
Christian Vanderwall

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ob-python.el-Fix-issue-with-sessions-on-remote-machi.patch --]
[-- Type: text/x-patch, Size: 1626 bytes --]

From 7cf3d4c53e12addced7d2f65de1632971f44f056 Mon Sep 17 00:00:00 2001
From: Christian Vanderwall <christian@cvdub.net>
Date: Tue, 11 Aug 2020 20:26:39 -0700
Subject: [PATCH] ob-python.el: Fix issue with sessions on remote machines

* lisp/ob-python.el (org-babel-python-evaluate-session): Process
temporary file name with `org-babel-process-file-name' before
inserting it into code Python code snippets.

Before this change, the entire temporary filename was sent to the
Python session for execution, causing a 'No such file' error when the
filename had a Tramp format such as
/ssh:user@server:/tmp/python-ABCDEF.

TINYCHANGE
---
 lisp/ob-python.el | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lisp/ob-python.el b/lisp/ob-python.el
index 69312f2c9..0e3c79522 100644
--- a/lisp/ob-python.el
+++ b/lisp/ob-python.el
@@ -335,7 +335,8 @@ last statement in BODY, as elisp."
 						  "python-")))
 			       (with-temp-file tmp-src-file (insert body))
 			       (format org-babel-python--exec-tmpfile
-				       tmp-src-file))
+				       (org-babel-process-file-name
+					tmp-src-file 'noquote)))
 			   body)))
 	       (mapconcat
 		#'org-trim
@@ -353,7 +354,8 @@ last statement in BODY, as elisp."
 					      "python-")))
 			   (with-temp-file tmp-src-file (insert body))
 			   (format org-babel-python--eval-ast
-				   tmp-src-file))))
+				   (org-babel-process-file-name
+				    tmp-src-file 'noquote)))))
                (org-babel-comint-with-output
                    (session org-babel-python-eoe-indicator nil body)
                  (let ((comint-process-echoes nil))
-- 
2.28.0

Re: [PATCH] ob-python.el: Fix issue with sessions on remote machines

[Jack Kamm]

Hi Christian,

Thanks for reporting this and providing a fix!

> The function `org-babel-python-evaluate-session' doesn't process temp
> file names with `org-babel-process-file-name' before inserting them into
> the Python code blocks. This causes a 'No such file' error when the
> executing the code blocks on a remote directory.

I tested this out and was able to reproduce the bug.

> The attached patch fixes this issue, allowing compilation of Python
> source blocks with a remote directory, such as :dir /ssh:user@server:/.

I tested your patch and it solves the problem.

> This is my first patch ever, so please let me know if there's ways I can
> improve.

The patch is pretty simple and I see no problems with it. Since it is so
small, FSF copyright assignment shouldn't be necessary. Also, it looks
like you followed all the guidelines on
https://orgmode.org/worg/org-contribute.html.

I'm fairly novice myself so will wait a couple days in case one of the
more experienced folks notices any issues here. But I'll merge this to
master by end of week if I don't hear anything.

Re: [PATCH] ob-python.el: Fix issue with sessions on remote machines

[Jack Kamm]

Merged now -- thanks for your contribution.

Cheers,
Jack

Re: [PATCH] ob-python.el: Fix issue with sessions on remote machines

[Bastien]

Hi Jack,

Jack Kamm <jackkamm@gmail.com> writes:

> Merged now -- thanks for your contribution.

Thanks for taking care of this.

Would you be okay to add yourself as the ob-python.el maintainer?

I suggest we have a policy that "Org maintainer(s)" have the last
words on everything in Org's core, but that individual maintainers,
when known from the header section of an Elisp file, have the very
"first look" on bug reports and feature suggestions.

WDYT?

-- 
 Bastien

Re: [PATCH] ob-python.el: Fix issue with sessions on remote machines

[Jack Kamm]

Hi Bastien,

> Would you be okay to add yourself as the ob-python.el maintainer?

Sure, I've added myself as maintainer to the header of ob-python.el.

> I suggest we have a policy that "Org maintainer(s)" have the last
> words on everything in Org's core, but that individual maintainers,
> when known from the header section of an Elisp file, have the very
> "first look" on bug reports and feature suggestions.
>
> WDYT?

I'm trying to review ob-python related patches and mail as I notice
them, and monitor the list for mails with "python" in the subject,
though some may fall through the cracks occasionally, especially when my
workload is heavy.

Should I merge in patches to ob-python.el, as I did here? Or should I
simply review them, and let the core maintainers merge them in after
review?

Jack

Re: [PATCH] ob-python.el: Fix issue with sessions on remote machines

[Bastien]

Hi Jack,

Jack Kamm <jackkamm@gmail.com> writes:

>> Would you be okay to add yourself as the ob-python.el maintainer?
>
> Sure, I've added myself as maintainer to the header of ob-python.el.

Thanks!

>> I suggest we have a policy that "Org maintainer(s)" have the last
>> words on everything in Org's core, but that individual maintainers,
>> when known from the header section of an Elisp file, have the very
>> "first look" on bug reports and feature suggestions.
>>
>> WDYT?
>
> I'm trying to review ob-python related patches and mail as I notice
> them, and monitor the list for mails with "python" in the subject,
> though some may fall through the cracks occasionally, especially when my
> workload is heavy.
>
> Should I merge in patches to ob-python.el, as I did here? Or should I
> simply review them, and let the core maintainers merge them in after
> review?

Sorry, my policy proposal was incomplete:

- A local maintainer is expected to reply to requests and bug reports
  regarding the local functionalities he oversees.

- A local maintainer can commit changes directly to the file(s) he
  maintains (either submitted changes or his own).

- Core maintainers have the final word on any change in any file (so
  in case of a disagreement with a local maintainer, core maintainers
  have priority.)

In general, I would like to encourage "optimistic merging" from more
"local" maintainers.

Does that sound right?  When in doubt, always discuss changes first.

Thanks!

-- 
 Bastien

Re: [PATCH] ob-python.el: Fix issue with sessions on remote machines

[Jack Kamm]

Hi Bastien,

> - A local maintainer is expected to reply to requests and bug reports
>   regarding the local functionalities he oversees.
>
> - A local maintainer can commit changes directly to the file(s) he
>   maintains (either submitted changes or his own).
>
> - Core maintainers have the final word on any change in any file (so
>   in case of a disagreement with a local maintainer, core maintainers
>   have priority.)
>
> In general, I would like to encourage "optimistic merging" from more
> "local" maintainers.
>
> Does that sound right?  When in doubt, always discuss changes first.

I agree with this policy, and am more or less following it already,
though it's good to be explicit.

I'm keeping an eye out for Python-related mail, but if it looks like
I've missed one, add me to the TO or CC field to ensure it lands in my
inbox.

I appreciate the freedom to make changes to ob-python, but of course
defer to you, Nicolas, and Kyle, and seek out guidance when unsure.

Cheers,
Jack

Re: [PATCH] ob-python.el: Fix issue with sessions on remote machines

[Bastien]

Hi Jack,

Jack Kamm <jackkamm@gmail.com> writes:

> I agree with this policy, and am more or less following it already,
> though it's good to be explicit.

Yep, for future references as well.

I think it would be good to have more maintainers, not just for files
but for bigger areas of the code (Babel, Agenda, Export engine, Table,
Spreadsheet, etc.)

> I'm keeping an eye out for Python-related mail, but if it looks like
> I've missed one, add me to the TO or CC field to ensure it lands in my
> inbox.
>
> I appreciate the freedom to make changes to ob-python, but of course
> defer to you, Nicolas, and Kyle, and seek out guidance when unsure.

Thanks for your time and help again!

-- 
 Bastien