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

From: [email-masked] (GNU bug Tracking System)
Date: Tue, 01 Jul 2025 20:17:02 +0000

Your bug report

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

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

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

Subject: Re: bug#78855: [PATCH] Expand DIR arg on `package-vc-install-from-checkout'
From: Philip Kaludercic <[email-masked]>
To: Roi Martin <[email-masked]>
Cc: Eli Zaretskii <[email-masked]>, [email-masked]
Date: Tue, 01 Jul 2025 20:16:27 +0000
List-Id: <debbugs-submit.debbugs.gnu.org>

Roi Martin <[email-masked]> writes:

> Philip Kaludercic <[email-masked]> writes:
>
>> Will change that.  Just want to know what Roi thinks of the change, and
>> if I should commit it under my or their name?
>
> LGTM.  Please, commit it under your name.  Thanks for giving it a look!

OK, but I have listed you under Co-developed-by FWIW.  I'm closing the
bug report now.

> BTW, in the future, I'll follow your approach for detecting interactive
> calls.  I saw that the Emacs Lisp reference recommends it, but I was
> worried about adding a new optional parameter to the function.

1+

>         Roi

Subject: [PATCH] Expand DIR arg on `package-vc-install-from-checkout'
From: Roi Martin <[email-masked]>
To: [email-masked]
Date: Sat, 21 Jun 2025 13:51:48 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

Tags: patch

When `package-vc-install-from-checkout' is called interactively, it asks
users to input a directory.  If users have enabled `fido-mode', when
selecting a directory and pressing RET, they will actually pass
"/path/to/pkg/./" to the function.  Because NAME defaults to the base
name of DIR, it is assigned the value ".".  That is problematic in
several ways.

First, the package would be named ".", which is unexpected.

Second, the following code in `package-vc-install-from-checkout':

  (let* ((name (or name (file-name-base (directory-file-name dir))))
         (pkg-dir (expand-file-name name package-user-dir))
	 ; ...
    (when (file-exists-p pkg-dir)
      (if (yes-or-no-p (format "Overwrite previous checkout for package `%s'?" name))
          (package--delete-directory pkg-dir)
        (error "There already exists a checkout for %s" name)))
    (make-directory pkg-dir t)
    ; ...

would assign `pkg-dir' the value of `package-user-dir', which is wrong.
I.e. it would check for the existence of the wrong directory, it will
try to create the wrong directory, etc.

This patch expands the directory name passed by the user.  So now the
behavior of the command is:

  NAME defaults to the base name of DIR after being expanded.

I don't think there is any case where the old behavior is expected by
users.  So, this change should not break any current use of the
function.  Is this a fair assumption?  Is there any case where the old
behavior can be useful?

        Roi

>From 73d15ca95c2842949050f3c2f64b1991b36c54b0 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 ".".
* lisp/emacs-lisp/package-vc.el (package-vc-install-from-checkout):
Expand the provided directory name.
---
 lisp/emacs-lisp/package-vc.el | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
index babc0b715245..69852e27107a 100644
--- a/lisp/emacs-lisp/package-vc.el
+++ b/lisp/emacs-lisp/package-vc.el
@@ -915,22 +915,23 @@ package-vc-checkout
 ;;;###autoload
 (defun package-vc-install-from-checkout (dir &optional name)
   "Install the package NAME from its source directory DIR.
-NAME defaults to the base name of DIR.
+NAME defaults to the base name of DIR after being expanded.
 Interactively, prompt the user for DIR, which should be a directory
 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: "))
-                      (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))))
          (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))
-- 
2.49.0