unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#72849: [PATCH] Keep project's exec-path during with-temp-buffer call
@ 2024-08-27 23:13 Evgenii Klimov via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-08-28  7:12 ` Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-08-28 12:12 ` Eli Zaretskii
  0 siblings, 2 replies; 8+ messages in thread
From: Evgenii Klimov via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-08-27 23:13 UTC (permalink / raw)
  To: 72849

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

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'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Keep-project-s-exec-path-during-with-temp-buffer-cal.patch --]
[-- Type: text/patch, Size: 3219 bytes --]

From 9a6ca1c11a2849753fd3b854a79561224629a6bf Mon Sep 17 00:00:00 2001
From: Evgenii Klimov <eugene.dev@lipklim.org>
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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#72849: [PATCH] Keep project's exec-path during with-temp-buffer call
  2024-08-27 23:13 bug#72849: [PATCH] Keep project's exec-path during with-temp-buffer call Evgenii Klimov via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-08-28  7:12 ` Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-08-28 12:12 ` Eli Zaretskii
  1 sibling, 0 replies; 8+ messages in thread
From: Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-08-28  7:12 UTC (permalink / raw)
  To: 72849; +Cc: eugene.dev

Evgenii Klimov via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

Hi Evgenii,

> 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’.

Correct. Remote processes do not use the variable exec-path, so you can
change it at will, w/o breaking remote processes. Of course, exec-path
should still ensure that local programs, needed for accessing a remote
host (like ssh), shall still be usable.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#72849: [PATCH] Keep project's exec-path during with-temp-buffer call
  2024-08-27 23:13 bug#72849: [PATCH] Keep project's exec-path during with-temp-buffer call Evgenii Klimov via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-08-28  7:12 ` Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-08-28 12:12 ` Eli Zaretskii
  2024-08-29 16:08   ` kobarity
  2024-08-29 22:51   ` Evgenii Klimov via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 2 replies; 8+ messages in thread
From: Eli Zaretskii @ 2024-08-28 12:12 UTC (permalink / raw)
  To: Evgenii Klimov, kobarity; +Cc: 72849

> 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" <bug-gnu-emacs@gnu.org>
> 
> `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".

kobarity, any comments or suggestions?

> >From 9a6ca1c11a2849753fd3b854a79561224629a6bf Mon Sep 17 00:00:00 2001
> From: Evgenii Klimov <eugene.dev@lipklim.org>
> 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
> 





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#72849: [PATCH] Keep project's exec-path during with-temp-buffer call
  2024-08-28 12:12 ` Eli Zaretskii
@ 2024-08-29 16:08   ` kobarity
  2024-08-29 16:25     ` Eli Zaretskii
  2024-08-29 22:51   ` Evgenii Klimov via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 8+ messages in thread
From: kobarity @ 2024-08-29 16:08 UTC (permalink / raw)
  To: Eli Zaretskii, Evgenii Klimov; +Cc: 72849

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

Eli Zaretskii wrote:
> > 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" <bug-gnu-emacs@gnu.org>
> > 
> > `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".
> 
> kobarity, any comments or suggestions?

The problem is that `with-temp-buffer' creates a new buffer, so the
buffer local value of `exec-path' is not maintained in the new buffer.
It is used to find the Python interpreter unless the interpreter is
specified using the absolute path.

I think Evgenii's patch is logically correct, but it would be better
to create a modified version of `with-temp-buffer' which keeps
`exec-path' in the new buffer.  The attached is my proposal to do so.

[-- Attachment #2: 0001-Fix-Python-prompt-detection-when-exec-path-is-buffer.patch --]
[-- Type: application/octet-stream, Size: 1906 bytes --]

From b94ba0d4e07635a0abddf0cba7548b10b47dad92 Mon Sep 17 00:00:00 2001
From: kobarity <kobarity@gmail.com>
Date: Fri, 30 Aug 2024 00:13:27 +0900
Subject: [PATCH] Fix Python prompt detection when 'exec-path' is buffer local

* lisp/progmodes/python.el (python-with-temp-buffer): New macro
same as 'with-temp-buffer', except the variable 'exec-path' is
kept in the new buffer.
(python-shell-prompt-detect): Use it. (Bug#72849)

Co-authored-by: Evgenii Klimov <eugene.dev@lipklim.org>
---
 lisp/progmodes/python.el | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index a00289d6de9..480713c3aca 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -3088,6 +3088,16 @@ 'python--parse-json-array
         (json-read-from-string string))))
   "Parse the JSON array in STRING into a Lisp list.")
 
