Subject: bug#78855: [PATCH] Expand DIR arg on `package-vc-install-from-checkout'

From: Philip Kaludercic <[email-masked]>
Date: Thu, 26 Jun 2025 20:41:17 +0000

Roi Martin <[email-masked]> writes:

> Eli Zaretskii <[email-masked]> writes:
>
>>> >>    (interactive (let* ((dir (read-directory-name "Directory: "))
>>> >> -                      (base (file-name-base (directory-file-name dir))))
>>> >> +                      (base (file-name-base (directory-file-name (expand-file-name dir)))))
>>> >>                   (list dir (and current-prefix-arg
>>> >>                                  (read-string
>>> >>                                   (format-prompt "Package name" base)
>>> >>                                   nil nil base)))))
>>> >>    (package-vc--archives-initialize)
>>> >> -  (let* ((name (or name (file-name-base (directory-file-name dir))))
>>> >> +  (let* ((dir (expand-file-name dir))
>>> >> +         (name (or name (file-name-base (directory-file-name dir))))
>>> >
>>> > Can we avoid this duplicate call to expand-file-name in the
>>> > interactive invocation?
>>> 
>>> I don't think so.  If I'm not wrong, `interactive' must be located at
>>> top-level and be the first form in the body (not counting `declare').
>>> Additionally, we also want to expand the directory name when the command
>>> is called from Emacs Lisp.  So, I don't see how to deduplicate that
>>> call.
>>
>> But there are ways to know if the call was interactive, right?
>
> Sorry, I thought you meant simplifying the code, not avoiding the second
> call at runtime.  I've attached a new version of the patch.
>
>         Roi
>
> From 16610a7da2f947e043db7ed95525091712ac38ff Mon Sep 17 00:00:00 2001
> From: Roi Martin <[email-masked]>
> Date: Sat, 21 Jun 2025 10:28:45 +0200
> Subject: [PATCH] Expand DIR arg on `package-vc-install-from-checkout'
>
> Expand the directory name passed to the
> `package-vc-install-from-checkout' function so that paths like
> "/path/to/pkg/./" are converted to "/path/to/pkg/".  This ensures that
> the package name defaults to "pkg" instead of "." (Bug#78855).
> * lisp/emacs-lisp/package-vc.el (package-vc-install-from-checkout):
> Expand the provided directory name.
> ---
>  lisp/emacs-lisp/package-vc.el | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
> index babc0b715245..11e5af884d0a 100644
> --- a/lisp/emacs-lisp/package-vc.el
> +++ b/lisp/emacs-lisp/package-vc.el
> @@ -920,17 +920,18 @@ package-vc-install-from-checkout
>  under version control, typically one created by `package-vc-checkout'.
>  If invoked interactively with a prefix argument, prompt the user
>  for the NAME of the package to set up."
> -  (interactive (let* ((dir (read-directory-name "Directory: "))
> +  (interactive (let* ((dir (expand-file-name (read-directory-name "Directory: ")))
>                        (base (file-name-base (directory-file-name dir))))
>                   (list dir (and current-prefix-arg
>                                  (read-string
>                                   (format-prompt "Package name" base)
>                                   nil nil base)))))
>    (package-vc--archives-initialize)
> -  (let* ((name (or name (file-name-base (directory-file-name dir))))
> +  (let* ((dir (if (called-interactively-p 'any) dir (expand-file-name dir)))

I do not get the advantage of this approach?  Even if we ignore that
`called-interactively-p' is tricky, the result is that whatever `dir' is
bound to after this point has passed through `expand-file-name'?  So
isn't the first patch the simpler solution, with the same effect?

> +         (name (or name (file-name-base (directory-file-name dir))))
>           (pkg-dir (expand-file-name name package-user-dir))
>           (package-vc-selected-packages
> -          (cons (list name :lisp-dir (expand-file-name dir))
> +          (cons (list name :lisp-dir dir)
>                  package-vc-selected-packages)))
>      (when (file-exists-p pkg-dir)
>        (if (yes-or-no-p (format "Overwrite previous checkout for package `%s'?" name))