Subject: bug#72849: closed (Re: bug#72849: [PATCH] Keep project's exec-path during with-temp-buffer call)

From: [email-masked] (GNU bug Tracking System)
Date: Sat, 31 Aug 2024 10:20:02 +0000

Your bug report

#72849: [PATCH] Keep project's exec-path during with-temp-buffer call

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
72849: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=72849
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#72849: [PATCH] Keep project's exec-path during with-temp-buffer call
From: Eli Zaretskii <[email-masked]>
To: kobarity <[email-masked]>
Cc: [email-masked], [email-masked]
Date: Sat, 31 Aug 2024 13:17:58 +0300
List-Id: <debbugs-submit.debbugs.gnu.org>

> Date: Fri, 30 Aug 2024 23:43:57 +0900
> From: kobarity <[email-masked]>
> Cc: [email-masked]
> 
> Evgenii Klimov wrote:
> > Eli Zaretskii <[email-masked]> writes:
> > 
> > >> Date: Wed, 28 Aug 2024 00:13:25 +0100
> > >> From:  Evgenii Klimov via "Bug reports for GNU Emacs,
> > >>  the Swiss army knife of text editors" <[email-masked]>
> > >> 
> > >> `with-temp-buffer' doesn't respect buffer-local environment variables,
> > >> `exec-path' in this case.  Which results in executables not being found,
> > >> or the wrong versions of executables being picked up.  E.g. if
> > >> environment variable is modified via .dir-local file or direnv/envrc
> > >> package.
> > >
> > > Hmm, this doesn't look clean to me: exec-path is just one variable,
> > > what makes it special here?
> > >
> > > Moreover, it sounds like python-shell-with-environment, which
> > > python-shell-prompt-detect calls, already attempts to have
> > > buffer-local value of exec-path to be available to Python, so why
> > > isn't that working for you?  And if it isn't work, I think we should
> > > amend python-shell-with-environment to do this, so we don't need to do
> > > it "by hand".
> > 
> > Indeed, my initial approach is too manual.
> > 
> > Here the problem that I have: I don't use Python's "venv" module to
> > create virtual environment for the project.  Instead, I use GNU Guix's
> > "guix shell" command [1] which provides augmented PATH and PYTHONPATH,
> > etc. to link project's dependencies.  Then, envrc.el package picks up
> > these environment variables and makes them buffer-local project-wise
> > (`exec-path' and `process-environment').
> > 
> > You are correct that `python-shell-with-environment' provides
> > buffer-local variables, but `with-temp-buffer' treats `exec-path' and
> > `process-environment' variables very specially.
> > 
> > I didn't find this behavior in documentation, but look at this example:
> > 
> >   (setq-default exec-path (list "global" "list"))
> >   (setq-local exec-path (cons "local"
> >                               (default-value 'exec-path)))
> >   (setq-default myvar (list "global" "list"))
> >   (setq-local myvar (cons "local" (default-value 'myvar)))
> > 
> >   (let ((exec-path exec-path) 		; takes buffer-local
> >         (myvar myvar)) 			; takes buffer-local
> >     (with-temp-buffer
> >       (insert (car exec-path) 		; uses global
> >               "\n"
> >               (car myvar)) 		; uses `let'-binded
> >       (buffer-string)))
> >   ;; => "global
> >   ;;     local"
> > 
> >   (require 'cl-lib)
> >   (let ((myvar myvar))
> >     ;; temporarily binds buffer-local value to global symbol
> >     (cl-letf (((default-value 'exec-path) exec-path))
> >       (with-temp-buffer
> >         ;; global variable is used, but it's value is temporarily equal
> >         ;; to buffer-local value
> >         (insert (car exec-path)
> >                 "\n"
> >                 (car myvar))
> >         (buffer-string))))
> >   ;; => "local
> >   ;;     local"
> > 
> > It's a simplified and expanded version of
> > `python-shell-with-environment' and `python-shell-prompt-detect'.  As
> > you can see, `exec-path' is treated differently inside of
> > `with-temp-buffer' and `cl-letf' is needed to force `with-temp-buffer'
> > to use buffer-local value of `exec-path'.
> > 
> > In the new patch attached I show how this can be overcome.  Don't know
> > if you'll consider my use case too narrow and specific, but I'll be glad
> > to hear your thoughts on this.
> > 
> > [1] https://guix.gnu.org/en/manual/en/guix.html#Invoking-guix-environment
> 
> Thank you for your explanation and the new patch.  It helped me to
> understand the problem correctly.  I also confirmed that the new patch
> resolves the issue.  I don't think it's a special use case, as I
> sometimes set `exec-path' locally using .dir-locals.el.  I agree with
> the new patch.

Thanks to both of you.  I've now installed this on the emacs-30
release branch, and I'm therefore closing this bug.

Subject: [PATCH] Keep project's exec-path during with-temp-buffer call
From: Evgenii Klimov <[email-masked]>
To: [email-masked]
Date: Wed, 28 Aug 2024 00:13:25 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Tags: patch

`with-temp-buffer' doesn't respect buffer-local environment variables,
`exec-path' in this case.  Which results in executables not being found,
or the wrong versions of executables being picked up.  E.g. if
environment variable is modified via .dir-local file or direnv/envrc
package.

I see that this function tries to be remote-host friendly (uses
`process-file') so I tried to ensure that this patch doesn't break this
effort, but I'm not sure that I understand the machinery behind TRAMP
correctly.  So please consider this aspect from your side.

This patch shouldn't interfere with TRAMP, if I understand
`process-file`s doc correctly:

     If a file name handler is invoked, it determines the program to run
     based on the first argument PROGRAM.  For instance, suppose that a
     handler for remote files is invoked.  Then the path that is used
     for searching for the program might be different from ‘exec-path’.

In GNU Emacs 30.0.60 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.24.41, cairo version 1.18.0)
Windowing system distributor 'The X.Org Foundation', version 11.0.12101011
System Description: Guix System

Configured using:
 'configure
 CONFIG_SHELL=/gnu/store/fl3l5wx8qynjrvx5lilz6c38hb77cf36-bash-minimal-5.1.16/bin/bash
 SHELL=/gnu/store/fl3l5wx8qynjrvx5lilz6c38hb77cf36-bash-minimal-5.1.16/bin/bash
 --prefix=/gnu/store/45nwc8hc8fn1fhvr9qw01ylkfpvzxwsw-emacs-next-30.0.60-1.4e22ef8
 --enable-fast-install --with-cairo --with-modules
 --with-native-compilation=aot --disable-build-details'

>From 9a6ca1c11a2849753fd3b854a79561224629a6bf Mon Sep 17 00:00:00 2001
From: Evgenii Klimov <[email-masked]>
Date: Tue, 27 Aug 2024 23:08:47 +0100
Subject: [PATCH] Keep project's exec-path during with-temp-buffer call

* lisp/progmodes/python.el (python-shell-prompt-detect):
`with-temp-buffer' doesn't respect buffer-local environment
variables, `exec-path' in this case.  Which results in executables
not being found, or the wrong versions of executables being picked
up.  E.g. if env var is modified via .dir-local file or
direnv/envrc package.
---
 lisp/progmodes/python.el | 44 +++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 7193cc19425..d6bb409c286 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -3116,27 +3116,29 @@ (defun python-shell-prompt-detect ()
              (interpreter python-shell-interpreter)
              (interpreter-arg python-shell-interpreter-interactive-arg)
              (output
-              (with-temp-buffer
-                ;; TODO: improve error handling by using
-                ;; `condition-case' and displaying the error message to
-                ;; the user in the no-prompts warning.
-                (ignore-errors
-                  (let ((code-file
-                         ;; Python 2.x on Windows does not handle
-                         ;; carriage returns in unbuffered mode.
-                         (let ((inhibit-eol-conversion (getenv "PYTHONUNBUFFERED")))
-                           (python-shell--save-temp-file code))))
-                    (unwind-protect
-                        ;; Use `process-file' as it is remote-host friendly.
-                        (process-file
-                         interpreter
-                         code-file
-                         '(t nil)
-                         nil
-                         interpreter-arg)
-                      ;; Try to cleanup
-                      (delete-file code-file))))
-                (buffer-string)))
+              (let ((current-exec-path exec-path))
+                (with-temp-buffer
+                  ;; TODO: improve error handling by using
+                  ;; `condition-case' and displaying the error message to
+                  ;; the user in the no-prompts warning.
+                  (ignore-errors
+                    (let ((code-file
+                           ;; Python 2.x on Windows does not handle
+                           ;; carriage returns in unbuffered mode.
+                           (let ((inhibit-eol-conversion (getenv "PYTHONUNBUFFERED")))
+                             (python-shell--save-temp-file code))))
+                      (unwind-protect
+			  ;; Use `process-file' as it is remote-host friendly.
+			  (let ((exec-path current-exec-path))
+			    (process-file
+			     interpreter
+			     code-file
+			     '(t nil)
+			     nil
+			     interpreter-arg))
+			;; Try to cleanup
+			(delete-file code-file))))
+                  (buffer-string))))
              (prompts
               (catch 'prompts
                 (dolist (line (split-string output "\n" t))
-- 
2.45.2