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

From: Stefan Monnier <[email-masked]>
Date: Sat, 09 Nov 2024 11:11:19 -0500

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

IIRC the `setq` is really there to set the value of `default-directory`
during the whole duration of the completion, not just during the setup hook.
This is needed/used if the user modifies the minibuffer's content to
hold only a relative file name, at which point we need that default
directory info in order to perform completions.

> ```
> 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)))
> ```

The let+setq could be simplified to

                        (let ((default-directory dir))

but none of the code within this `let` uses `default-directory` since
there are only calls to `car/cdr`, `setq`, and `set-syntax-table`, none
of which touch file names, so we may as well remove the
(setq default-directory dir) instead.

I'm leaning towards declaring the `shell-command` behavior above as
a feature rather than a bug.

        Stefan