GNU bug report logs - #77468
30.1.50; package-quickstart file is not relocatable

Previous Next

Package: emacs;

Reported by: Spencer Baugh <sbaugh <at> janestreet.com>

Date: Wed, 2 Apr 2025 18:47:01 UTC

Severity: normal

Found in version 30.1.50

Done: Stefan Monnier <monnier <at> iro.umontreal.ca>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 77468 in the body.
You can then email your comments to 77468 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to monnier <at> iro.umontreal.ca, bug-gnu-emacs <at> gnu.org:
bug#77468; Package emacs. (Wed, 02 Apr 2025 18:47:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Spencer Baugh <sbaugh <at> janestreet.com>:
New bug report received and forwarded. Copy sent to monnier <at> iro.umontreal.ca, bug-gnu-emacs <at> gnu.org. (Wed, 02 Apr 2025 18:47:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.1.50; package-quickstart file is not relocatable
Date: Wed, 02 Apr 2025 14:45:53 -0400
The package-quickstart file hardcodes various absolute file names.  This
means it can't be used in situations where the package directory might
be at different absolute names at package-quickstart-refresh time
and at usage time.

For example:

- network filesystems where the user's home directory has a different
  absolute name on different hosts

- a system-wide package-directory-list entry which has a
  package-quickstart file generated for it at build time which has a
  different absolute name at install time (this is my use case; there
  are other problems with it, but the absolute names are the biggest
  one)

I think we should make package-quickstart use file names which are
relative to the location of the package-quickstart file itself.  I'm
happy to write the patch for this if that sounds reasonable.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#77468; Package emacs. (Wed, 02 Apr 2025 19:49:02 GMT) Full text and rfc822 format available.

Message #8 received at 77468 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 77468 <at> debbugs.gnu.org
Subject: Re: bug#77468: 30.1.50; package-quickstart file is not relocatable
Date: Wed, 02 Apr 2025 15:48:12 -0400
> I think we should make package-quickstart use file names which are
> relative to the location of the package-quickstart file itself.

It would probably slow down startup a little bit since
`package-quickstart.el` (PQ) would need to `expand-file-name` every time
it binds `load-file-name`, but yes I think it would be a good change.

There is still the problem that the relative file names may themselves
break when your `package-directory-list` has directories that are
outside of the home directory.

Actually, now that I think about it, the change you suggest would break
one of my use cases (where my packages are always at the same place,
outside of $HOME, but $HOME does move).

Hmm...


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#77468; Package emacs. (Wed, 02 Apr 2025 19:55:01 GMT) Full text and rfc822 format available.

Message #11 received at 77468 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 77468 <at> debbugs.gnu.org
Subject: Re: bug#77468: 30.1.50; package-quickstart file is not relocatable
Date: Wed, 02 Apr 2025 15:54:16 -0400
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> I think we should make package-quickstart use file names which are
>> relative to the location of the package-quickstart file itself.
>
> It would probably slow down startup a little bit since
> `package-quickstart.el` (PQ) would need to `expand-file-name` every time
> it binds `load-file-name`, but yes I think it would be a good change.
>
> There is still the problem that the relative file names may themselves
> break when your `package-directory-list` has directories that are
> outside of the home directory.
>
> Actually, now that I think about it, the change you suggest would break
> one of my use cases (where my packages are always at the same place,
> outside of $HOME, but $HOME does move).

Oh, right.  I didn't think of the case where package-quickstart.el
refers to packages outside the directory it lives in.

I definitely don't think package-quickstart.el should use relative names
to those packages - that would be quite weird - and I do expect it would
break some use cases.

Maybe package-quickstart.el could use relative names to packages in
package-user-dir (since it itself lives in package-user-dir), and
absolute names to packages outside that?

That would also fix my issue and I'd be happy to implement it.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#77468; Package emacs. (Wed, 02 Apr 2025 20:25:01 GMT) Full text and rfc822 format available.

Message #14 received at 77468 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 77468 <at> debbugs.gnu.org
Subject: Re: bug#77468: 30.1.50; package-quickstart file is not relocatable
Date: Wed, 02 Apr 2025 16:24:38 -0400
> Maybe package-quickstart.el could use relative names to packages in
> package-user-dir (since it itself lives in package-user-dir), and
> absolute names to packages outside that?

Sounds OK.  Tho maybe we should check `file*-in-directory-p` (or the
presence of "../" in the output of `file-relative-name`) rather than
distinguishing based on `package-user-dir` vs `package-directory-list`?

In any case, it sounds like a good compromise.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#77468; Package emacs. (Wed, 02 Apr 2025 21:23:02 GMT) Full text and rfc822 format available.

Message #17 received at 77468 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 77468 <at> debbugs.gnu.org
Subject: Re: bug#77468: 30.1.50; package-quickstart file is not relocatable
Date: Wed, 02 Apr 2025 17:22:22 -0400
[Message part 1 (text/plain, inline)]
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> Maybe package-quickstart.el could use relative names to packages in
>> package-user-dir (since it itself lives in package-user-dir), and
>> absolute names to packages outside that?
>
> Sounds OK.  Tho maybe we should check `file*-in-directory-p` (or the
> presence of "../" in the output of `file-relative-name`) rather than
> distinguishing based on `package-user-dir` vs `package-directory-list`?
>
> In any case, it sounds like a good compromise.

Makes sense.  How about this?

[0001-Use-relative-names-where-possible-in-package-quickst.patch (text/x-patch, inline)]
From 7f465bee1605e0687f4f7713bcfeb9ac39b28355 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Wed, 2 Apr 2025 17:21:24 -0400
Subject: [PATCH] Use relative names where possible in package-quickstart.el

package-quickstart.el hardcodes many absolute file names, which
makes it break if user-emacs-directory moves, or in other
situations.  To be slightly more robust to this, use relative
file names to packages that are located in the same directory as
package-quickstart.el.

* lisp/emacs-lisp/package.el (package--quickstart-relative):
Add.
(package-quickstart-refresh): Use package--quickstart-relative
on file names.  (bug#77468)
---
 lisp/emacs-lisp/package.el | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index b9a8dacab15..1269a9c95e7 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -4591,6 +4591,14 @@ package--quickstart-maybe-refresh
     (delete-file (concat package-quickstart-file "c"))
     (delete-file package-quickstart-file)))
 
