bug#63757: 29.0.91 order of package paths changed: random old versions of packages in load-path

["Евгений Бойков via Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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

I've tested two other variants.

The simplest fix

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ba0e3618f28..6eaf261f5f4 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -922,11 +922,13 @@ correspond to previously loaded files."
                                  (v2 (package-desc-version p2)))
                              (or
                               ;; Prefer VC packages.
-                              (package-vc-p p1)
-                              (package-vc-p p2)
+                              (and
+                               (package-vc-p p1)
+                               (not (package-vc-p p2)))
                               ;; Prefer builtin packages.
-                              (package-disabled-p p1 v1)
-                              (not (package-disabled-p p2 v2))))))))
+                              (and
+                               (package-built-in-p p1 v1)
+                               (not (package-built-in-p p2 v2)))))))))
     ;; Check if PACKAGE is available in `package-alist'.
     (while
         (when pkg-descs



"Move to package-process-define-package" variant

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ba0e3618f28..c184625d754 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -686,7 +686,7 @@ Convert EXP into a `package-desc' object using the

 If there already exists a package by the same name in
 `package-alist', insert this object there such that the packages
-are sorted with the highest version first."
+are sorted with VC and builtin packages first and the highest version
second."
   (when (eq (car-safe exp) 'define-package)
     (let* ((new-pkg-desc (apply #'package-desc-from-define (cdr exp)))
            (name (package-desc-name new-pkg-desc))
@@ -696,13 +696,29 @@ are sorted with the highest version first."
           ;; If there's no old package, just add this to `package-alist'.
           (push (list name new-pkg-desc) package-alist)
         ;; If there is, insert the new package at the right place in the
list.
-        (while
-            (if (and (cdr old-pkgs)
-                     (version-list-< version
-                                     (package-desc-version (cadr
old-pkgs))))
-                (setq old-pkgs (cdr old-pkgs))
-              (push new-pkg-desc (cdr old-pkgs))
-              nil)))
+        (progn
+          ;; Prefer newer packages
+          (while
+              (if (and (cdr old-pkgs)
+                       (version-list-< version
+                                       (package-desc-version (cadr
old-pkgs))))
+                  (setq old-pkgs (cdr old-pkgs))
+                (push new-pkg-desc (cdr old-pkgs))
+                nil))
+          (let ((pkg-desc (sort (cdr (assq name package-alist))
+                                (lambda (p1 p2)
+                                  (let ((v1 (package-desc-version p1))
+                                        (v2 (package-desc-version p2)))
+                                    (or
+                                     (and
+                                      ;; Prefer VC packages.
+                                      (package-vc-p p1)
+                                      (not (package-vc-p p2)))
+                                     ;; Prefer builtin packages.
+                                     (and
+                                      (package-built-in-p p1 v1)
+                                      (not (package-built-in-p p2
v2)))))))))
+            (setq new-pkg-desc (car pkg-desc)))))
       new-pkg-desc)))

 (declare-function package-vc-commit "package-vc" (pkg))
@@ -916,17 +932,7 @@ correspond to previously loaded files."

 (defun package--get-activatable-pkg (pkg-name)
   ;; Is "activatable" a word?
-  (let ((pkg-descs (sort (cdr (assq pkg-name package-alist))
-                         (lambda (p1 p2)
-                           (let ((v1 (package-desc-version p1))
-                                 (v2 (package-desc-version p2)))
-                             (or
-                              ;; Prefer VC packages.
-                              (package-vc-p p1)
-                              (package-vc-p p2)
-                              ;; Prefer builtin packages.
-                              (package-disabled-p p1 v1)
-                              (not (package-disabled-p p2 v2))))))))
+  (let ((pkg-descs (cdr (assq pkg-name package-alist))))
     ;; Check if PACKAGE is available in `package-alist'.
     (while
         (when pkg-descs

On Sun, Jun 4, 2023 at 10:07 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Philip Kaludercic <philipk@posteo.net>
> > Cc: artscan@list.ru,  monnier@iro.umontreal.ca,  63757@debbugs.gnu.org
> > Date: Sun, 04 Jun 2023 11:39:53 +0000
> >
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> > > Is that a reverse diff or something?  The code we have now already
> > > does sort the list...
> >
> > No, the current code passes a non-nil NOSORT flag to `directory-files',
> > which inhibits sorting?
>
> I'm confused.  The code which the patch adds, viz.:
>
> >  (defun package--get-activatable-pkg (pkg-name)
> >    ;; Is "activatable" a word?
> > -  (let ((pkg-descs (cdr (assq pkg-name package-alist))))
> > +  (let ((pkg-descs (sort (cdr (assq pkg-name package-alist))
> > +                         (lambda (p1 p2)
> > +                           (let ((v1 (package-desc-version p1))
> > +                                 (v2 (package-desc-version p2)))
> > +                             (or
> > +                              ;; Prefer source packages.
> > +                              (eq (package-desc-kind p1) 'vc)
> > +                              (not (eq (package-desc-kind p2) 'vc))
> > +                              ;; Prefer builtin packages.
> > +                              (package-disabled-p p1 v1)
> > +                              (not (package-disabled-p p2 v2))))))))
> >      ;; Check if PACKAGE is available in `package-alist'.
>
> is already in the version of package.el on the emacs-29 branch.  And
> the code which the patch removes, viz.:
>
> >      ;; Check if PACKAGE is available in `package-alist'.
> >      (while
> >          (when pkg-descs
> >            (let ((available-version (package-desc-version (car
> pkg-descs))))
> > -            (or (package-disabled-p pkg-name available-version)
> > -                ;; Prefer a builtin package.
> > -                (package-built-in-p pkg-name available-version))))
> > +            (package-disabled-p pkg-name available-version)))
> >        (setq pkg-descs (cdr pkg-descs)))
> >      (car pkg-descs)))
>
> is already removed on the emacs-29 branch.
>
> What am I missing here?
>


-- 
__________________________

С уважением,
Бойков Евгений Алексеевич
сот. 8-924-202-25-65
e-mail: artscan@list.ru

[-- Attachment #2: Type: text/html, Size: 9503 bytes --]