On Fri, 30 Oct 2026, Jonas Bernoulli wrote:
> Augusto Stoffel <[email protected]> writes: > >> On Tue, 27 Oct 2026, Michael Albinus wrote: >> >>> I don't know magit at all, so it might be dumb what I'm saying. However, >>> there is >>> >>> (defmacro magit--with-temp-process-buffer (&rest body) >>> "Like `with-temp-buffer', but always propagate `process-environment'. >>> When that var is buffer-local in the calling buffer, it is not >>> propagated by `with-temp-buffer', so we explicitly ensure that >>> happens, so that processes will be invoked consistently. BODY is >>> as for that macro." >>> (declare (indent 0) (debug (body))) >>> (let ((p (gensym))) >>> `(let ((,p process-environment)) >>> (with-temp-buffer >>> (setq-local process-environment ,p) >>> ,@body)))) > > Ah, I forgot about that because Magit does not make that variable local > for its own purposes. It sets it here to deal with the fact that other > packages may make this variable buffer-local. > >> On the same vein as the aforementioned bug#81175, I'd say the correct >> version of that macro should read >> >> (let ((,p (or (ignore-error void-variable >> (buffer-local-toplevel-value 'process-environment)) >> (default-toplevel-value 'process-environment)))) >> ...) > > > The problem isn't with how this macro retrieves the value, but in how it > "sets" it in the temporary buffer. > > Fixed with: > > (defmacro magit--with-temp-process-buffer (&rest body) > "Like `with-temp-buffer', but propagate `process-environment' binding. > > When that variable is buffer-local in the previously current buffer, > it is not propagated by `with-temp-buffer', so we explicitly ensure > that happens, so that processes are invoked consistently. BODY is > as for that macro. > > Note that itself Magit does not ever make this variable buffer-local. > This macro only exists to cope with third-party code doing that." > (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))) 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. If your goal is integrating with packages that set process-environment locally (thanks for that!) 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.
