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

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

[-- Attachment #1: Type: text/plain, Size: 2807 bytes --]

On Wed, May 24, 2023 at 11:32 AM Eli Zaretskii <eliz@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

[-- Attachment #2: 0001-avoid-to-load-the-entire-info-package-V3.patch --]
[-- Type: text/x-patch, Size: 4950 bytes --]

From a17f389ce6af7600714d0cadf925a9bcd5a6ccb1 Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Tue, 11 Apr 2023 00:00:13 +0000
Subject: [PATCH] avoid to load the entire info package

*lisp/emacs-lisp/package.el: don't require info package
*lisp/info.el: use explicit mark `Info--initialized'
---
 lisp/emacs-lisp/package.el | 17 ++++++-----------
 lisp/info.el               | 34 +++++++++++++++++++---------------
 2 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 293c1c39ca..46de846b80 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -907,9 +907,6 @@ 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))
       (push name package-activated-list)
       ;; Don't return nil.
@@ -4478,7 +4475,6 @@ package-quickstart-refresh
   "(Re)Generate the `package-quickstart-file'."
   (interactive)
   (package-initialize 'no-activate)
-  (require 'info)
   (let ((package--quickstart-pkgs ())
         ;; Pretend we haven't activated anything yet!
         (package-activated-list ())
@@ -4521,13 +4517,12 @@ package-quickstart-refresh
                   (append ',(mapcar #'package-desc-name package--quickstart-pkgs)
                           package-activated-list)))
           (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)))
-              (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)))
       ;; Use `\s' instead of a space character, so this code chunk is not
       ;; mistaken for an actual file-local section of package.el.
       (insert "\f
diff --git a/lisp/info.el b/lisp/info.el
index 035dff66e7..631c35140b 100644
--- a/lisp/info.el
+++ b/lisp/info.el
@@ -400,6 +400,9 @@ Info-virtual-nodes
 (defvar-local Info-current-node-virtual nil
   "Non-nil if the current Info node is virtual.")
 
+(defvar info--initialized nil
+  "Non-nil if `info-initialize' has been run.")
+
 (defun Info-virtual-file-p (filename)
   "Check if Info file FILENAME is virtual."
   (Info-virtual-fun 'find-file filename nil))
@@ -700,29 +703,29 @@ Info-default-dirs
 
 (defun info-initialize ()
   "Initialize `Info-directory-list', if that hasn't been done yet."
-  (unless Info-directory-list
+  (unless info--initialized
     (let ((path (getenv "INFOPATH"))
 	  (sep (regexp-quote path-separator)))
-      (setq Info-directory-list
-	    (prune-directory-list
-	     (if path
-		 (if (string-match-p (concat sep "\\'") path)
-		     (append (split-string (substring path 0 -1) sep)
-			     (Info-default-dirs))
-		   (split-string path sep))
-	       (Info-default-dirs))))
+      (dolist (dir
+	       (prune-directory-list
+	        (if path
+		    (if (string-match-p (concat sep "\\'") path)
+		        (append (split-string (substring path 0 -1) sep)
+			        (Info-default-dirs))
+		      (split-string path sep))
+	          (Info-default-dirs))))
+        (add-to-list 'Info-directory-list dir))
       ;; For a self-contained (ie relocatable) NS build, AFAICS we
       ;; always want the included info directory to be at the head of
       ;; the search path, unless it's already in INFOPATH somewhere.
       ;; It's at the head of Info-default-directory-list,
       ;; but there's no way to get it at the head of Info-directory-list
       ;; except by doing it here.
-      (and path
-	   (featurep 'ns)
-	   (let ((dir (expand-file-name "../info" data-directory)))
-	     (and (file-directory-p dir)
-		  (not (member dir (split-string path ":" t)))
-		  (push dir Info-directory-list)))))))
+      (when (and path (featurep 'ns))
+        (when-let* ((dir (expand-file-name "../info" data-directory))
+                    ((file-directory-p dir)))
+	  (add-to-list 'Info-directory-list dir)))))
+  (setq info--initialized t))
 
 ;;;###autoload
 (defun info-other-window (&optional file-or-node buffer)
@@ -1279,6 +1282,7 @@ Info-dir-file-name
 ;; default-directory to the first directory we actually get any text
 ;; from.
 (defun Info-insert-dir ()
+  (info-initialize)
   (if (and Info-dir-contents Info-dir-file-attributes
 	   ;; Verify that none of the files we used has changed
 	   ;; since we used it.
-- 
2.20.5