bug#40971: Updating built-in packages that seq depends on is broken due to a bug in package.el

bug#40971: Updating built-in packages that seq depends on is broken due to a bug in package.el

[Clément Pit-Claudel]

Hi all,

This is a follow-up to the thread at https://lists.gnu.org/archive/html/emacs-devel/2020-04/msg01974.html.

`seq` and a few other packages are distributed through ELPA but also built-in.  In theory, this makes it possible to upgrade these packages after installing Emacs.

However, because implementation of package.el requires `seq', seq is loaded very early after starting Emacs — so early, in fact, that when package-initialize adds elpa/ directories to the load-path, seq is already loaded.

As a result, even if a newer version of seq is installed, it is never loaded, because the built-in seq library is already loaded.  Concretely, this means that it isn't possible to update seq.

I got bitten by this while developing a package that uses seq-sort-by, which isn't available in Emacs 25's built-in seq.  I added a dependency on seq 2.14, which does contain seq-sort-by, but that doesn't help: package.el does install the dependency, but it is never loaded.

I believe the bug lies in `package--list-loaded-files'.  Theoretically, after upgrading a package, it should be reloaded if it was already in the load history.  But the code to find previously loaded files uses file-name-sans-extension to match load-history entries to library file names.  This fails because seq.el is installed as seq.el.gz, so file-name-sans-extension returns seq.el, not seq.  

The correct approach would likely be to use get-load-suffixes, although it might also be enough to use find-library--from-load-history, which seems to already do most of the needed work.

Clément.

bug#40971: Status: Updating built-in packages that seq depends on is broken due to a bug in package.el

[Stefan Monnier]

> I believe the bug lies in `package--list-loaded-files'.
> Theoretically, after upgrading a package, it should be reloaded if it
> was already in the load history.  But the code to find previously
> loaded files uses file-name-sans-extension to match load-history
> entries to library file names.  This fails because seq.el is installed
> as seq.el.gz, so file-name-sans-extension returns seq.el, not seq.

Hmm... have you traced this function to see how it's called and what
it returns?

AFAIK `seq.el.gz` should "never" be loaded (it's `seq.elc` which is
loaded), so I'm not sure it can make much difference.  Furthermore,
I think that when activating packages at startup we don't bother to look
for already loaded files and reload them.  We only do that *during*
a package upgrade (in order to try and avoid having the old version's
macros&functions get in the way while compiling the new code, mostly).

I was about to look at tweaking `package.el` so it doesn't depend on
`seq` (and hence delay loading `seq` just enough that we end up loading
the right version) but then remembered that `seq.el` may very well
become preloaded in some not too distant future, so we should come up
with a better fix.  E.g. improve `package-activate-all` so it can handle
the situation where some packages have already been activated and where
some of those are now obsolete.  This would also be useful for users who
want to change some `package.el` settings from the `.emacs` (i.e. after
`package-activate-all` has already been called) and then call
`package-activate-all` manually to take those changes into account.

> although it might also be enough to use
> find-library--from-load-history, which seems to already do most of the
> needed work.

Sounds like a good idea, indeed.


        Stefan

bug#40971: Updating built-in packages that seq depends on is broken due to a bug in package.el

[Clément Pit-Claudel]

On 05/05/2020 23.35, Stefan Monnier wrote:
> Hmm... have you traced this function to see how it's called and what 
> it returns? AFAIK `seq.el.gz` should "never" be loaded (it's 
> `seq.elc` which is loaded), so I'm not sure it can make much 
> difference.

I did :) Here's the function:

(defun package--list-loaded-files (dir)
  "Recursively list all files in DIR which correspond to loaded features.
Returns the `file-name-sans-extension' of each file, relative to
DIR, sorted by most recently loaded last."
  (let* ((history (delq nil
                        (mapcar (lambda (x)
                                  (let ((f (car x)))
                                    (and (stringp f)
(1)                                      (file-name-sans-extension f))))
                                load-history)))
         (dir (file-truename dir))
         ;; List all files that have already been loaded.
         (list-of-conflicts
          (delq
           nil
           (mapcar
               (lambda (x) (let* ((file (file-relative-name x dir))
                             ;; Previously loaded file, if any.
                             (previous
                              (ignore-errors
                                (file-name-sans-extension
(2)                              (file-truename (find-library-name file)))))
                             (pos (when previous (member previous history))))
                        ;; Return (RELATIVE-FILENAME . HISTORY-POSITION)
                        (when pos
                          (cons (file-name-sans-extension file) (length pos)))))
             (directory-files-recursively dir "\\`[^\\.].*\\.el\\'")))))

On line (1) it uses file-name-sans-extension when iterating over load-history.  On line (2) it uses file-name-sans-extension + find-library-name when iterating over the package's files.
The problem is find-library-name: (find-library-name file) returns "/usr/local/share/emacs/28.0.50/lisp/emacs-lisp/seq.el.gz"

> Furthermore, I think that when activating packages at startup we 
> don't bother to look for already loaded files and reload them.  We 
> only do that *during* a package upgrade (in order to try and avoid 
> having the old version's macros&functions get in the way while 
> compiling the new code, mostly).

Good point.