+(defun package--quickstart-relative (load-name file)
+  "Return an expression which evaluates to FILE while loading LOAD-NAME."
+  (let* ((dir (file-name-directory (expand-file-name load-name)))
+         (relname (file-relative-name file dir)))
+    (if (string-prefix-p ".." relname)
+        file
+      `(file-name-concat (file-name-directory load-file-name) ,relname))))
+
 (defun package-quickstart-refresh ()
   "(Re)Generate the `package-quickstart-file'."
   (interactive)
@@ -4605,7 +4613,8 @@ package-quickstart-refresh
         ;; aren't truncated.
         (print-length nil)
         (print-level nil)
-        (Info-directory-list '("")))
+        (Info-directory-list '(""))
+        (qs-dir (file-name-directory (expand-file-name package-quickstart-file))))
     (dolist (elt package-alist)
       (condition-case err
           (package-activate (car elt))
@@ -4622,7 +4631,7 @@ package-quickstart-refresh
                 ;; Prefer uncompiled files (and don't accept .so files).
                 (let ((load-suffixes '(".el" ".elc")))
                   (locate-library (package--autoloads-file-name pkg))))
-               (pfile (prin1-to-string file)))
+               (pfile (prin1-to-string (package--quickstart-relative package-quickstart-file file))))
           (insert "(let* ((load-file-name " pfile ")\
 \(load-true-file-name load-file-name))\n")
           (insert-file-contents file)
@@ -4638,7 +4647,10 @@ package-quickstart-refresh
                   (append ',(mapcar #'package-desc-name package--quickstart-pkgs)
                           package-activated-list)))
           (current-buffer))
-      (let ((info-dirs (butlast Info-directory-list)))
+      (let ((info-dirs
+             (mapcar (lambda (info-dir)
+                       (package--quickstart-relative package-quickstart-file info-dir))
+                     (butlast Info-directory-list))))
         (when info-dirs
           (pp `(progn (require 'info)
                       (info-initialize)
-- 
2.39.3


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#77468; Package emacs. (Wed, 02 Apr 2025 22:31:02 GMT) Full text and rfc822 format available.

Message #20 received at 77468 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 77468 <at> debbugs.gnu.org
Subject: Re: bug#77468: 30.1.50; package-quickstart file is not relocatable
Date: Wed, 02 Apr 2025 18:30:07 -0400
> Makes sense.  How about this?

See comments below.

> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index b9a8dacab15..1269a9c95e7 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -4591,6 +4591,14 @@ package--quickstart-maybe-refresh
>      (delete-file (concat package-quickstart-file "c"))
>      (delete-file package-quickstart-file)))
>  
> +(defun package--quickstart-relative (load-name file)
> +  "Return an expression which evaluates to FILE while loading LOAD-NAME."
> +  (let* ((dir (file-name-directory (expand-file-name load-name)))
> +         (relname (file-relative-name file dir)))
> +    (if (string-prefix-p ".." relname)
> +        file
> +      `(file-name-concat (file-name-directory load-file-name) ,relname))))

I think I'd test "../" just in case of dirnames that start with "..".
Also, could we compute (file-name-directory load-file-name)
once at the beginning of the quickstart file instead of once per
inlined file?

>  (defun package-quickstart-refresh ()
>    "(Re)Generate the `package-quickstart-file'."
>    (interactive)
> @@ -4605,7 +4613,8 @@ package-quickstart-refresh
>          ;; aren't truncated.
>          (print-length nil)
>          (print-level nil)
> -        (Info-directory-list '("")))
> +        (Info-directory-list '(""))
> +        (qs-dir (file-name-directory (expand-file-name package-quickstart-file))))

AFAICT `qs-dir` is not used.

> @@ -4622,7 +4631,7 @@ package-quickstart-refresh
>                  ;; Prefer uncompiled files (and don't accept .so files).
>                  (let ((load-suffixes '(".el" ".elc")))
>                    (locate-library (package--autoloads-file-name pkg))))
> -               (pfile (prin1-to-string file)))
> +               (pfile (prin1-to-string (package--quickstart-relative package-quickstart-file file))))
>            (insert "(let* ((load-file-name " pfile ")\
>  \(load-true-file-name load-file-name))\n")
>            (insert-file-contents file)

I think this needs wrapping to fit within 80 columns.

> @@ -4638,7 +4647,10 @@ package-quickstart-refresh
>                    (append ',(mapcar #'package-desc-name package--quickstart-pkgs)
>                            package-activated-list)))
>            (current-buffer))
> -      (let ((info-dirs (butlast Info-directory-list)))
> +      (let ((info-dirs
> +             (mapcar (lambda (info-dir)
> +                       (package--quickstart-relative package-quickstart-file info-dir))
> +                     (butlast Info-directory-list))))
>          (when info-dirs
>            (pp `(progn (require 'info)
>                        (info-initialize)

I think this won't work because the rest of the code does:

                      (setq Info-directory-list
                            (append ',info-dirs Info-directory-list)))

so the `file-name-concat`s won't be evaluated.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#77468; Package emacs. (Thu, 03 Apr 2025 05:17:01 GMT) Full text and rfc822 format available.

Message #23 received at 77468 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: monnier <at> iro.umontreal.ca, 77468 <at> debbugs.gnu.org
Subject: Re: bug#77468: 30.1.50; package-quickstart file is not relocatable
Date: Thu, 03 Apr 2025 08:16:37 +0300
> Cc: 77468 <at> debbugs.gnu.org
> Date: Wed, 02 Apr 2025 17:22:22 -0400
> From:  Spencer Baugh via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
> >> Maybe package-quickstart.el could use relative names to packages in
> >> package-user-dir (since it itself lives in package-user-dir), and
> >> absolute names to packages outside that?
> >
> > Sounds OK.  Tho maybe we should check `file*-in-directory-p` (or the
> > presence of "../" in the output of `file-relative-name`) rather than
> > distinguishing based on `package-user-dir` vs `package-directory-list`?
> >
> > In any case, it sounds like a good compromise.
> 
> Makes sense.  How about this?

Thanks, but please always remember to comment such non-trivial code,
explaining its purpose and the problems it attempts to solve.
Otherwise we make Emacs much harder to maintain for those who come
after us.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#77468; Package emacs. (Thu, 03 Apr 2025 13:25:02 GMT) Full text and rfc822 format available.

Message #26 received at 77468 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 77468 <at> debbugs.gnu.org
Subject: Re: bug#77468: 30.1.50; package-quickstart file is not relocatable
Date: Thu, 03 Apr 2025 09:24:19 -0400
[Message part 1 (text/plain, inline)]
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>> Makes sense.  How about this?
>
> See comments below.
>
>> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
>> index b9a8dacab15..1269a9c95e7 100644
>> --- a/lisp/emacs-lisp/package.el
>> +++ b/lisp/emacs-lisp/package.el
>> @@ -4591,6 +4591,14 @@ package--quickstart-maybe-refresh
>>      (delete-file (concat package-quickstart-file "c"))
>>      (delete-file package-quickstart-file)))
>>  
>> +(defun package--quickstart-relative (load-name file)
>> +  "Return an expression which evaluates to FILE while loading LOAD-NAME."
>> +  (let* ((dir (file-name-directory (expand-file-name load-name)))
>> +         (relname (file-relative-name file dir)))
>> +    (if (string-prefix-p ".." relname)
>> +        file
>> +      `(file-name-concat (file-name-directory load-file-name) ,relname))))
>
> I think I'd test "../" just in case of dirnames that start with "..".

Good catch... actually I think I should just use file-in-directory-p,
that's more self-explanatory.

> Also, could we compute (file-name-directory load-file-name)
> once at the beginning of the quickstart file instead of once per
> inlined file?

Done.

>>  (defun package-quickstart-refresh ()
>>    "(Re)Generate the `package-quickstart-file'."
>>    (interactive)
>> @@ -4605,7 +4613,8 @@ package-quickstart-refresh
>>          ;; aren't truncated.
>>          (print-length nil)
>>          (print-level nil)
>> -        (Info-directory-list '("")))
>> +        (Info-directory-list '(""))
>> +        (qs-dir (file-name-directory (expand-file-name package-quickstart-file))))
>
> AFAICT `qs-dir` is not used.

Fixed.

>> @@ -4622,7 +4631,7 @@ package-quickstart-refresh
>>                  ;; Prefer uncompiled files (and don't accept .so files).
>>                  (let ((load-suffixes '(".el" ".elc")))
>>                    (locate-library (package--autoloads-file-name pkg))))
>> -               (pfile (prin1-to-string file)))
>> +               (pfile (prin1-to-string (package--quickstart-relative package-quickstart-file file))))
>>            (insert "(let* ((load-file-name " pfile ")\
>>  \(load-true-file-name load-file-name))\n")
>>            (insert-file-contents file)
>
> I think this needs wrapping to fit within 80 columns.

Fixed.

>> @@ -4638,7 +4647,10 @@ package-quickstart-refresh
>>                    (append ',(mapcar #'package-desc-name package--quickstart-pkgs)
>>                            package-activated-list)))
>>            (current-buffer))
>> -      (let ((info-dirs (butlast Info-directory-list)))
>> +      (let ((info-dirs
>> +             (mapcar (lambda (info-dir)
>> +                       (package--quickstart-relative package-quickstart-file info-dir))
>> +                     (butlast Info-directory-list))))
>>          (when info-dirs
>>            (pp `(progn (require 'info)
>>                        (info-initialize)
>
> I think this won't work because the rest of the code does:
>
>                       (setq Info-directory-list
>                             (append ',info-dirs Info-directory-list)))
>
> so the `file-name-concat`s won't be evaluated.

Oops... I guess I didn't run info when testing this.  Fixed.

[0001-Use-relative-names-where-possible-in-package-quickst.patch (text/x-patch, inline)]
From e25d657cbe01b6fb1162c17c8d344eff63621131 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Wed, 2 Apr 2025 17:21:24 -0400
Subject: [PATCH] Use relative names where possible in package-quickstart.el

package-quickstart.el hardcodes many absolute file names, which
makes it break if user-emacs-directory moves, or in other
situations.  To be slightly more robust to this, use relative
file names to packages that are located in the same directory as
package-quickstart.el.

* lisp/emacs-lisp/package.el (package--quickstart-rel): Add.
(package-quickstart-refresh): Use package--quickstart-rel on
file names.  (bug#77468)
---
 lisp/emacs-lisp/package.el | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index b9a8dacab15..a575dea7b92 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -4591,6 +4591,14 @@ package--quickstart-maybe-refresh
     (delete-file (concat package-quickstart-file "c"))
     (delete-file package-quickstart-file)))
 
+(defun package--quickstart-rel (dir file)
+  "Return an expression which evaluates to FILE while loading a file in DIR."
+  (if (file-in-directory-p file dir)
+      ;; Use a relative name so we can still find FILE if DIR is moved.
+      `(file-name-concat (file-name-directory load-file-name)
+                         ,(file-relative-name file dir))
+    file))
+
 (defun package-quickstart-refresh ()
   "(Re)Generate the `package-quickstart-file'."
   (interactive)
@@ -4605,7 +4613,8 @@ package-quickstart-refresh
         ;; aren't truncated.
         (print-length nil)
         (print-level nil)
-        (Info-directory-list '("")))
+        (Info-directory-list '(""))
+        (qs-dir (file-name-directory (expand-file-name package-quickstart-file))))
     (dolist (elt package-alist)
       (condition-case err
           (package-activate (car elt))
@@ -4622,7 +4631,7 @@ package-quickstart-refresh
                 ;; Prefer uncompiled files (and don't accept .so files).
                 (let ((load-suffixes '(".el" ".elc")))
                   (locate-library (package--autoloads-file-name pkg))))
-               (pfile (prin1-to-string file)))
+               (pfile (prin1-to-string (package--quickstart-rel qs-dir file))))
           (insert "(let* ((load-file-name " pfile ")\
 \(load-true-file-name load-file-name))\n")
           (insert-file-contents file)
@@ -4638,12 +4647,15 @@ package-quickstart-refresh
                   (append ',(mapcar #'package-desc-name package--quickstart-pkgs)
                           package-activated-list)))
           (current-buffer))
-      (let ((info-dirs (butlast Info-directory-list)))
+      (let ((info-dirs
+             (mapcar (lambda (info-dir)
+                       (package--quickstart-rel qs-dir info-dir))
+                     (butlast Info-directory-list))))
         (when info-dirs
           (pp `(progn (require 'info)
                       (info-initialize)
                       (setq Info-directory-list
-                            (append ',info-dirs Info-directory-list)))
+                            (append (list . ,info-dirs) Info-directory-list)))
               (current-buffer))))
       ;; Use `\s' instead of a space character, so this code chunk is not
       ;; mistaken for an actual file-local section of package.el.
-- 
2.39.3


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#77468; Package emacs. (Thu, 03 Apr 2025 13:25:04 GMT) Full text and rfc822 format available.

Message #29 received at 77468 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: monnier <at> iro.umontreal.ca, 77468 <at> debbugs.gnu.org
Subject: Re: bug#77468: 30.1.50; package-quickstart file is not relocatable
Date: Thu, 03 Apr 2025 09:24:32 -0400
Eli Zaretskii <eliz <at> gnu.org> writes:
>> Cc: 77468 <at> debbugs.gnu.org
>> Date: Wed, 02 Apr 2025 17:22:22 -0400
>> From:  Spencer Baugh via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>> 
>> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> >> Maybe package-quickstart.el could use relative names to packages in
>> >> package-user-dir (since it itself lives in package-user-dir), and
>> >> absolute names to packages outside that?
>> >
>> > Sounds OK.  Tho maybe we should check `file*-in-directory-p` (or the
>> > presence of "../" in the output of `file-relative-name`) rather than
>> > distinguishing based on `package-user-dir` vs `package-directory-list`?
>> >
>> > In any case, it sounds like a good compromise.
>> 
>> Makes sense.  How about this?
>
> Thanks, but please always remember to comment such non-trivial code,
> explaining its purpose and the problems it attempts to solve.
> Otherwise we make Emacs much harder to maintain for those who come
> after us.

Added a comment in the new version.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#77468; Package emacs. (Fri, 04 Apr 2025 22:55:02 GMT) Full text and rfc822 format available.

Message #32 received at 77468 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 77468 <at> debbugs.gnu.org
Subject: Re: bug#77468: 30.1.50; package-quickstart file is not relocatable
Date: Fri, 04 Apr 2025 18:54:23 -0400
>> Also, could we compute (file-name-directory load-file-name)
>> once at the beginning of the quickstart file instead of once per
>> inlined file?
> Done.

I don't see it:

> +(defun package--quickstart-rel (dir file)
> +  "Return an expression which evaluates to FILE while loading a file in DIR."
> +  (if (file-in-directory-p file dir)
> +      ;; Use a relative name so we can still find FILE if DIR is moved.
> +      `(file-name-concat (file-name-directory load-file-name)
> +                         ,(file-relative-name file dir))
> +    file))

We still generate code that computes `(file-name-directory
load-file-name)` N times, AFAICT.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#77468; Package emacs. (Mon, 07 Apr 2025 17:03:02 GMT) Full text and rfc822 format available.

Message #35 received at 77468 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 77468 <at> debbugs.gnu.org
Subject: Re: bug#77468: 30.1.50; package-quickstart file is not relocatable
Date: Mon, 07 Apr 2025 13:02:41 -0400
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>>> Also, could we compute (file-name-directory load-file-name)
>>> once at the beginning of the quickstart file instead of once per
>>> inlined file?
>> Done.
>
> I don't see it:
>
>> +(defun package--quickstart-rel (dir file)
>> +  "Return an expression which evaluates to FILE while loading a file in DIR."
>> +  (if (file-in-directory-p file dir)
>> +      ;; Use a relative name so we can still find FILE if DIR is moved.
>> +      `(file-name-concat (file-name-directory load-file-name)
>> +                         ,(file-relative-name file dir))
>> +    file))
>
> We still generate code that computes `(file-name-directory
> load-file-name)` N times, AFAICT.

Oh, I misunderstood what you were asking for, I see now.

I guess file-name-directory can be slow due to find-file-name-handler...
that's sad since it's otherwise just a trivial string operation.

I guess I could wrap the entire file in a let binding, but that's mildly
annoying and ugly...

How about putting the let binding outside the file?  Like this:

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index b9a8dacab15..d65d3baa7fa 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1699,7 +1699,10 @@ package-activate-all
   (let* ((elc (concat package-quickstart-file "c"))
          (qs (if (file-readable-p elc) elc
                (if (file-readable-p package-quickstart-file)
-                   package-quickstart-file))))
+                   package-quickstart-file)))
+         (default-directory
+          ;; For expressions produced by `package--quickstart-rel'.
+          (file-name-directory (expand-file-name package-quickstart-file))))
     ;; The quickstart file presumes that it has a blank slate,
     ;; so don't use it if we already activated some packages.
     (or (and qs (not (bound-and-true-p package-activated-list))
@@ -4591,6 +4594,16 @@ package--quickstart-maybe-refresh
     (delete-file (concat package-quickstart-file "c"))
     (delete-file package-quickstart-file)))
 
+(defun package--quickstart-rel (file)
+  "Return an expression which evaluates to FILE.
+
+If FILE is in `default-directory', returns an expression that is
+relative to `default-directory', so if that directory is moved we can
+still find FILE."
+  (if (file-in-directory-p file default-directory)
+      `(file-name-concat default-directory ,(file-relative-name file))
+    file))
+
 (defun package-quickstart-refresh ()
   "(Re)Generate the `package-quickstart-file'."
   (interactive)
@@ -4605,7 +4618,10 @@ package-quickstart-refresh
         ;; aren't truncated.
         (print-length nil)
         (print-level nil)
-        (Info-directory-list '("")))
+        (Info-directory-list '(""))
+        (default-directory
+         ;; So `package--quickstart-rel' produces relative expressions.
+         (file-name-directory (expand-file-name package-quickstart-file))))
     (dolist (elt package-alist)
       (condition-case err
           (package-activate (car elt))
@@ -4622,7 +4638,7 @@ package-quickstart-refresh
                 ;; Prefer uncompiled files (and don't accept .so files).
                 (let ((load-suffixes '(".el" ".elc")))
                   (locate-library (package--autoloads-file-name pkg))))
-               (pfile (prin1-to-string file)))
+               (pfile (prin1-to-string (package--quickstart-rel file))))
           (insert "(let* ((load-file-name " pfile ")\
 \(load-true-file-name load-file-name))\n")
           (insert-file-contents file)
@@ -4638,12 +4654,13 @@ package-quickstart-refresh
                   (append ',(mapcar #'package-desc-name package--quickstart-pkgs)
                           package-activated-list)))
           (current-buffer))
-      (let ((info-dirs (butlast Info-directory-list)))
+      (let ((info-dirs
+             (mapcar #'package--quickstart-rel (butlast Info-directory-list))))
         (when info-dirs
           (pp `(progn (require 'info)
                       (info-initialize)
                       (setq Info-directory-list
-                            (append ',info-dirs Info-directory-list)))
+                            (append (list . ,info-dirs) Info-directory-list)))
               (current-buffer))))
       ;; Use `\s' instead of a space character, so this code chunk is not
       ;; mistaken for an actual file-local section of package.el.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#77468; Package emacs. (Mon, 07 Apr 2025 17:12:02 GMT) Full text and rfc822 format available.

Message #38 received at 77468 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 77468 <at> debbugs.gnu.org
Subject: Re: bug#77468: 30.1.50; package-quickstart file is not relocatable
Date: Mon, 07 Apr 2025 13:10:52 -0400
> I guess file-name-directory can be slow due to find-file-name-handler...
> that's sad since it's otherwise just a trivial string operation.

It's not a major performance issue, but I can't think of a reason why we
wouldn't want to make this trivial optimization.

> I guess I could wrap the entire file in a let binding, but that's mildly
> annoying and ugly...

We can just `setq` a private global var.  It's conceptually
ugly, but it gets the job done.  As long as we support only a single
global `package-quickstart.el` that should be good enough.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#77468; Package emacs. (Mon, 07 Apr 2025 17:31:02 GMT) Full text and rfc822 format available.

Message #41 received at 77468 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 77468 <at> debbugs.gnu.org
Subject: Re: bug#77468: 30.1.50; package-quickstart file is not relocatable
Date: Mon, 07 Apr 2025 13:29:54 -0400
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> I guess file-name-directory can be slow due to find-file-name-handler...
>> that's sad since it's otherwise just a trivial string operation.
>
> It's not a major performance issue, but I can't think of a reason why we
> wouldn't want to make this trivial optimization.

Agree, agreed.  Just thinking about whether there's possibilities for
further optimization (like, do we really need to have non-nil
find-file-name-handler in the quickstart?).

>> I guess I could wrap the entire file in a let binding, but that's mildly
>> annoying and ugly...
>
> We can just `setq` a private global var.  It's conceptually
> ugly, but it gets the job done.  As long as we support only a single
> global `package-quickstart.el` that should be good enough.

I guess you prefer this to the approach in the patch I sent, where I
let-bind default-directory outside the load of package-quickstart-file?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#77468; Package emacs. (Mon, 07 Apr 2025 17:33:03 GMT) Full text and rfc822 format available.

Message #44 received at 77468 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 77468 <at> debbugs.gnu.org
Subject: Re: bug#77468: 30.1.50; package-quickstart file is not relocatable
Date: Mon, 07 Apr 2025 13:31:59 -0400
>> We can just `setq` a private global var.  It's conceptually
>> ugly, but it gets the job done.  As long as we support only a single
>> global `package-quickstart.el` that should be good enough.
> I guess you prefer this to the approach in the patch I sent, where I
> let-bind default-directory outside the load of package-quickstart-file?

Your approach is cleaner but I'm just worried that someone out there
loads the package-quickstart file "by hand".

Maybe we can do both (and emit a warning if the `setq` is not
redundant)?


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#77468; Package emacs. (Tue, 08 Apr 2025 18:31:02 GMT) Full text and rfc822 format available.

Message #47 received at 77468 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 77468 <at> debbugs.gnu.org
Subject: Re: bug#77468: 30.1.50; package-quickstart file is not relocatable
Date: Tue, 08 Apr 2025 14:30:26 -0400
[Message part 1 (text/plain, inline)]
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>>> We can just `setq` a private global var.  It's conceptually
>>> ugly, but it gets the job done.  As long as we support only a single
>>> global `package-quickstart.el` that should be good enough.
>> I guess you prefer this to the approach in the patch I sent, where I
>> let-bind default-directory outside the load of package-quickstart-file?
>
> Your approach is cleaner but I'm just worried that someone out there
> loads the package-quickstart file "by hand".

Ah, good point.  I'll just do the setq, see patch:

[0001-Use-relative-names-where-possible-in-package-quickst.patch (text/x-patch, inline)]
From 4699b39c629433e295988a1695261ce3d56400ea Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Wed, 2 Apr 2025 17:21:24 -0400
Subject: [PATCH] Use relative names where possible in package-quickstart.el

package-quickstart.el hardcodes many absolute file names, which
makes it break if user-emacs-directory moves, or in other
situations.  To be slightly more robust to this, use relative
file names to packages that are located in the same directory as
package-quickstart.el.

* lisp/emacs-lisp/package.el (package--quickstart-dir,
package--quickstart-rel): Add.
(package-quickstart-refresh): Use package--quickstart-rel on
file names.  (bug#77468)
---
 lisp/emacs-lisp/package.el | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index b9a8dacab15..bec44a6b637 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -4591,6 +4591,19 @@ package--quickstart-maybe-refresh
     (delete-file (concat package-quickstart-file "c"))
     (delete-file package-quickstart-file)))
 
+(defvar package--quickstart-dir nil
+  "Set by `package-quickstart-file' to the directory containing it.")
+
+(defun package--quickstart-rel (file)
+  "Return an expr depending on `package--quickstart-dir' which evaluates to FILE.
+
+If FILE is in `package--quickstart-dir', returns an expression that is
+relative to that directory, so if that directory is moved we can still
+find FILE."
+  (if (file-in-directory-p file package--quickstart-dir)
+      `(file-name-concat package--quickstart-dir ,(file-relative-name file package--quickstart-dir))
+    file))
+
 (defun package-quickstart-refresh ()
   "(Re)Generate the `package-quickstart-file'."
   (interactive)
@@ -4605,7 +4618,8 @@ package-quickstart-refresh
         ;; aren't truncated.
         (print-length nil)
         (print-level nil)
-        (Info-directory-list '("")))
+        (Info-directory-list '(""))
+        (package--quickstart-dir nil))
     (dolist (elt package-alist)
       (condition-case err
           (package-activate (car elt))
@@ -4617,12 +4631,16 @@ package-quickstart-refresh
       (emacs-lisp-mode)                 ;For `syntax-ppss'.
       (insert ";;; Quickstart file to activate all packages at startup  -*- lexical-binding:t -*-\n")
       (insert ";; ¡¡ This file is autogenerated by `package-quickstart-refresh', DO NOT EDIT !!\n\n")
+      (setq package--quickstart-dir
+            (file-name-directory (expand-file-name package-quickstart-file)))
+      (insert (pp '(setq package--quickstart-dir
+                         (file-name-directory (expand-file-name load-file-name)))))
       (dolist (pkg package--quickstart-pkgs)
         (let* ((file
                 ;; Prefer uncompiled files (and don't accept .so files).
                 (let ((load-suffixes '(".el" ".elc")))
                   (locate-library (package--autoloads-file-name pkg))))
-               (pfile (prin1-to-string file)))
+               (pfile (prin1-to-string (package--quickstart-rel file))))
           (insert "(let* ((load-file-name " pfile ")\
 \(load-true-file-name load-file-name))\n")
           (insert-file-contents file)
@@ -4638,12 +4656,13 @@ package-quickstart-refresh
                   (append ',(mapcar #'package-desc-name package--quickstart-pkgs)
                           package-activated-list)))
           (current-buffer))
-      (let ((info-dirs (butlast Info-directory-list)))
+      (let ((info-dirs
+             (mapcar #'package--quickstart-rel (butlast Info-directory-list))))
         (when info-dirs
           (pp `(progn (require 'info)
                       (info-initialize)
                       (setq Info-directory-list
-                            (append ',info-dirs Info-directory-list)))
+                            (append (list . ,info-dirs) Info-directory-list)))
               (current-buffer))))
       ;; Use `\s' instead of a space character, so this code chunk is not
       ;; mistaken for an actual file-local section of package.el.
-- 
2.39.3


Reply sent to Stefan Monnier <monnier <at> iro.umontreal.ca>:
You have taken responsibility. (Tue, 08 Apr 2025 18:41:02 GMT) Full text and rfc822 format available.

Notification sent to Spencer Baugh <sbaugh <at> janestreet.com>:
bug acknowledged by developer. (Tue, 08 Apr 2025 18:41:02 GMT) Full text and rfc822 format available.

Message #52 received at 77468-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 77468-done <at> debbugs.gnu.org
Subject: Re: bug#77468: 30.1.50; package-quickstart file is not relocatable
Date: Tue, 08 Apr 2025 14:40:46 -0400
>> Your approach is cleaner but I'm just worried that someone out there
>> loads the package-quickstart file "by hand".
>
> Ah, good point.  I'll just do the setq, see patch:

Thanks, pushed to `master`,


        Stefan





bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 07 May 2025 11:24:22 GMT) Full text and rfc822 format available.

This bug report was last modified 101 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.