+(defmacro python-with-temp-buffer (&rest body)
+  "Python mode specific version of `with-temp-buffer'.
+Call `with-temp-buffer' with BODY.  The variable `exec-path' is kept in
+the new buffer even if it is buffer local."
+  (declare (indent 0) (debug t))
+  `(let ((python--exec-path exec-path))
+     (with-temp-buffer
+       (let ((exec-path python--exec-path))
+         ,@body))))
+
 (defun python-shell-prompt-detect ()
   "Detect prompts for the current `python-shell-interpreter'.
 When prompts can be retrieved successfully from the
@@ -3116,7 +3126,7 @@ python-shell-prompt-detect
              (interpreter python-shell-interpreter)
              (interpreter-arg python-shell-interpreter-interactive-arg)
              (output
-              (with-temp-buffer
+              (python-with-temp-buffer
                 ;; TODO: improve error handling by using
                 ;; `condition-case' and displaying the error message to
                 ;; the user in the no-prompts warning.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#72849: [PATCH] Keep project's exec-path during with-temp-buffer call
  2024-08-29 16:08   ` kobarity
@ 2024-08-29 16:25     ` Eli Zaretskii
  0 siblings, 0 replies; 8+ messages in thread
From: Eli Zaretskii @ 2024-08-29 16:25 UTC (permalink / raw)
  To: kobarity; +Cc: eugene.dev, 72849

> Date: Fri, 30 Aug 2024 01:08:25 +0900
> From: kobarity <kobarity@gmail.com>
> Cc: 72849@debbugs.gnu.org
> 
> Eli Zaretskii wrote:
> > > 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" <bug-gnu-emacs@gnu.org>
> > > 
> > > `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".
> > 
> > kobarity, any comments or suggestions?
> 
> The problem is that `with-temp-buffer' creates a new buffer, so the
> buffer local value of `exec-path' is not maintained in the new buffer.
> It is used to find the Python interpreter unless the interpreter is
> specified using the absolute path.
> 
> I think Evgenii's patch is logically correct, but it would be better
> to create a modified version of `with-temp-buffer' which keeps
> `exec-path' in the new buffer.  The attached is my proposal to do so.

Thanks, but what about python-shell-with-environment: doesn't it
already try to do this, including preserving exec-path from the
original buffer?





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#72849: [PATCH] Keep project's exec-path during with-temp-buffer call
  2024-08-28 12:12 ` Eli Zaretskii
  2024-08-29 16:08   ` kobarity
@ 2024-08-29 22:51   ` Evgenii Klimov via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-08-30 14:43     ` kobarity
  1 sibling, 1 reply; 8+ messages in thread
From: Evgenii Klimov via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-08-29 22:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: kobarity, 72849

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

Eli Zaretskii <eliz@gnu.org> 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" <bug-gnu-emacs@gnu.org>
>> 
>> `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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-python-shell-with-environment-respect-buffer-lo.patch --]
[-- Type: text/x-patch, Size: 1449 bytes --]

From 6db90f6087d0d3270e1d7a7d77d8830eac1f0154 Mon Sep 17 00:00:00 2001
From: Evgenii Klimov <eugene.dev@lipklim.org>
Date: Thu, 29 Aug 2024 23:36:12 +0100
Subject: [PATCH] Make python-shell--with-environment respect buffer-local
 variables

* lisp/progmodes/python.el (python-shell--with-environment): Make
`with-temp-buffer' respect buffer-local values of
`process-environment' and `exec-path', if set.
---
 lisp/progmodes/python.el | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 7193cc19425..3e543442dba 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -3030,11 +3030,11 @@ (defun python-shell--with-environment (extraenv bodyfun)
                  (tramp-dissect-file-name default-directory 'noexpand)))))
     (if vec
         (python-shell--tramp-with-environment vec extraenv bodyfun)
-      (let ((process-environment
-             (append extraenv process-environment))
-            (exec-path
-             ;; FIXME: This is still Python-specific.
-             (python-shell-calculate-exec-path)))
+      (cl-letf (((default-value 'process-environment)
+		 (append extraenv process-environment))
+		((default-value 'exec-path)
+		 ;; FIXME: This is still Python-specific.
+		 (python-shell-calculate-exec-path)))
         (funcall bodyfun)))))
 
 (defun python-shell--tramp-with-environment (vec extraenv bodyfun)
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#72849: [PATCH] Keep project's exec-path during with-temp-buffer call
  2024-08-29 22:51   ` Evgenii Klimov via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-08-30 14:43     ` kobarity
  2024-08-31 10:17       ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: kobarity @ 2024-08-30 14:43 UTC (permalink / raw)
  To: Evgenii Klimov, Eli Zaretskii; +Cc: 72849

Evgenii Klimov wrote:
> Eli Zaretskii <eliz@gnu.org> 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" <bug-gnu-emacs@gnu.org>
> >> 
> >> `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.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#72849: [PATCH] Keep project's exec-path during with-temp-buffer call
  2024-08-30 14:43     ` kobarity
@ 2024-08-31 10:17       ` Eli Zaretskii
  0 siblings, 0 replies; 8+ messages in thread
From: Eli Zaretskii @ 2024-08-31 10:17 UTC (permalink / raw)
  To: kobarity; +Cc: 72849-done, eugene.dev

> Date: Fri, 30 Aug 2024 23:43:57 +0900
> From: kobarity <kobarity@gmail.com>
> Cc: 72849@debbugs.gnu.org
> 
> Evgenii Klimov wrote:
> > Eli Zaretskii <eliz@gnu.org> 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" <bug-gnu-emacs@gnu.org>
> > >> 
> > >> `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.





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-08-31 10:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-27 23:13 bug#72849: [PATCH] Keep project's exec-path during with-temp-buffer call Evgenii Klimov via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-08-28  7:12 ` Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-08-28 12:12 ` Eli Zaretskii
2024-08-29 16:08   ` kobarity
2024-08-29 16:25     ` Eli Zaretskii
2024-08-29 22:51   ` Evgenii Klimov via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-08-30 14:43     ` kobarity
2024-08-31 10:17       ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).