bug#56614: two problems with `package--reload-previously-loaded'

[Paul Pogonyshev <pogonyshev@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2044 bytes --]

> Could you prepare a patch for both these
> issues?  (It's safer if you do it so that you can verify that things
> work fine after the change (since you already have a test case).)

Please find attached. I added comments for both changes that explain the
reasoning, so that they are not accidentally reverted by a later change.

Also replaced obsolete `find-function-source-path' with its new name
`find-library-source-path'.

Paul

On Sat, 23 Jul 2022 at 10:30, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Paul Pogonyshev <pogonyshev@gmail.com> writes:
>
> > 1. Function excludes package's current directory, see variable
> > `load-path-sans-dir'.  This is problematic if a package is reinstalled
> > from the same directory after changes have been made. I know this is
> > not how things usually work, but it seems to create problems for no
> > reason: if the same directory is never used, why bother about it here?
> > Normal usecases are not affected at all, why "special" usecases that
> > reuse the same directory suffer.
>
> Yes, that seems odd.
>
> > 2. When searching in `load-history', the function ignores changes from
> byte-compiled
> > to source and vice versa, i.e. `.el' -> `.elc' and `.elc' -> `.el'. In
> other words, if you
> > replace a byte-compiled file with a non-compiled (or vice versa), it
> never gets
> > reloaded. A simple fix (having zero effect in normal usecases) would be
> e.g. this:
> >
> >         (truename (file-truename canonical))
> >         (found (or (member truename history)
> >                    (member (if (string-suffix-p ".el" truename)
> >                                (replace-regexp-in-string (rx ".el" eos)
> ".elc" truename t)
> >                              (replace-regexp-in-string (rx ".elc" eos)
> ".el" truename t))
> >                            history)))
> >
>
> I think that makes sense.  Could you prepare a patch for both these
> issues?  (It's safer if you do it so that you can verify that things
> work fine after the change (since you already have a test case).)
>
>

[-- Attachment #1.2: Type: text/html, Size: 2924 bytes --]

[-- Attachment #2: 0001-Tweak-package-reload-previously-loaded-bug-56614.patch --]
[-- Type: text/x-patch, Size: 2919 bytes --]

From ba0fec69f810f924cb10231694fd741bd3915103 Mon Sep 17 00:00:00 2001
From: Paul Pogonyshev <pogonyshev@gmail.com>
Date: Sun, 24 Jul 2022 11:00:46 +0200
Subject: [PATCH] Tweak `package--reload-previously-loaded' (bug#56614)

---
 lisp/emacs-lisp/package.el | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 00beee811b..6b55726546 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -785,10 +785,14 @@ package--reload-previously-loaded
   (with-demoted-errors "Error in package--load-files-for-activation: %s"
     (let* (result
            (dir (package-desc-dir pkg-desc))
-           (load-path-sans-dir
-            (cl-remove-if (apply-partially #'string= dir)
-                          (or (bound-and-true-p find-function-source-path)
-                              load-path)))
+           ;; A previous implementation would skip `dir' itself.
+           ;; However, in normal use reloading from the same directory
+           ;; never happens anyway, while in certain cases external to
+           ;; Emacs a package in the same directory not necessary
+           ;; stays byte-identical, e.g.  during development.  Just
+           ;; don't special-case `dir'.
+           (effective-path (or (bound-and-true-p find-library-source-path)
+                               load-path))
            (files (directory-files-recursively dir "\\`[^\\.].*\\.el\\'"))
            (history (mapcar #'file-truename
                             (cl-remove-if-not #'stringp
@@ -796,8 +800,17 @@ package--reload-previously-loaded
       (dolist (file files)
         (when-let ((library (package--library-stem
                              (file-relative-name file dir)))
-                   (canonical (locate-library library nil load-path-sans-dir))
-                   (found (member (file-truename canonical) history))
+                   (canonical (locate-library library nil effective-path))
+                   (truename (file-truename canonical))
+                   ;; Normally, all files in a package are compiled by
+                   ;; now, but don't assume that.  E.g. different
+                   ;; versions can add or remove `no-byte-compile'.
+                   (altname (if (string-suffix-p ".el" truename)
+                                (replace-regexp-in-string "\\.el\\'" ".elc" truename t)
+                              (replace-regexp-in-string "\\.elc\\'" ".el" truename t)))
+                   (found (or (member truename history)
+                              (and (not (string= altname truename))
+                                   (member altname history))))
                    (recent-index (length found)))
           (unless (equal (file-name-base library)
                          (format "%s-autoloads" (package-desc-name pkg-desc)))
-- 
2.35.1