GNU bug report logs - #62767
29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after info package

Previous Next

To reply to this bug, email your comments to 62767 AT debbugs.gnu.org.

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

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

From: lin Sun <sunlin7 <at> yahoo.com>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set variables after
 info package
Date: Tue, 11 Apr 2023 05:15:45 +0000

[Message part 1 (text/plain, inline)]

Hi,

The package.el will always require the entire `info.el` on its package
activating function.
Using the `with-eval-after-load` to arrange the work can avoid the
unnecessary loading the `info` package.

Please help review the patch. Thanks.

Best regards
Lin

[0001-lisp-emacs-lisp-package.el-set-variables-after-info-.patch (text/x-patch, attachment)]

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: lin Sun <sunlin7 <at> yahoo.com>
Cc: 62767 <at> debbugs.gnu.org, eliz <at> gnu.org
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Fri, 14 Apr 2023 20:07:54 +0000

[Message part 1 (text/plain, inline)]

lin Sun <sunlin7 <at> yahoo.com> writes:

> Hi,
>
> The package.el will always require the entire `info.el` on its package
> activating function.
> Using the `with-eval-after-load` to arrange the work can avoid the
> unnecessary loading the `info` package.

Note that this is not the only place where info is required.  If this is
an acceptable solution (which I am not sure since `with-eval-after-load'
is something I usually encounter in user configurations), then we should
think about consistently addressing the issue:

[Message part 2 (text/plain, inline)]

3 matches for "require 'info" in buffer: package.el
       :        (add-to-list 'load-path (directory-file-name pkg-dir)))
       :      ;; Add info node.
       :      (when (file-exists-p (expand-file-name "dir" pkg-dir))
       :        ;; FIXME: not the friendliest, but simple.
    896:        (require 'info)
       :        (info-initialize)
       :        (add-to-list 'Info-directory-list pkg-dir))
       :      (push name package-activated-list)
       :      ;; Don't return nil.
-------
       :(defun package-quickstart-refresh ()
       :  "(Re)Generate the `package-quickstart-file'."
       :  (interactive)
       :  (package-initialize 'no-activate)
   4444:  (require 'info)
       :  (let ((package--quickstart-pkgs ())
       :        ;; Pretend we haven't activated anything yet!
       :        (package-activated-list ())
       :        ;; Make sure we can load this file without load-source-file-function.
-------
       :                          package-activated-list)))
       :          (current-buffer))
       :      (let ((info-dirs (butlast Info-directory-list)))
       :        (when info-dirs
   4489:          (pp `(progn (require 'info)
       :                      (info-initialize)
       :                      (setq Info-directory-list
       :                            (append ',info-dirs Info-directory-list)))
       :              (current-buffer))))

[Message part 3 (text/plain, inline)]

> Please help review the patch. Thanks.
>
> Best regards
> Lin
>
> From 4908999eefa315cfa1a1434baf891ce98ee8d871 Mon Sep 17 00:00:00 2001
> From: Lin Sun <sunlin7 <at> hotmail.com>
> Date: Tue, 11 Apr 2023 00:00:13 +0000
> Subject: [PATCH] *lisp/emacs-lisp/package.el: set variables after info package
>  be loaded
>
> ---
>  lisp/emacs-lisp/package.el | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index f92afe56b7..bb0491e509 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -892,10 +892,9 @@ package-activate-1
>          (add-to-list 'load-path (directory-file-name pkg-dir)))
>        ;; Add info node.
>        (when (file-exists-p (expand-file-name "dir" pkg-dir))
> -        ;; FIXME: not the friendliest, but simple.
> -        (require 'info)
> -        (info-initialize)
> -        (add-to-list 'Info-directory-list pkg-dir))
> +        (with-eval-after-load 'info
> +          (info-initialize)
> +          (add-to-list 'Info-directory-list pkg-dir)))
>        (push name package-activated-list)
>        ;; Don't return nil.
>        t)))

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

From: lin Sun <sunlin7 <at> yahoo.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 62767 <at> debbugs.gnu.org, eliz <at> gnu.org
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Fri, 14 Apr 2023 22:12:48 +0000

Hi Philip,

Thanks for the comment.

I'm tracking the startup performance issue on my local, and found the
line 896, after change to the patched lines, it works for me several
weeks without regression.

I'll check other lines you mentioned, thanks

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 62767 <at> debbugs.gnu.org, sunlin7 <at> yahoo.com
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Sat, 15 Apr 2023 12:15:29 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: 62767 <at> debbugs.gnu.org,  eliz <at> gnu.org
> Date: Fri, 14 Apr 2023 20:07:54 +0000
> 
> lin Sun <sunlin7 <at> yahoo.com> writes:
> 
> > Hi,
> >
> > The package.el will always require the entire `info.el` on its package
> > activating function.
> > Using the `with-eval-after-load` to arrange the work can avoid the
> > unnecessary loading the `info` package.
> 
> Note that this is not the only place where info is required.  If this is
> an acceptable solution (which I am not sure since `with-eval-after-load'
> is something I usually encounter in user configurations), then we should
> think about consistently addressing the issue:

What exactly are the problems with requiring info.el?  We require
packages that define some function in gazillion other places, so what
makes this one case special?

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 62767 <at> debbugs.gnu.org, sunlin7 <at> yahoo.com
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Sat, 15 Apr 2023 10:43:21 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: 62767 <at> debbugs.gnu.org,  eliz <at> gnu.org
>> Date: Fri, 14 Apr 2023 20:07:54 +0000
>> 
>> lin Sun <sunlin7 <at> yahoo.com> writes:
>> 
>> > Hi,
>> >
>> > The package.el will always require the entire `info.el` on its package
>> > activating function.
>> > Using the `with-eval-after-load` to arrange the work can avoid the
>> > unnecessary loading the `info` package.
>> 
>> Note that this is not the only place where info is required.  If this is
>> an acceptable solution (which I am not sure since `with-eval-after-load'
>> is something I usually encounter in user configurations), then we should
>> think about consistently addressing the issue:
>
> What exactly are the problems with requiring info.el?  We require
> packages that define some function in gazillion other places, so what
> makes this one case special?

Some people optimise the startup time of Emacs and want to avoid loading
files that are not necessary for Emacs to start-up.

-- 
Philip Kaludercic

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 62767 <at> debbugs.gnu.org, sunlin7 <at> yahoo.com
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Sat, 15 Apr 2023 13:58:05 +0300

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: sunlin7 <at> yahoo.com,  62767 <at> debbugs.gnu.org
> Date: Sat, 15 Apr 2023 10:43:21 +0000
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Philip Kaludercic <philipk <at> posteo.net>
> >> Cc: 62767 <at> debbugs.gnu.org,  eliz <at> gnu.org
> >> Date: Fri, 14 Apr 2023 20:07:54 +0000
> >> 
> >> lin Sun <sunlin7 <at> yahoo.com> writes:
> >> 
> >> > Hi,
> >> >
> >> > The package.el will always require the entire `info.el` on its package
> >> > activating function.
> >> > Using the `with-eval-after-load` to arrange the work can avoid the
> >> > unnecessary loading the `info` package.
> >> 
> >> Note that this is not the only place where info is required.  If this is
> >> an acceptable solution (which I am not sure since `with-eval-after-load'
> >> is something I usually encounter in user configurations), then we should
> >> think about consistently addressing the issue:
> >
> > What exactly are the problems with requiring info.el?  We require
> > packages that define some function in gazillion other places, so what
> > makes this one case special?
> 
> Some people optimise the startup time of Emacs and want to avoid loading
> files that are not necessary for Emacs to start-up.

And this is the only place in Emacs where we have (require 'FOO) and
FOO is not necessary for Emacs to start up?  I'd be surprised.

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

From: lin Sun <sunlin7 <at> yahoo.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 62767 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Mon, 17 Apr 2023 06:13:16 +0000

[Message part 1 (text/plain, inline)]

Hi Philip,

I had researched these three calls with the "require 'info", and
updated the patch for better performance.
The 1 th and 3rd can be rewritten with the new form, but the 2nd one
is necessary, and should not be changed.
Please help review the new attached patch file. Thanks.

Hi Eli,
> And this is the only place in Emacs where we have (require 'FOO) and
> FOO is not necessary for Emacs to start up?
My configuration will load the package.el at startup, so I tracked the
initial packages, and found the "require `info" will take ~1 seconds
on my old PC.
I didn't pay attention to other packages, if I found a heavy one, I'll
post it on the mail list.

[0001-lisp-emacs-lisp-package.el-better-way-to-load-the-in.patch (text/x-patch, attachment)]

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

From: Ruijie Yu <ruijie <at> netyu.xyz>
To: lin Sun <sunlin7 <at> yahoo.com>
Cc: 62767 <at> debbugs.gnu.org, Philip Kaludercic <philipk <at> posteo.net>,
 bug-gnu-emacs <at> gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Mon, 17 Apr 2023 14:53:50 +0800

lin Sun via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org> writes:

> Hi Philip,
>
> I had researched these three calls with the "require 'info", and
> updated the patch for better performance.
> The 1 th and 3rd can be rewritten with the new form, but the 2nd one
> is necessary, and should not be changed.

> @@ -4427,7 +4426,7 @@ package-quickstart-refresh
>    "(Re)Generate the `package-quickstart-file'."
>    (interactive)
>    (package-initialize 'no-activate)
> -  (require 'info)
> +  (require 'info) ; insure Info-directory-list avaliable for package-activate

"insure" -> "ensure"?

>    (let ((package--quickstart-pkgs ())
>          ;; Pretend we haven't activated anything yet!
>          (package-activated-list ())
> @@ -4472,10 +4471,10 @@ package-quickstart-refresh
>            (current-buffer))
>        (let ((info-dirs (butlast Info-directory-list)))
>          (when info-dirs
> -          (pp `(progn (require 'info)
> -                      (info-initialize)
> -                      (setq Info-directory-list
> -                            (append ',info-dirs Info-directory-list)))
> +          (pp `(with-eval-after-load 'info
> +                 (info-initialize)
> +                 (setq Info-directory-list
> +                       (append ',info-dirs Info-directory-list)))
>                (current-buffer))))

I did some testing, and noticed that `with-eval-after-load' returns nil
to its caller when the feature is not loaded.  That is, when `info' is
not already loaded, you are effectively calling (pp nil).  This is not
feature-equivalent to the original code, so this might require the
`require' form.

-- 
Best,


RY

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

From: Ruijie Yu <ruijie <at> netyu.xyz>
To: Ruijie Yu <ruijie <at> netyu.xyz>
Cc: 62767 <at> debbugs.gnu.org, lin Sun <sunlin7 <at> yahoo.com>,
 Eli Zaretskii <eliz <at> gnu.org>, Philip Kaludercic <philipk <at> posteo.net>,
 bug-gnu-emacs <at> gnu.org
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Mon, 17 Apr 2023 14:59:00 +0800

Ruijie Yu <ruijie <at> netyu.xyz> writes:

> lin Sun via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org> writes:
>
>>    (let ((package--quickstart-pkgs ())
>>          ;; Pretend we haven't activated anything yet!
>>          (package-activated-list ())
>> @@ -4472,10 +4471,10 @@ package-quickstart-refresh
>>            (current-buffer))
>>        (let ((info-dirs (butlast Info-directory-list)))
>>          (when info-dirs
>> -          (pp `(progn (require 'info)
>> -                      (info-initialize)
>> -                      (setq Info-directory-list
>> -                            (append ',info-dirs Info-directory-list)))
>> +          (pp `(with-eval-after-load 'info
>> +                 (info-initialize)
>> +                 (setq Info-directory-list
>> +                       (append ',info-dirs Info-directory-list)))
>>                (current-buffer))))
>
> I did some testing, and noticed that `with-eval-after-load' returns nil
> to its caller when the feature is not loaded.  That is, when `info' is
> not already loaded, you are effectively calling (pp nil).  This is not
> feature-equivalent to the original code, so this might require the
> `require' form.

Sorry, I misread the code.  Please ignore this portion of my previous
email.

-- 
Best,


RY

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: lin Sun <sunlin7 <at> yahoo.com>
Cc: 62767 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Mon, 17 Apr 2023 13:24:56 +0000

lin Sun <sunlin7 <at> yahoo.com> writes:

> Hi Philip,
>
> I had researched these three calls with the "require 'info", and
> updated the patch for better performance.
> The 1 th and 3rd can be rewritten with the new form, 

OK, then we still have to find out if these changes are safe.  As
mentioned before, I hesitate in using `with-eval-after-load', probably
because of the complicated history of `eval-after-load'.  I suppose that
Stefan might know more (as it is due to him that I am careful here), so
I have added him to the thread.

>                                                      but the 2nd one
> is necessary, and should not be changed.

How did you come to the conclusion about the second one?

> Please help review the new attached patch file. Thanks.
>
> Hi Eli,
>> And this is the only place in Emacs where we have (require 'FOO) and
>> FOO is not necessary for Emacs to start up?
> My configuration will load the package.el at startup, so I tracked the
> initial packages, and found the "require `info" will take ~1 seconds
> on my old PC.
> I didn't pay attention to other packages, if I found a heavy one, I'll
> post it on the mail list.
>
> From 91a4bdbccc6f3210f43f46891077c57afd128830 Mon Sep 17 00:00:00 2001
> From: Lin Sun <sunlin7 <at> hotmail.com>
> Date: Tue, 11 Apr 2023 00:00:13 +0000
> Subject: [PATCH] *lisp/emacs-lisp/package.el: better way to load the info
>  package
>
> ---
>  lisp/emacs-lisp/package.el | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index f92afe56b7..98153a49c7 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -892,10 +892,9 @@ package-activate-1
>          (add-to-list 'load-path (directory-file-name pkg-dir)))
>        ;; Add info node.
>        (when (file-exists-p (expand-file-name "dir" pkg-dir))
> -        ;; FIXME: not the friendliest, but simple.
> -        (require 'info)
> -        (info-initialize)
> -        (add-to-list 'Info-directory-list pkg-dir))
> +        (with-eval-after-load 'info
> +          (info-initialize)
> +          (add-to-list 'Info-directory-list pkg-dir)))
>        (push name package-activated-list)
>        ;; Don't return nil.
>        t)))
> @@ -4427,7 +4426,7 @@ package-quickstart-refresh
>    "(Re)Generate the `package-quickstart-file'."
>    (interactive)
>    (package-initialize 'no-activate)
> -  (require 'info)
> +  (require 'info) ; insure Info-directory-list avaliable for package-activate
>    (let ((package--quickstart-pkgs ())
>          ;; Pretend we haven't activated anything yet!
>          (package-activated-list ())
> @@ -4472,10 +4471,10 @@ package-quickstart-refresh
>            (current-buffer))
>        (let ((info-dirs (butlast Info-directory-list)))
>          (when info-dirs
> -          (pp `(progn (require 'info)
> -                      (info-initialize)
> -                      (setq Info-directory-list
> -                            (append ',info-dirs Info-directory-list)))
> +          (pp `(with-eval-after-load 'info
> +                 (info-initialize)
> +                 (setq Info-directory-list
> +                       (append ',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.

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

From: lin Sun <sunlin7 <at> yahoo.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 62767 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Mon, 17 Apr 2023 15:27:09 +0000

[Message part 1 (text/plain, inline)]

Hi Philip,

> How did you come to the conclusion about the second one?
The 2nd one is necessary because the following lines will use the full
`Info-directory-list` to print its values into the
package-quickstart.el, whose value was modified by the function
`package-activate'.
If I comment out or use the `eval-after-load` on the 2nd one, the
`Info-directory-list` will be empty, and missed in the
package-quickstart.el file.

Hi Ruijie,

> "insure" -> "ensure"?
You're right.
I had fixed my typo in the patch file (only typo in comment), and
attached the new one, please help review again. Thanks

[0001-lisp-emacs-lisp-package.el-better-way-to-load-the-in.patch (text/x-patch, attachment)]

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: lin Sun <sunlin7 <at> yahoo.com>
Cc: 62767 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Mon, 17 Apr 2023 16:03:56 +0000

lin Sun <sunlin7 <at> yahoo.com> writes:

> Hi Philip,
>
>> How did you come to the conclusion about the second one?
> The 2nd one is necessary because the following lines will use the full
> `Info-directory-list` to print its values into the
> package-quickstart.el, whose value was modified by the function
> `package-activate'.
> If I comment out or use the `eval-after-load` on the 2nd one, the
> `Info-directory-list` will be empty, and missed in the
> package-quickstart.el file.

OK, I get it now.  I was confused because it seemed as though
`Info-directory-list' could just be bound and declared using a defvar
before the function, but if we do not load info then the
with-eval-after-load blocks in `package-activate-1' that are invoked for
the side effect of manipulating `Info-directory-list' do not get
evaluated, which is why we see that the value is empty.

But as the function `package-quickstart-refresh' is not invoked on
startup, but as `package-quickstart' says

    This requires the use of `package-quickstart-refresh' every time the
    activations need to be changed, such as when `package-load-list' is
    modified.

This should be fine.

> Hi Ruijie,
>
>> "insure" -> "ensure"?
> You're right.
> I had fixed my typo in the patch file (only typo in comment), and
> attached the new one, please help review again. Thanks

Again, assuming that there is no issue with `with-eval-after-load', the
code should be fine.  The commit message looks a bit distorted though?
Did you generate it using C-c C-w?  I get this template when I apply
your commit:

* lisp/emacs-lisp/package.el (package-activate-1):
(package-quickstart-refresh):

Also, it would make sense to motivate the changes in more detail.  What
do the changes improve?

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 62767 <at> debbugs.gnu.org, lin Sun <sunlin7 <at> yahoo.com>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Mon, 17 Apr 2023 12:30:19 -0400

> OK, then we still have to find out if these changes are safe.  As
> mentioned before, I hesitate in using `with-eval-after-load', probably
> because of the complicated history of `eval-after-load'.  I suppose that
> Stefan might know more (as it is due to him that I am careful here), so
> I have added him to the thread.

The problem with eval-after-load is not the history but the fact that
it has various corner-case caveats.

(with-eval-after-load "compile" ...) for example makes it unclear
exactly when it should be executed, e.g.:

   (load "compile")                          => yes clearly.
   (load "compile.el")                       => probably, yes.
   (load "srecode/compile.el")               => probably not.
   (load "/foo/bar/lisp/compile.el")         => hmm... yeah, I think so.
   (load "/foo/bar/lisp/srecode/compile.el") => hmm... nah, maybe not.

The next game is to try and guess what the code does, to see when it
matches expectations.

The use (with-eval-after-load 'info ...) is much better in this respect
because it's not linked to the file name but to the feature name, which
is precise.

But there's still the issue of exactly when this is executed.  Should it
be executed just the next time the file is loaded (usually what's
wanted), or every other time after that as well (what actually happens)?

Also the implementation for the good case where we provide a feature
name rather than a file name is a bit roundabout/fiddly.  This is an
internal problem, so any bug there can be fixed, thus it's probably not
a good reason not to use it, but it still makes me prefer to avoid it :-)

I wish there was an easier way to add to `Info-directory-list`.
That variable is set in a fairly delicate manner, so I think Eli would
be better placed to come up with a better long term solution.
Maybe we should just add a (preloaded) `Info-extra-directory-list`?

In the mean time, Lin Sun's patch is probably "as good as it gets" if
you want to load `info.el` lazily (which I obviously didn't bother to do
when I wrote that code :-)


        Stefan

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: lin Sun <sunlin7 <at> yahoo.com>
Cc: 62767 <at> debbugs.gnu.org, philipk <at> posteo.net, monnier <at> iro.umontreal.ca
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Mon, 17 Apr 2023 20:25:43 +0300

> From: lin Sun <sunlin7 <at> yahoo.com>
> Date: Mon, 17 Apr 2023 15:27:09 +0000
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 62767 <at> debbugs.gnu.org, 
> 	Stefan Monnier <monnier <at> iro.umontreal.ca>
> 
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -892,10 +892,9 @@ package-activate-1
>          (add-to-list 'load-path (directory-file-name pkg-dir)))
>        ;; Add info node.
>        (when (file-exists-p (expand-file-name "dir" pkg-dir))
> -        ;; FIXME: not the friendliest, but simple.
> -        (require 'info)
> -        (info-initialize)
> -        (add-to-list 'Info-directory-list pkg-dir))
> +        (with-eval-after-load 'info
> +          (info-initialize)
> +          (add-to-list 'Info-directory-list pkg-dir)))
>        (push name package-activated-list)
>        ;; Don't return nil.
>        t)))
> @@ -4427,7 +4426,7 @@ package-quickstart-refresh
>    "(Re)Generate the `package-quickstart-file'."
>    (interactive)
>    (package-initialize 'no-activate)
> -  (require 'info)
> +  (require 'info) ; ensure Info-directory-list available for package-activate
>    (let ((package--quickstart-pkgs ())
>          ;; Pretend we haven't activated anything yet!
>          (package-activated-list ())
> @@ -4472,10 +4471,10 @@ package-quickstart-refresh
>            (current-buffer))
>        (let ((info-dirs (butlast Info-directory-list)))
>          (when info-dirs
> -          (pp `(progn (require 'info)
> -                      (info-initialize)
> -                      (setq Info-directory-list
> -                            (append ',info-dirs Info-directory-list)))
> +          (pp `(with-eval-after-load 'info
> +                 (info-initialize)
> +                 (setq Info-directory-list
> +                       (append ',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.20.5

I don't know yet whether I'm okay with this change, but if this does
go in, this tricky/unusual code must have comments explaining why on
Earth we do it this way, and only with info.el.

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

From: lin Sun <sunlin7 <at> yahoo.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 62767 <at> debbugs.gnu.org, Philip Kaludercic <philipk <at> posteo.net>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Wed, 19 Apr 2023 05:11:03 +0000

[Message part 1 (text/plain, inline)]

Hi Eli,

I had researched the `info.el', and found the variable
`Info-default-directory-list' is a more proper variable than the
original `Info-directory-list', which is an autoload customer
variable.

Then the patch can be simpler than the previous one, I attached the
new patch, it avoided to require 'info. I verified it on my local.

Please help review the change, thanks !

[0001-lisp-emacs-lisp-package.el-avoid-to-load-the-entire-.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: lin Sun <sunlin7 <at> yahoo.com>
Cc: 62767 <at> debbugs.gnu.org, philipk <at> posteo.net, monnier <at> iro.umontreal.ca
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Thu, 20 Apr 2023 12:12:29 +0300

> From: lin Sun <sunlin7 <at> yahoo.com>
> Date: Wed, 19 Apr 2023 05:11:03 +0000
> Cc: Philip Kaludercic <philipk <at> posteo.net>, Eli Zaretskii <eliz <at> gnu.org>, 62767 <at> debbugs.gnu.org
> 
> I had researched the `info.el', and found the variable
> `Info-default-directory-list' is a more proper variable than the
> original `Info-directory-list', which is an autoload customer
> variable.
> 
> Then the patch can be simpler than the previous one, I attached the
> new patch, it avoided to require 'info. I verified it on my local.

Thanks, but we cannot possibly change Info-default-directory-list
behind the user's back.  Info-default-directory-list is a user option,
so Emacs itself cannot modify its value.

Moreover, Info-default-directory-list is considered only once, when
info.el is loaded and initialized for the first time.  Whereas
package.el can be used for installing a package in the middle of a
running Emacs session, when info.el was already loaded, and so
modifying Info-default-directory-list will not have any effect until
the next restart of Emacs.  Do we require users to restart Emacs after
installing a package via package.el?

Going back to the original problem, AFAIU you wanted to avoid loading
info.el at startup, because that makes startup slower, is that right?
Then how about adding a new variable, Info-packages-directory-list,
say, to which package.el will add directories of the installed
packages, and info.el will use when it first loads?  Would that solve
the problem?

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

From: lin Sun <sunlin7 <at> yahoo.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 62767 <at> debbugs.gnu.org, Philip Kaludercic <philipk <at> posteo.net>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Thu, 20 Apr 2023 16:16:55 +0000

On Thu, 20 Apr 2023 12:12:29 +0300 Eli Zaretskii <eliz <at> gnu.org> wrote:
> Thanks, but we cannot possibly change Info-default-directory-list
> behind the user's back.  Info-default-directory-list is a user option,
> so Emacs itself cannot modify its value.
>
> Moreover, Info-default-directory-list is considered only once, when
> info.el is loaded and initialized for the first time.  Whereas
> package.el can be used for installing a package in the middle of a
> running Emacs session, when info.el was already loaded, and so
> modifying Info-default-directory-list will not have any effect until
> the next restart of Emacs.  Do we require users to restart Emacs after
> installing a package via package.el?
>
> Going back to the original problem, AFAIU you wanted to avoid loading
> info.el at startup, because that makes startup slower, is that right?
> Then how about adding a new variable, Info-packages-directory-list,
> say, to which package.el will add directories of the installed
> packages, and info.el will use when it first loads?  Would that solve
> the problem?
Totally, Info-default-directory-list is a customer variable, and it is
considered only once, not the perfect one.
And yes, the original motivation is to avoid loading info.el at startup.
With your comment, I'm going to search better way to avoid loading
info.el, thanks !

Best Regards
Lin

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

From: lin Sun <sunlin7 <at> yahoo.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 62767 <at> debbugs.gnu.org, Philip Kaludercic <philipk <at> posteo.net>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Tue, 23 May 2023 22:11:21 +0000

[Message part 1 (text/plain, inline)]

Hi Eli, Stefan,

I am experimenting with a different approach to avoid the loading of
the entire info.el file using package.el.
Instead, I am adding the directories to the `Info-directory-list',
which will be utilized by other functions responsible for loading the
specific info files.

Please help review the patch I attached. Thanks.

Best regards
Lin

[0001-avoid-to-load-the-entire-info-package.patch (text/x-patch, attachment)]

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

From: lin Sun <sunlin7 <at> yahoo.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Eli Zaretskii <eliz <at> gnu.org>, 
 Philip Kaludercic <philipk <at> posteo.net>, 62767 <at> debbugs.gnu.org
Subject: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Wed, 24 May 2023 02:53:08 +0000

[Message part 1 (text/plain, inline)]

> Hi Eli,
> I am experimenting with a different approach to avoid the loading of
> the entire info.el file using package.el.
> Instead, I am adding the directories to the `Info-directory-list',
> which will be utilized by other functions responsible for loading the
> specific info files.
> Please help review the patch I attached. Thanks.

[0001-avoid-to-load-the-entire-info-package-V2.patch (application/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: lin Sun <sunlin7 <at> yahoo.com>
Cc: 62767 <at> debbugs.gnu.org, philipk <at> posteo.net, monnier <at> iro.umontreal.ca
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Wed, 24 May 2023 14:32:41 +0300

> From: lin Sun <sunlin7 <at> yahoo.com>
> Date: Wed, 24 May 2023 02:53:08 +0000
> 
> > I am experimenting with a different approach to avoid the loading of
> > the entire info.el file using package.el.
> > Instead, I am adding the directories to the `Info-directory-list',
> > which will be utilized by other functions responsible for loading the
> > specific info files.
> > Please help review the patch I attached. Thanks.

Could you please describe how this makes sure Info-directory-list will
be set to a correct value, in the various scenarios that are relevant?

In particular, Info-directory-list is not the right variable to tweak
here, as it is computed by info.el.  I think we need a separate
variable.

> -      (let ((info-dirs (butlast Info-directory-list)))
> -        (when info-dirs
> -          (pp `(progn (require 'info)
> -                      (info-initialize)
> -                      (setq Info-directory-list
> -                            (append ',info-dirs Info-directory-list)))
> -              (current-buffer))))
> +      (when-let ((info-dirs (butlast Info-directory-list)))
> +        (pp `(defvar Info-directory-list '()) (current-buffer))
> +        (pp `(setq Info-directory-list
> +                   (delete-dups
> +                    (append ',info-dirs Info-directory-list)))
> +            (current-buffer)))

Using 'append' here could cause duplicate directories in
Info-directory-list.

> +(defvar Info--initialized nil
> +  "Non-nil if `info-initialize' has been run.")

There's no need to capitalize the first letter of the name of an
internal variable.  We capitalize 'I' in "Info" so that it would be
easier to type info.el commands with completion (other commands that
begin with 'i' use lower-case 'i').  This is not a factor for internal
variables.

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

From: lin Sun <sunlin7 <at> yahoo.com>
To: Eli Zaretskii <eliz <at> gnu.org>, monnier <at> iro.umontreal.ca, philipk <at> posteo.net
Cc: 62767 <at> debbugs.gnu.org
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Thu, 25 May 2023 00:46:58 +0000

[Message part 1 (text/plain, inline)]

On Wed, May 24, 2023 at 11:32 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> ...
> Could you please describe how this makes sure Info-directory-list will
> be set to a correct value, in the various scenarios that are relevant?
>
> In particular, Info-directory-list is not the right variable to tweak
> here, as it is computed by info.el.  I think we need a separate
> variable.
In the original function `info-initialize', it will rely on the value
`Info-directory-list' to indicate function initialization and parse
the "INFOPATH" or get system info dirs as the initialized value.

The bug comes for: package.el will add several paths into
`Info-directory-list', that maybe lead function `info-initialize'
return without parsing the "INFOPATH" or getting the system info dires
as initial value.

So package.el required the entire `info.el` and called function
`info-initialize' to initialize the value first (by parsing the
"INFOPATH" or get system info dir), then the package.el modify the
`Info-directory-list'.

The patch  will allow users to set the `Info-directory-list' before
calling function `info-initialize'; when info.el use the
`Info-directory-list' variable to read plain *.info file, will call
function `info-initialize' to add system info dirs.

Two functions `Info-insert-dir' and `Info-find-file', will use
(get/read) the value of `Info-directory-list' to read the plain *.info
files.
In the patch file, both functions will call the function
`info-initialize' at their entry to ensure the `Info-directory-list'
is initialized with the "INFOPATH" env string.
Users will just insert/remove dirs from/to `Info-directory-list', but
won't use the list to read/get plain *.info files.
So users can define the `Info-directory-list' and modify its value on
their elisp files.

> > +      (when-let ((info-dirs (butlast Info-directory-list)))
> > +        (pp `(defvar Info-directory-list '()) (current-buffer))
> > +        (pp `(setq Info-directory-list
> > +                   (delete-dups
> > +                    (append ',info-dirs Info-directory-list)))
> > +            (current-buffer)))
>
> Using 'append' here could cause duplicate directories in
> Info-directory-list.
The `delete-dups' will avoid that.

> > +(defvar Info--initialized nil
> > +  "Non-nil if `info-initialize' has been run.")
> There's no need to capitalize the first letter of the name of an
> internal variable.  We capitalize 'I' in "Info" so that it would be
> easier to type info.el commands with completion (other commands that
> begin with 'i' use lower-case 'i').  This is not a factor for internal
> variables.
Very helpful to understand the naming rules. And rename it to
`info--initialized', new patch attached, please review again.

Best Regards
Lin

[0001-avoid-to-load-the-entire-info-package-V3.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: lin Sun <sunlin7 <at> yahoo.com>
Cc: 62767 <at> debbugs.gnu.org, philipk <at> posteo.net, monnier <at> iro.umontreal.ca
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Fri, 26 May 2023 12:37:37 +0300

> From: lin Sun <sunlin7 <at> yahoo.com>
> Date: Thu, 25 May 2023 00:46:58 +0000
> Cc: 62767 <at> debbugs.gnu.org
> 
> On Wed, May 24, 2023 at 11:32 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> > ...
> > Could you please describe how this makes sure Info-directory-list will
> > be set to a correct value, in the various scenarios that are relevant?
> >
> > In particular, Info-directory-list is not the right variable to tweak
> > here, as it is computed by info.el.  I think we need a separate
> > variable.
> In the original function `info-initialize', it will rely on the value
> `Info-directory-list' to indicate function initialization and parse
> the "INFOPATH" or get system info dirs as the initialized value.
> 
> The bug comes for: package.el will add several paths into
> `Info-directory-list', that maybe lead function `info-initialize'
> return without parsing the "INFOPATH" or getting the system info dires
> as initial value.
> 
> So package.el required the entire `info.el` and called function
> `info-initialize' to initialize the value first (by parsing the
> "INFOPATH" or get system info dir), then the package.el modify the
> `Info-directory-list'.
> 
> The patch  will allow users to set the `Info-directory-list' before
> calling function `info-initialize'; when info.el use the
> `Info-directory-list' variable to read plain *.info file, will call
> function `info-initialize' to add system info dirs.
> 
> Two functions `Info-insert-dir' and `Info-find-file', will use
> (get/read) the value of `Info-directory-list' to read the plain *.info
> files.
> In the patch file, both functions will call the function
> `info-initialize' at their entry to ensure the `Info-directory-list'
> is initialized with the "INFOPATH" env string.
> Users will just insert/remove dirs from/to `Info-directory-list', but
> won't use the list to read/get plain *.info files.
> So users can define the `Info-directory-list' and modify its value on
> their elisp files.

I understand that part.  But I don't think it's TRT to have users or
Lisp programs outside of info.el manipulate Info-directory-list and
mutate it.  Instead, I think we should have a package.el-specific
directory list of Info files, say, package-info-directory-list, and we
should arrange for info.el to look in those directories _before_ it
looks in the directories mentioned by Info-directory-list.  Then
there'll be no need to play with info-initialize and the variable we
use to indicate info-initialize was already called, and (more
importantly), no code outside of info.el will mess with
Info-directory-list.  And package.el will add directories to this new
variable.

OK?

> > > +      (when-let ((info-dirs (butlast Info-directory-list)))
> > > +        (pp `(defvar Info-directory-list '()) (current-buffer))
> > > +        (pp `(setq Info-directory-list
> > > +                   (delete-dups
> > > +                    (append ',info-dirs Info-directory-list)))
> > > +            (current-buffer)))
> >
> > Using 'append' here could cause duplicate directories in
> > Info-directory-list.
> The `delete-dups' will avoid that.

Yes, but why not use add-to-list in the first place?

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

From: lin Sun <sunlin7 <at> yahoo.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 62767 <at> debbugs.gnu.org, philipk <at> posteo.net, monnier <at> iro.umontreal.ca
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Tue, 30 May 2023 03:56:00 +0000

On Fri, May 26, 2023 at 9:37 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>  But I don't think it's TRT to have users or
> Lisp programs outside of info.el manipulate Info-directory-list and
> mutate it.  Instead, I think we should have a package.el-specific
> directory list of Info files, say, package-info-directory-list, and we
> should arrange for info.el to look in those directories _before_ it
> looks in the directories mentioned by Info-directory-list.  Then
> there'll be no need to play with info-initialize and the variable we
> use to indicate info-initialize was already called, and (more
> importantly), no code outside of info.el will mess with
> Info-directory-list.  And package.el will add directories to this new
> variable.
>
> OK?
Totally understand your concern.
The proposed mechanism will request info.el to reserve a variable that
allows customer change outside of info.el, then info.el reads the
variable and proceeds the paths before the info.el major function. So
adding a new variable package-info-directory-list or using the current
Info-directory-list are nearly equal, except that the new name is for
package.el special.

> > > > +      (when-let ((info-dirs (butlast Info-directory-list)))
> > > > +        (pp `(defvar Info-directory-list '()) (current-buffer))
> > > > +        (pp `(setq Info-directory-list
> > > > +                   (delete-dups
> > > > +                    (append ',info-dirs Info-directory-list)))
> > > > +            (current-buffer)))
> > >
> > > Using 'append' here could cause duplicate directories in
> > > Info-directory-list.
> > The `delete-dups' will avoid that.
>
> Yes, but why not use add-to-list in the first place?
`add-to-list' will be a good choice here.
The code is following its previous code lines, almost same format,
paste as follow,
>      (pp `(defvar package-activated-list) (current-buffer))
>      (pp `(setq package-activated-list
>                 (delete-dups
>                  (append ',(mapcar #'package-desc-name package--quickstart-pkgs)

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: lin Sun <sunlin7 <at> yahoo.com>
Cc: 62767 <at> debbugs.gnu.org, philipk <at> posteo.net, monnier <at> iro.umontreal.ca
Subject: Re: bug#62767: 29.0.90; [PATCH] *lisp/emacs-lisp/package.el: set
 variables after info package
Date: Tue, 30 May 2023 13:25:46 +0300

> From: lin Sun <sunlin7 <at> yahoo.com>
> Date: Tue, 30 May 2023 03:56:00 +0000
> Cc: monnier <at> iro.umontreal.ca, philipk <at> posteo.net, 62767 <at> debbugs.gnu.org
> 
> The proposed mechanism will request info.el to reserve a variable that
> allows customer change outside of info.el, then info.el reads the
> variable and proceeds the paths before the info.el major function. So
> adding a new variable package-info-directory-list or using the current
> Info-directory-list are nearly equal, except that the new name is for
> package.el special.

Yes, that's the idea.  This way, we keep info.el and package.el very
loosely coupled, as they should be, and each one of them can apply its
own logic to its variable.

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

From: Lin Sun <sunlin7.mail <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Sun Lin <sunlin7 <at> yahoo.com>, bug-gnu-emacs <at> gnu.org
Subject: Re: 29.0.1;
 [PATCH] *doc/misc/gnus.texi: Fix unmatched quote in gnus doc
Date: Mon, 23 Oct 2023 16:30:55 -0700

[Message part 1 (text/plain, inline)]

Appreciate, thank you so much!

Send from my Android phone

On Mon, Oct 23, 2023, 11:57 Eli Zaretskii <eliz <at> gnu.org> wrote:

> close 66710 29.2
> thanks
>
> > From: Lin Sun <sunlin7.mail <at> gmail.com>
> > Date: Mon, 23 Oct 2023 17:15:18 +0000
> > Cc: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>, Eli Zaretskii <
> eliz <at> gnu.org>
> >
> > An unmatched quote in the doc/misc/gnus.texi causes its following part
> > to be displayed as a string.
> >
> > This patch will fix the issue.
> >
> > Please help to apply it. Thanks.
>
> Thanks, I installed this on the emacs-29 branch.
>
> Please note that the commit log message was not entirely according to
> our standards; please see how I modified it, and try in the future to
> use this style (see CONTRIBUTE for more details).
>

[Message part 2 (text/html, inline)]

Previous Next

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