Subject: bug#74208: closed (Re: bug#74208: 31.0.50; minibuffer read-file-name-default mutates global value of default-directory incorrectly)

From: [email-masked] (GNU bug Tracking System)
Date: Mon, 09 Dec 2024 08:18:02 +0000

Your bug report

#74208: 31.0.50; minibuffer read-file-name-default mutates global value of default-directory incorrectly

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

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

Subject: Re: bug#74208: 31.0.50; minibuffer read-file-name-default mutates global value of default-directory incorrectly
From: Michael Albinus <[email-masked]>
To: Madhu <[email-masked]>
Cc: [email-masked]
Date: Mon, 09 Dec 2024 09:17:20 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Version: 31.1

Madhu <[email-masked]> writes:

Hi Madhu,

> I meant the defaulting use in the call to get_current_directory in
> callproc.c

I see. But as said, it isn't a robust setting:

--8<---------------cut here---------------start------------->8---
# env HOME=/foo emacs -Q --batch --eval '(let ((default-directory (expand-file-name "~"))) (shell-command "ls"))'

Error: file-missing ("Setting current directory" "No such file or directory" "/foo")
  call-shell-region(1 1 "ls" nil #<buffer *Shell Command Output*>)
  shell-command-on-region(1 1 "ls" nil nil nil)
  shell-command("ls")
  (let ((default-directory (expand-file-name "~"))) (shell-command "ls"))
  eval((let ((default-directory (expand-file-name "~"))) (shell-command "ls")) t)
  command-line-1(("--eval" "(let ((default-directory (expand-file-name \"~\"))) (shell-command \"ls\"))"))
  command-line()
  normal-top-level()

(Shell command failed with error)
Setting current directory: No such file or directory, /foo
--8<---------------cut here---------------end--------------->8---

> Speaking for myself: no. I'm a bit dissapointed that the setq doesn't
> get fixed, but I think the main problem has been addressed.

Thanks, so I'm closing the bug.

Best regards, Michael.

Subject: 31.0.50; minibuffer read-file-name-default mutates global value of default-directory incorrectly
From: Madhu <[email-masked]>
To: [email-masked]
Date: Tue, 05 Nov 2024 07:39:24 +0530 (IST)
List-Id: <debbugs-submit.debbugs.gnu.org>

minibuffer.el: (read-file-name-default) has the following code:
```
                     (minibuffer-with-setup-hook
                         (lambda ()
                           (setq default-directory dir)
		   ...
			  (set-syntax-table minibuffer-local-filename-syntax))
```

This mutates the global binding of default-directory which is incorrect.

To demonstrate the problem, in emacs -Q

(insert "http://example.com")
(setq enable-recursive-minibuffers t)

position point at in the middle of the string  http://example.com
M-: (ffap)

;; while waiting for input
M-: default-directory
;; => default-directory is bound to "http://example.com"
M-: (shell-command "echo foo")
; ;=>
;; Debugger entered--Lisp error: (file-missing "Setting current directory" "No such file or directory" "http://example.com/")

I think this can be addressed by binding default-directory before
modifying it.

```
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3671,6 +3671,7 @@ read-file-name-default
                               (expand-file-name dir))))
                     (minibuffer-with-setup-hook
                         (lambda ()
+                         (let ((default-directory default-directory))
                           (setq default-directory dir)
                           ;; When the first default in `minibuffer-default'
                           ;; duplicates initial input `insdef',
@@ -3689,7 +3690,7 @@ read-file-name-default
                                  (with-current-buffer
                                      (window-buffer (minibuffer-selected-window))
 				   (read-file-name--defaults dir initial))))
-			  (set-syntax-table minibuffer-local-filename-syntax))
+			  (set-syntax-table minibuffer-local-filename-syntax)))
                       (completing-read prompt 'read-file-name-internal
                                        pred require-match insdef
                                        'file-name-history default-filename)))
```

[I'm sending this before I forget so it can be reviewed, but I can
send this patch as as attachment if needed, and it'll take some more
time until I figure out how to write a test for it and test it.]