Here's what I've been using as a workaround. It may not be any help in resolving any underlying issues, however.
(with-eval-after-load 'tramp (advice-add 'tramp-local-environment-variable-p :around (lambda (orig arg) (if (and (string-prefix-p "GIT_INDEX_FILE=" arg) (string-match-p "index\\.magit\\." arg)) nil (funcall orig arg))))) It sounds like I should report this issue to magit? Thanks, Zach On 11/1/26 8:41 AM, Jonas Bernoulli wrote:
I'm adding Steve to the discussion. Augusto Stoffel <[email protected]> writes:On Fri, 30 Oct 2026, Jonas Bernoulli wrote:(declare (indent 0) (debug (body)))(declare (indent 0) (debug (body))) (let ((p (gensym))) `(let ((,p process-environment)) (with-temp-buffer (let ((process-environment ,p)) ,@body)))))A.k.a. (defmacro magit--with-temp-process-buffer (&rest body) (declare (indent 0) (debug (body))) (let ((process-environment process-environment)) (with-temp-buffer ,@body)))Not really. [ I am sorry, this ended up stream-of-though style, because I am in a hurry. A family event is coming up. So please bear with me. I will get to a (probably sufficient) solution at the end. But the detour is interesting too. (I think I found an Emacs bug.) ] `process-environment' *seems to* behave differently, so let's use another variable: (defvar *var* :default) (with-temp-buffer (let ((*var* :outer)) (with-temp-buffer *var*))) => :outer (since not buffer-local) (with-temp-buffer (setq-local *var* :outer) (with-temp-buffer *var*)) => :default (as anyone would expect) (with-temp-buffer (make-local-variable '*var*) (let ((*var* :outer)) (with-temp-buffer *var*))) => :default (surprising when first encountered, but ...) makes sense and documented in (info "(elisp)Intro to Buffer-Local"): *Warning:* When a variable has buffer-local bindings in one or more buffers, ‘let’ rebinds the binding that’s currently in effect. For instance, if the current buffer has a buffer-local value, ‘let’ temporarily rebinds that. If no buffer-local bindings are in effect, ‘let’ rebinds the default value. If inside the ‘let’ you then change to a different current buffer in which a different binding is in effect, you won’t see the ‘let’ binding any more. And if you exit the ‘let’ while still in the other buffer, you won’t see the unbinding occur (though it will occur properly). Here is an example to illustrate: That is what I try to counter with: (defmacro with-temp-buffer-transplant-local-*var* (&rest body) `(let ((-var- *var*)) (with-temp-buffer (let ((*var* -var-)) *var*)))) (with-temp-buffer (make-local-variable '*var*) (let ((*var* :outer)) (with-temp-buffer-transplant-local-*var* *var*))) => :outer (as needed in this case) But `process-environment' *seems to* behave differently: (setenv "FOO" ":default") (with-temp-buffer (let ((process-environment (copy-sequence process-environment))) (setenv "FOO" ":outer") (with-temp-buffer (getenv "FOO")))) => :outer (since not buffer-local) (with-temp-buffer (setq-local process-environment (copy-sequence process-environment)) (setenv "FOO" ":outer") (with-temp-buffer (getenv "FOO"))) => :outer (why?) The reason is that `setenv' modifies the strings in `process-environment'. This seems questionable. It means that (let ((process-environment (copy-sequence process-environment))) (setenv ...) ...) might end up changing the outer `process-environment'. The fix is to use (let ((process-environment (copy-tree process-environment))) (setenv ...) ;^^^^ ...) instead. I haven't checked whether that is documented anywhere, but even the official macro for scoped envvar bindings fails to do this, so it must be a common bug: (defmacro with-environment-variables (variables &rest body) "Set VARIABLES in the environment and execute BODY. VARIABLES is a list of variable settings of the form (VAR VALUE), where VAR is the name of the variable (a string) and VALUE is its value (also a string). The previous values will be restored upon exit." (declare (indent 1) (debug (sexp body))) (unless (consp variables) (error "Invalid VARIABLES: %s" variables)) `(let ((process-environment (copy-sequence process-environment))) ,@(mapcar (lambda (elem) `(setenv ,(car elem) ,(cadr elem))) variables) ,@body)) I'll switch to using `copy-tree' myself. But I think that's something that needs to be fixed in Emacs.But this changes Tramp's behavior in that if you do (setq-local process-environment (cons "SECRETIVE_SECRET=123" process-environment)) then `make-process' will not propagate SECRETIVE_SECRET to the remote machine, while (magit--with-temp-process-buffer (make-process ...)) will propagate it.Indeed, and that's of course a big issue.If your goal is integrating with packages that set process-environment locally (thanks for that!)"Locally" here means "using a let-binding", right? (As opposed to "using setq-local".) Well, the goal was to fix "Stashes are not saved when using envrc" (https://github.com/magit/magit/issues/4160). Steve, author of envrc.el proposed this macro. I am unsure how that package used to "set it locally" back then. Now doing so it involves: ;;; Propagate local environment to commands that use temp buffers(defun envrc-propagate-environment (orig &rest args)"Advice function to wrap a command ORIG and make it use our local env. This can be used to force compliance where ORIG starts processes in a temp buffer. ARGS is as for ORIG." (if envrc-mode (inheritenv (apply orig args)) (apply orig args)))(defun envrc-propagate-tramp-environment (buf)"Advice function to propagate tramp vars into local values in BUF. `tramp-remote-path' and `tramp-remote-process-environment' are propagated." (when envrc-mode (let ((cur-path envrc--remote-path) (cur-env tramp-remote-process-environment)) (with-current-buffer buf (setq-local tramp-remote-process-environment cur-env) (setq-local envrc--remote-path cur-path)))) buf)(defun envrc-get-remote-path (fn vec)"Advice function to wrap FN (`tramp-get-remote-path') with its argument VEC. Shortcuts tramp caching direnv sets the variable `exec-path'." (with-current-buffer (tramp-get-connection-buffer vec) (or envrc--remote-path (apply fn vec nil))))(advice-add 'async-shell-command :around #'envrc-propagate-environment)(advice-add 'shell-command :around #'envrc-propagate-environment) (advice-add 'shell-command-to-string :around #'envrc-propagate-environment) (advice-add 'dired-shell-command :around #'envrc-propagate-environment) (advice-add 'org-babel-eval :around #'envrc-propagate-environment) (advice-add 'org-export-file :around #'envrc-propagate-environment) (advice-add 'vc-do-command :around #'envrc-propagate-environment) (advice-add 'vc-call-backend :around #'envrc-propagate-environment) (advice-add 'vc-dir :around #'envrc-propagate-environment) (advice-add 'tramp-get-connection-buffer :filter-return #'envrc-propagate-tramp-environment) (advice-add 'tramp-get-remote-path :around #'envrc-get-remote-path) I do not know whether 1. that is the full story 2. this makes the kludge in Magit unnecessary (at least for this package) 3. that by itself causes secret leakage. I do know that I want A. Magit to not cause leakage B. Magit to not lose data because $GIT_INDEX_FILE is unset C. create a Magit release today, because I already skipped last months monthly release and this also holds up monthly releases of other packages So I think what I will do is I. make magit--with-temp-process-buffer an alias for with-temp-file, for the time being II. add a safety-net (defmacro magit-with-temp-index (tree arg &rest body) (declare (indent 2) (debug (form form body))) (let ((file (gensym "file"))) `(let ((magit--refresh-cache nil) (,file (magit-convert-filename-for-git (make-temp-name (expand-file-name "index.magit." (magit-gitdir)))))) (unwind-protect (magit-with-toplevel (when-let* ((tree ,tree) (_(not (magit-git-success "read-tree" ,arg tree (concat "--index-output=" ,file))))) (error "Cannot read tree %s" tree)) + (with-environment-variables (("GIT_INDEX_FILE" ,file)) + (unless (equal (magit-git-string + "-c" "alias.GIF=!echo \"$GIT_INDEX_FILE\"" "GIF") + ,file) + (error "BUG: $GIT_INDEX_FILE ends up unset"))) (with-environment-variables (("GIT_INDEX_FILE" ,file)) ,@body)) (ignore-errors (delete-file (concat (file-remote-p default-directory) ,file))))))) III. maybe (conceptually) replace (copy-sequence process-environment) with (copy-tree process-environment)while at the same time being compatible with Tramp's trickery around environment variables (cf. tramp-local-environment-variable-p), then the right thing to do is to call buffer-local-toplevel-value in the original buffer and then set-buffer-local-toplevel-value in the temp buffer.I think I now understand what you are getting at, but this accomplishes the opposite of what we want. We must ensure that the let-binding in the temporary buffer only binds the value that are let-bound in the previous buffer. What you propose ensures that we only let-bind the values that are *not* let-bound in the previous buffer. But if we actually do the same in magit--with-temp-process-buffer as is being done in tramp-sh-handle-make-process, then we (unsurprisingly) get the behavior we want: (with-temp-buffer ;; I.e., in some magit buffer. ;; Third-party makes process-environment local (make-local-variable 'process-environment) ;; Avoid Emacs bug. (setq process-environment (copy-tree process-environment)) ;; Forth-party sets a secret (setenv "SECRET" "12345") ;; We bind an envvar (let ((process-environment (cons "GIT_INDEX_FILE=temp" process-environment))) (list ;; Using the naive approach, both value are missing. (with-temp-buffer (list (getenv "SECRET") (getenv "GIT_INDEX_FILE"))) ;; Using Magit's old approach, both value are visible. (let ((p process-environment)) (with-temp-buffer (let ((process-environment p)) (list (getenv "SECRET") (getenv "GIT_INDEX_FILE"))))) ;; Using proposed approch, we hide value we don't want ;; to hide and, don't hide value we want to hide. (let ((p (or (ignore-error void-variable (buffer-local-toplevel-value 'process-environment)) (default-toplevel-value 'process-environment)))) (with-temp-buffer (let ((process-environment p)) (list (getenv "SECRET") (getenv "GIT_INDEX_FILE"))))) ;;; ^ BAD ;;; v GOOD ;; So do the oppsite. I.e., REALLY do the same thing as Tramp. (list (cons :orig-bindings (*tramp-env-diff*)) (let ((p (*tramp-env-diff*))) (with-temp-buffer (let ((process-environment p)) (cons :copied-bindings (*tramp-env-diff*))))))))) (defun *tramp-env-diff* () ;; Copied from tramp-handle-make-process. (let (env) (dolist (elt process-environment env) (when (and (string-search "=" elt) (not (tramp-local-environment-variable-p elt))) (setq env (cons elt env)))))) So, instead of a placeholder alias as I said above, I might actually go with (defmacro magit--with-temp-process-buffer (&rest body) (let ((p (gensym))) `(let ((,p (seq-filter (lambda (elt) (and (string-search "=" elt) (not (tramp-local-environment-variable-p elt)))) process-environment))) (with-temp-buffer (let ((process-environment ,p)) ,@body))))) Unfortunately I now have to run. Cheers, Jonas
OpenPGP_0x6ADE77B6D65ED882.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
