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

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]

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

Hello.

I am not confident how to classify this kind of behaviour change.
You can see the beginning of the discussion below.

Briefly, if you have different versions of packages in package-user-dir
then pretest 29 loads random old versions of them whereas 28 loads mostly
latest versions.

---------- Forwarded message ---------
From: Philip Kaludercic <philipk@posteo.net>
Date: Sat, May 27, 2023 at 6:23 PM
Subject: Re: emacs 04c4c578 pretest 29.0.91
To: Евгений Бойков <artscan@list.ru>


Евгений Бойков <artscan@list.ru> writes:

> Hello, Philip.

Hi,

> In your commit 04c4c578, package directories became NOSORT due to (fourth
t
> argument)
>     (directory-files dir)
> replaced by
>     (directory-files dir t "^[^.]" t)
>
> It affects package-alist forming. For example, there are old and new
> versions of a package in my elpa directory:
>
> go-mode-20170726.555
> go-mode-20220114.2239
>
> After (package-initialize)
> in emacs 28 (require 'go-mode) gives 2022 version, e.g. the latest version
> of a package in most cases, directory-files default sorting works.
> In emacs 29.0.91 I get old 2017 version because of NOSORT unpredictable
> version order in package-alist and hence random paths in load-path.
>
> I just tried to move from 28 to 29.0.91 and got many errors due to old
> versions of many packages.
> Excuse me if it is a feature and I missed something.

No, this is not a feature but I think it might have uncovered a bug.
Relying on the ordering of file names (which IIRC is lexiographic not
numeric) is not a robust way to decide what packages should be preferred
when loading descriptors.  Could you submit a bug report to
bug-gnu-emacs@gnu.org so that this issue can be discussed with the other
maintainers?
-- 
__________________________

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

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

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

[Eli Zaretskii]

> Date: Sun, 28 May 2023 02:30:18 +1000
> From:  Евгений Бойков via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> I am not confident how to classify this kind of behaviour change.
> You can see the beginning of the discussion below.
> 
> Briefly, if you have different versions of packages in package-user-dir then pretest 29 loads random
> old versions of them whereas 28 loads mostly latest versions.
> 
> ---------- Forwarded message ---------
> From: Philip Kaludercic <philipk@posteo.net>
> Date: Sat, May 27, 2023 at 6:23 PM
> Subject: Re: emacs 04c4c578 pretest 29.0.91
> To: Евгений Бойков <artscan@list.ru>
> 
> Евгений Бойков <artscan@list.ru> writes:
> 
> > Hello, Philip.
> 
> Hi,
> 
> > In your commit 04c4c578, package directories became NOSORT due to (fourth t
> > argument)
> >     (directory-files dir)
> > replaced by
> >     (directory-files dir t "^[^.]" t)
> >
> > It affects package-alist forming. For example, there are old and new
> > versions of a package in my elpa directory:
> >
> > go-mode-20170726.555
> > go-mode-20220114.2239
> >
> > After (package-initialize)
> > in emacs 28 (require 'go-mode) gives 2022 version, e.g. the latest version
> > of a package in most cases, directory-files default sorting works.
> > In emacs 29.0.91 I get old 2017 version because of NOSORT unpredictable
> > version order in package-alist and hence random paths in load-path.
> >
> > I just tried to move from 28 to 29.0.91 and got many errors due to old
> > versions of many packages.
> > Excuse me if it is a feature and I missed something.
> 
> No, this is not a feature but I think it might have uncovered a bug.
> Relying on the ordering of file names (which IIRC is lexiographic not
> numeric) is not a robust way to decide what packages should be preferred
> when loading descriptors.  Could you submit a bug report to
> bug-gnu-emacs@gnu.org so that this issue can be discussed with the other
> maintainers?

Philip, are you looking into this?  Or do you have any comments?

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

[Philip Kaludercic]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sun, 28 May 2023 02:30:18 +1000
>> From:  Евгений Бойков via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> I am not confident how to classify this kind of behaviour change.
>> You can see the beginning of the discussion below.
>> 
>> Briefly, if you have different versions of packages in package-user-dir then pretest 29 loads random
>> old versions of them whereas 28 loads mostly latest versions.
>> 
>> ---------- Forwarded message ---------
>> From: Philip Kaludercic <philipk@posteo.net>
>> Date: Sat, May 27, 2023 at 6:23 PM
>> Subject: Re: emacs 04c4c578 pretest 29.0.91
>> To: Евгений Бойков <artscan@list.ru>
>> 
>> Евгений Бойков <artscan@list.ru> writes:
>> 
>> > Hello, Philip.
>> 
>> Hi,
>> 
>> > In your commit 04c4c578, package directories became NOSORT due to (fourth t
>> > argument)
>> >     (directory-files dir)
>> > replaced by
>> >     (directory-files dir t "^[^.]" t)
>> >
>> > It affects package-alist forming. For example, there are old and new
>> > versions of a package in my elpa directory:
>> >
>> > go-mode-20170726.555
>> > go-mode-20220114.2239
>> >
>> > After (package-initialize)
>> > in emacs 28 (require 'go-mode) gives 2022 version, e.g. the latest version
>> > of a package in most cases, directory-files default sorting works.
>> > In emacs 29.0.91 I get old 2017 version because of NOSORT unpredictable
>> > version order in package-alist and hence random paths in load-path.
>> >
>> > I just tried to move from 28 to 29.0.91 and got many errors due to old
>> > versions of many packages.
>> > Excuse me if it is a feature and I missed something.
>> 
>> No, this is not a feature but I think it might have uncovered a bug.
>> Relying on the ordering of file names (which IIRC is lexiographic not
>> numeric) is not a robust way to decide what packages should be preferred
>> when loading descriptors.  Could you submit a bug report to
>> bug-gnu-emacs@gnu.org so that this issue can be discussed with the other
>> maintainers?
>
> Philip, are you looking into this?  Or do you have any comments?

Евгений sent me a message with some more details, so yes I am looking at
it:

> I sent the bug report https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63757 (
> 63757@debbugs.gnu.org) and did more research.
> It turned out that the final ordering is defined (fb87d5) by

>   (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))))))))
>
> So I can fix it like this
>
> (sort (cdr (assq 'go-mode package-alist))
>                          (lambda (p1 p2)
>                            (let ((v1 (package-desc-version p1))
>                                  (v2 (package-desc-version p2)))
>                              (or
>      (not (version-list-< v1 v2))
>      ))))
>
> But I still can't understand how to fix it along with builtin and VC
> preferred ordering.

Won't this work:


[-- Attachment #2: Type: text/plain, Size: 733 bytes --]

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 01826da273d..7eb185e7042 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -927,7 +927,9 @@ package--get-activatable-pkg
                               (package-vc-p p2)
                               ;; Prefer builtin packages.
                               (package-disabled-p p1 v1)
-                              (not (package-disabled-p p2 v2))))))))
+                              (not (package-disabled-p p2 v2))
+                              ;; Prever newer packages
+                              (version-list-< v2 v1)))))))
     ;; Check if PACKAGE is available in `package-alist'.
     (while
         (when pkg-descs

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

[Philip Kaludercic]

Philip Kaludercic <philipk@posteo.net> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> Date: Sun, 28 May 2023 02:30:18 +1000
>>> From:  Евгений Бойков via "Bug reports for GNU Emacs,
>>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>>> 
>>> I am not confident how to classify this kind of behaviour change.
>>> You can see the beginning of the discussion below.
>>> 
>>> Briefly, if you have different versions of packages in package-user-dir then pretest 29 loads random
>>> old versions of them whereas 28 loads mostly latest versions.
>>> 
>>> ---------- Forwarded message ---------
>>> From: Philip Kaludercic <philipk@posteo.net>
>>> Date: Sat, May 27, 2023 at 6:23 PM
>>> Subject: Re: emacs 04c4c578 pretest 29.0.91
>>> To: Евгений Бойков <artscan@list.ru>
>>> 
>>> Евгений Бойков <artscan@list.ru> writes:
>>> 
>>> > Hello, Philip.
>>> 
>>> Hi,
>>> 
>>> > In your commit 04c4c578, package directories became NOSORT due to (fourth t
>>> > argument)
>>> >     (directory-files dir)
>>> > replaced by
>>> >     (directory-files dir t "^[^.]" t)
>>> >
>>> > It affects package-alist forming. For example, there are old and new
>>> > versions of a package in my elpa directory:
>>> >
>>> > go-mode-20170726.555
>>> > go-mode-20220114.2239
>>> >
>>> > After (package-initialize)
>>> > in emacs 28 (require 'go-mode) gives 2022 version, e.g. the latest version
>>> > of a package in most cases, directory-files default sorting works.
>>> > In emacs 29.0.91 I get old 2017 version because of NOSORT unpredictable
>>> > version order in package-alist and hence random paths in load-path.
>>> >
>>> > I just tried to move from 28 to 29.0.91 and got many errors due to old
>>> > versions of many packages.
>>> > Excuse me if it is a feature and I missed something.
>>> 
>>> No, this is not a feature but I think it might have uncovered a bug.
>>> Relying on the ordering of file names (which IIRC is lexiographic not
>>> numeric) is not a robust way to decide what packages should be preferred
>>> when loading descriptors.  Could you submit a bug report to
>>> bug-gnu-emacs@gnu.org so that this issue can be discussed with the other
>>> maintainers?
>>
>> Philip, are you looking into this?  Or do you have any comments?
>
> Евгений sent me a message with some more details, so yes I am looking at
> it:
>
>> I sent the bug report https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63757 (
>> 63757@debbugs.gnu.org) and did more research.
>> It turned out that the final ordering is defined (fb87d5) by
>
>>   (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))))))))
>>
>> So I can fix it like this
>>
>> (sort (cdr (assq 'go-mode package-alist))
>>                          (lambda (p1 p2)
>>                            (let ((v1 (package-desc-version p1))
>>                                  (v2 (package-desc-version p2)))
>>                              (or
>>      (not (version-list-< v1 v2))
>>      ))))
>>
>> But I still can't understand how to fix it along with builtin and VC
>> preferred ordering.
>
> Won't this work:
>
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index 01826da273d..7eb185e7042 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -927,7 +927,9 @@ package--get-activatable-pkg
>                                (package-vc-p p2)
>                                ;; Prefer builtin packages.
>                                (package-disabled-p p1 v1)
> -                              (not (package-disabled-p p2 v2))))))))
> +                              (not (package-disabled-p p2 v2))
> +                              ;; Prever newer packages
> +                              (version-list-< v2 v1)))))))
>      ;; Check if PACKAGE is available in `package-alist'.
>      (while
>          (when pkg-descs

Ping?

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

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: Евгений Бойков <artscan@list.ru>,
>   63757@debbugs.gnu.org,  Stefan
>  Monnier <monnier@iro.umontreal.ca>
> Date: Sat, 03 Jun 2023 10:19:24 +0000
> 
> > Won't this work:
> >
> > diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> > index 01826da273d..7eb185e7042 100644
> > --- a/lisp/emacs-lisp/package.el
> > +++ b/lisp/emacs-lisp/package.el
> > @@ -927,7 +927,9 @@ package--get-activatable-pkg
> >                                (package-vc-p p2)
> >                                ;; Prefer builtin packages.
> >                                (package-disabled-p p1 v1)
> > -                              (not (package-disabled-p p2 v2))))))))
> > +                              (not (package-disabled-p p2 v2))
> > +                              ;; Prever newer packages
> > +                              (version-list-< v2 v1)))))))
> >      ;; Check if PACKAGE is available in `package-alist'.
> >      (while
> >          (when pkg-descs
> 
> Ping?

If there's no response, and you think this is the right fix, we can
simply install it, I guess.

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]

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

The fix above will not work.

I can do it for "Prefer VC packages" only:

(require 'cl)
(defun package--get-activatable-pkg (pkg-name)
  ;; Is "activatable" a word?
  (let ((pkg-descs
         (flatten-list
          (mapcar (lambda (x)
                    (sort x
                          (lambda (p1 p2)
                            (let ((v1 (package-desc-version p1))
                                  (v2 (package-desc-version p2)))
                              (version-list-< v2 v1)))))
                  (reduce
                   (lambda (p1 p2)
                     (if (package-vc-p p1)
                         (push p1 (first p2))
                       (push p1 (second p2)))
                     p2)
                   (cdr (assq pkg-name package-alist))
                   :initial-value (list nil nil)
                   :from-end t)))))
    ;; Check if PACKAGE is available in `package-alist'.
    (while
        (when pkg-descs
          (let ((available-version (package-desc-version (car pkg-descs))))
            (package-disabled-p pkg-name available-version)))
      (setq pkg-descs (cdr pkg-descs)))
    (car pkg-descs)))


Still can't for "Prefer builtin packages".


On Sat, Jun 3, 2023 at 8:37 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Philip Kaludercic <philipk@posteo.net>
> > Cc: Евгений Бойков <artscan@list.ru>,
> >   63757@debbugs.gnu.org,  Stefan
> >  Monnier <monnier@iro.umontreal.ca>
> > Date: Sat, 03 Jun 2023 10:19:24 +0000
> >
> > > Won't this work:
> > >
> > > diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> > > index 01826da273d..7eb185e7042 100644
> > > --- a/lisp/emacs-lisp/package.el
> > > +++ b/lisp/emacs-lisp/package.el
> > > @@ -927,7 +927,9 @@ package--get-activatable-pkg
> > >                                (package-vc-p p2)
> > >                                ;; Prefer builtin packages.
> > >                                (package-disabled-p p1 v1)
> > > -                              (not (package-disabled-p p2 v2))))))))
> > > +                              (not (package-disabled-p p2 v2))
> > > +                              ;; Prever newer packages
> > > +                              (version-list-< v2 v1)))))))
> > >      ;; Check if PACKAGE is available in `package-alist'.
> > >      (while
> > >          (when pkg-descs
> >
> > Ping?
>
> If there's no response, and you think this is the right fix, we can
> simply install it, I guess.
>


-- 
__________________________

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

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

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]

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

The idea is to split the list to 3 parts and to sort each of them by
version-list-< separately (but I am not sure which combination of
preference is correct) and to flatten at the end

((VC packages) (builtin packages) (the rest))

I use (package-vc-p p1) and (assq p1 package--builtins) predicates for
splitting (not sure for builtin). The code below needs refactoring.

(require 'cl)
(defun package--get-activatable-pkg (pkg-name)
  ;; Is "activatable" a word?
  (let ((pkg-descs
         (flatten-list
          (mapcar (lambda (x)
                    (sort x
                          (lambda (p1 p2)
                            (let ((v1 (package-desc-version p1))
                                  (v2 (package-desc-version p2)))
                              (version-list-< v2 v1)))))
                  (let ((y (reduce
                            (lambda (p1 p2)
                              (if (package-vc-p p1)
                                  (push p1 (first p2))
                                (push p1 (second p2)))
                              p2)
                            (cdr (assq pkg-name package-alist))
                            :initial-value (list nil nil)
                            :from-end t)))
                    (append (list (car y))
                            (reduce
                             (lambda (p1 p2)
                               (if (assq p1 package--builtins)
                                   (push p1 (first p2))
                                 (push p1 (second p2))
                                 p2))
                             (cadr y)
                             :initial-value (list nil nil)
                             :from-end t)))))))
    ;; Check if PACKAGE is available in `package-alist'.
    (while
        (when pkg-descs
          (let ((available-version (package-desc-version (car pkg-descs))))
            (package-disabled-p pkg-name available-version)))
      (setq pkg-descs (cdr pkg-descs)))
    (car pkg-descs)))



On Sat, Jun 3, 2023 at 9:50 PM Евгений Бойков <artscan@list.ru> wrote:

> The fix above will not work.
>
> I can do it for "Prefer VC packages" only:
>
> (require 'cl)
> (defun package--get-activatable-pkg (pkg-name)
>   ;; Is "activatable" a word?
>   (let ((pkg-descs
>          (flatten-list
>           (mapcar (lambda (x)
>                     (sort x
>                           (lambda (p1 p2)
>                             (let ((v1 (package-desc-version p1))
>                                   (v2 (package-desc-version p2)))
>                               (version-list-< v2 v1)))))
>                   (reduce
>                    (lambda (p1 p2)
>                      (if (package-vc-p p1)
>                          (push p1 (first p2))
>                        (push p1 (second p2)))
>                      p2)
>                    (cdr (assq pkg-name package-alist))
>                    :initial-value (list nil nil)
>                    :from-end t)))))
>     ;; Check if PACKAGE is available in `package-alist'.
>     (while
>         (when pkg-descs
>           (let ((available-version (package-desc-version (car pkg-descs))))
>             (package-disabled-p pkg-name available-version)))
>       (setq pkg-descs (cdr pkg-descs)))
>     (car pkg-descs)))
>
>
> Still can't for "Prefer builtin packages".
>
>
> On Sat, Jun 3, 2023 at 8:37 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
>> > From: Philip Kaludercic <philipk@posteo.net>
>> > Cc: Евгений Бойков <artscan@list.ru>,
>> >   63757@debbugs.gnu.org,  Stefan
>> >  Monnier <monnier@iro.umontreal.ca>
>> > Date: Sat, 03 Jun 2023 10:19:24 +0000
>> >
>> > > Won't this work:
>> > >
>> > > diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
>> > > index 01826da273d..7eb185e7042 100644
>> > > --- a/lisp/emacs-lisp/package.el
>> > > +++ b/lisp/emacs-lisp/package.el
>> > > @@ -927,7 +927,9 @@ package--get-activatable-pkg
>> > >                                (package-vc-p p2)
>> > >                                ;; Prefer builtin packages.
>> > >                                (package-disabled-p p1 v1)
>> > > -                              (not (package-disabled-p p2 v2))))))))
>> > > +                              (not (package-disabled-p p2 v2))
>> > > +                              ;; Prever newer packages
>> > > +                              (version-list-< v2 v1)))))))
>> > >      ;; Check if PACKAGE is available in `package-alist'.
>> > >      (while
>> > >          (when pkg-descs
>> >
>> > Ping?
>>
>> If there's no response, and you think this is the right fix, we can
>> simply install it, I guess.
>>
>
>
> --
> __________________________
>
> С уважением,
> Бойков Евгений Алексеевич
> сот. 8-924-202-25-65
> e-mail: artscan@list.ru
>


-- 
__________________________

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

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

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]

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

By the way, is "Prefer builtin packages" related to (package-disabled-p p1
v1)? I think the comment and the code mean different things.


On Sat, Jun 3, 2023 at 10:53 PM Евгений Бойков <artscan@list.ru> wrote:

> The idea is to split the list to 3 parts and to sort each of them by
> version-list-< separately (but I am not sure which combination of
> preference is correct) and to flatten at the end
>
> ((VC packages) (builtin packages) (the rest))
>
> I use (package-vc-p p1) and (assq p1 package--builtins) predicates for
> splitting (not sure for builtin). The code below needs refactoring.
>
> (require 'cl)
> (defun package--get-activatable-pkg (pkg-name)
>   ;; Is "activatable" a word?
>   (let ((pkg-descs
>          (flatten-list
>           (mapcar (lambda (x)
>                     (sort x
>                           (lambda (p1 p2)
>                             (let ((v1 (package-desc-version p1))
>                                   (v2 (package-desc-version p2)))
>                               (version-list-< v2 v1)))))
>                   (let ((y (reduce
>                             (lambda (p1 p2)
>                               (if (package-vc-p p1)
>                                   (push p1 (first p2))
>                                 (push p1 (second p2)))
>                               p2)
>                             (cdr (assq pkg-name package-alist))
>                             :initial-value (list nil nil)
>                             :from-end t)))
>                     (append (list (car y))
>                             (reduce
>                              (lambda (p1 p2)
>                                (if (assq p1 package--builtins)
>                                    (push p1 (first p2))
>                                  (push p1 (second p2))
>                                  p2))
>                              (cadr y)
>                              :initial-value (list nil nil)
>                              :from-end t)))))))
>     ;; Check if PACKAGE is available in `package-alist'.
>     (while
>         (when pkg-descs
>           (let ((available-version (package-desc-version (car pkg-descs))))
>             (package-disabled-p pkg-name available-version)))
>       (setq pkg-descs (cdr pkg-descs)))
>     (car pkg-descs)))
>
>
>
> On Sat, Jun 3, 2023 at 9:50 PM Евгений Бойков <artscan@list.ru> wrote:
>
>> The fix above will not work.
>>
>> I can do it for "Prefer VC packages" only:
>>
>> (require 'cl)
>> (defun package--get-activatable-pkg (pkg-name)
>>   ;; Is "activatable" a word?
>>   (let ((pkg-descs
>>          (flatten-list
>>           (mapcar (lambda (x)
>>                     (sort x
>>                           (lambda (p1 p2)
>>                             (let ((v1 (package-desc-version p1))
>>                                   (v2 (package-desc-version p2)))
>>                               (version-list-< v2 v1)))))
>>                   (reduce
>>                    (lambda (p1 p2)
>>                      (if (package-vc-p p1)
>>                          (push p1 (first p2))
>>                        (push p1 (second p2)))
>>                      p2)
>>                    (cdr (assq pkg-name package-alist))
>>                    :initial-value (list nil nil)
>>                    :from-end t)))))
>>     ;; Check if PACKAGE is available in `package-alist'.
>>     (while
>>         (when pkg-descs
>>           (let ((available-version (package-desc-version (car
>> pkg-descs))))
>>             (package-disabled-p pkg-name available-version)))
>>       (setq pkg-descs (cdr pkg-descs)))
>>     (car pkg-descs)))
>>
>>
>> Still can't for "Prefer builtin packages".
>>
>>
>> On Sat, Jun 3, 2023 at 8:37 PM Eli Zaretskii <eliz@gnu.org> wrote:
>>
>>> > From: Philip Kaludercic <philipk@posteo.net>
>>> > Cc: Евгений Бойков <artscan@list.ru>,
>>> >   63757@debbugs.gnu.org,  Stefan
>>> >  Monnier <monnier@iro.umontreal.ca>
>>> > Date: Sat, 03 Jun 2023 10:19:24 +0000
>>> >
>>> > > Won't this work:
>>> > >
>>> > > diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
>>> > > index 01826da273d..7eb185e7042 100644
>>> > > --- a/lisp/emacs-lisp/package.el
>>> > > +++ b/lisp/emacs-lisp/package.el
>>> > > @@ -927,7 +927,9 @@ package--get-activatable-pkg
>>> > >                                (package-vc-p p2)
>>> > >                                ;; Prefer builtin packages.
>>> > >                                (package-disabled-p p1 v1)
>>> > > -                              (not (package-disabled-p p2 v2))))))))
>>> > > +                              (not (package-disabled-p p2 v2))
>>> > > +                              ;; Prever newer packages
>>> > > +                              (version-list-< v2 v1)))))))
>>> > >      ;; Check if PACKAGE is available in `package-alist'.
>>> > >      (while
>>> > >          (when pkg-descs
>>> >
>>> > Ping?
>>>
>>> If there's no response, and you think this is the right fix, we can
>>> simply install it, I guess.
>>>
>>
>>
>> --
>> __________________________
>>
>> С уважением,
>> Бойков Евгений Алексеевич
>> сот. 8-924-202-25-65
>> e-mail: artscan@list.ru
>>
>
>
> --
> __________________________
>
> С уважением,
> Бойков Евгений Алексеевич
> сот. 8-924-202-25-65
> e-mail: artscan@list.ru
>


-- 
__________________________

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

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

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> Won't this work:
>>
>> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
>> index 01826da273d..7eb185e7042 100644
>> --- a/lisp/emacs-lisp/package.el
>> +++ b/lisp/emacs-lisp/package.el
>> @@ -927,7 +927,9 @@ package--get-activatable-pkg
>>                                (package-vc-p p2)
>>                                ;; Prefer builtin packages.
>>                                (package-disabled-p p1 v1)
>> -                              (not (package-disabled-p p2 v2))))))))
>> +                              (not (package-disabled-p p2 v2))
>> +                              ;; Prever newer packages
>> +                              (version-list-< v2 v1)))))))
>>      ;; Check if PACKAGE is available in `package-alist'.
>>      (while
>>          (when pkg-descs
>
> Ping?

The packages should already be sorted by version in `package-alist`, so
at best this will paper over the problem.  This said, the `sort` call's
predicate looks weird indeed:

                         (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))))))))

- If both p1 and p2 are VC, then it will return non-nil, so
  they're both "strictly less than" the other :-(
- If neither is VC nor disabled, we return non-nil, which again means they're
   both "strictly less than" the other.
- The comment says "Prefer builtin packages" but the code checks
  `package-disabled-p`.

And now that I look more carefully, maybe this `sort` is the culprit
after all, because it operates directly on the list contained in
`packages-alist`, modifying it in-place :-(

I think we should *not* sort here: we should instead sort when we
populate `package-alist` (like we already do).  So the "Prefer VC" part
of the sorting should be moved to `package-process-define-package`
(and the `package-disabled-p` can be dropped because it's already taken
care of by the loop that follows the above code).


        Stefan

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]

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

move "Prefer VC packages" to `package-process-define-package`
drop `package-disabled-p` and "Prefer builtin packages" comment


diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ba0e3618f28..65ec07b945a 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 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,30 @@ 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))
+          ;; Prefer VC packages
+          (let* ((pkg-desc0 (cdr (assq name package-alist)))
+                 (pkg-desc
+                  (apply 'append
+                         (cl-reduce
+                          (lambda (p1 p2)
+                            (if (package-vc-p p1)
+                                (push p1 (cl-first p2))
+                              (push p1 (cl-second p2)))
+                            p2)
+                          pkg-desc0
+                          :initial-value (list nil nil)
+                          :from-end t))))
+            (setf pkg-desc0 pkg-desc)
+            (setq new-pkg-desc (car pkg-desc)))))
       new-pkg-desc)))

 (declare-function package-vc-commit "package-vc" (pkg))
@@ -916,17 +933,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 12:06 AM Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> >> Won't this work:
> >>
> >> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> >> index 01826da273d..7eb185e7042 100644
> >> --- a/lisp/emacs-lisp/package.el
> >> +++ b/lisp/emacs-lisp/package.el
> >> @@ -927,7 +927,9 @@ package--get-activatable-pkg
> >>                                (package-vc-p p2)
> >>                                ;; Prefer builtin packages.
> >>                                (package-disabled-p p1 v1)
> >> -                              (not (package-disabled-p p2 v2))))))))
> >> +                              (not (package-disabled-p p2 v2))
> >> +                              ;; Prever newer packages
> >> +                              (version-list-< v2 v1)))))))
> >>      ;; Check if PACKAGE is available in `package-alist'.
> >>      (while
> >>          (when pkg-descs
> >
> > Ping?
>
> The packages should already be sorted by version in `package-alist`, so
> at best this will paper over the problem.  This said, the `sort` call's
> predicate looks weird indeed:
>
>                          (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))))))))
>
> - If both p1 and p2 are VC, then it will return non-nil, so
>   they're both "strictly less than" the other :-(
> - If neither is VC nor disabled, we return non-nil, which again means
> they're
>    both "strictly less than" the other.
> - The comment says "Prefer builtin packages" but the code checks
>   `package-disabled-p`.
>
> And now that I look more carefully, maybe this `sort` is the culprit
> after all, because it operates directly on the list contained in
> `packages-alist`, modifying it in-place :-(
>
> I think we should *not* sort here: we should instead sort when we
> populate `package-alist` (like we already do).  So the "Prefer VC" part
> of the sorting should be moved to `package-process-define-package`
> (and the `package-disabled-p` can be dropped because it's already taken
> care of by the loop that follows the above code).
>
>
>         Stefan
>
>

-- 
__________________________

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

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

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]

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

Excuse me for extra messages, I've already found a bug in the previous
patch.
I need a review, especially with in-place operations.

move "Prefer VC packages" to `package-process-define-package`
drop `package-disabled-p` and "Prefer builtin packages" comment

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ba0e3618f28..87c57990705 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 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))
+          ;; Prefer VC packages with greater priority than newer packages
+          (let* ((pkg-desc
+                  (apply 'append
+                         (cl-reduce
+                          (lambda (p1 p2)
+                            (if (package-vc-p p1)
+                                (push p1 (cl-first p2))
+                              (push p1 (cl-second p2)))
+                            p2)
+                          (cdr (assq name package-alist))
+                          :initial-value (list nil nil)
+                          :from-end t))))
+            (setf (cdr (assq name package-alist)) pkg-desc)
+            (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 2:46 PM Евгений Бойков <artscan@list.ru> wrote:

> move "Prefer VC packages" to `package-process-define-package`
> drop `package-disabled-p` and "Prefer builtin packages" comment
>
>
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index ba0e3618f28..65ec07b945a 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 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,30 @@ 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))
> +          ;; Prefer VC packages
> +          (let* ((pkg-desc0 (cdr (assq name package-alist)))
> +                 (pkg-desc
> +                  (apply 'append
> +                         (cl-reduce
> +                          (lambda (p1 p2)
> +                            (if (package-vc-p p1)
> +                                (push p1 (cl-first p2))
> +                              (push p1 (cl-second p2)))
> +                            p2)
> +                          pkg-desc0
> +                          :initial-value (list nil nil)
> +                          :from-end t))))
> +            (setf pkg-desc0 pkg-desc)
> +            (setq new-pkg-desc (car pkg-desc)))))
>        new-pkg-desc)))
>
>  (declare-function package-vc-commit "package-vc" (pkg))
> @@ -916,17 +933,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 12:06 AM Stefan Monnier <monnier@iro.umontreal.ca>
> wrote:
>
>> >> Won't this work:
>> >>
>> >> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
>> >> index 01826da273d..7eb185e7042 100644
>> >> --- a/lisp/emacs-lisp/package.el
>> >> +++ b/lisp/emacs-lisp/package.el
>> >> @@ -927,7 +927,9 @@ package--get-activatable-pkg
>> >>                                (package-vc-p p2)
>> >>                                ;; Prefer builtin packages.
>> >>                                (package-disabled-p p1 v1)
>> >> -                              (not (package-disabled-p p2 v2))))))))
>> >> +                              (not (package-disabled-p p2 v2))
>> >> +                              ;; Prever newer packages
>> >> +                              (version-list-< v2 v1)))))))
>> >>      ;; Check if PACKAGE is available in `package-alist'.
>> >>      (while
>> >>          (when pkg-descs
>> >
>> > Ping?
>>
>> The packages should already be sorted by version in `package-alist`, so
>> at best this will paper over the problem.  This said, the `sort` call's
>> predicate looks weird indeed:
>>
>>                          (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))))))))
>>
>> - If both p1 and p2 are VC, then it will return non-nil, so
>>   they're both "strictly less than" the other :-(
>> - If neither is VC nor disabled, we return non-nil, which again means
>> they're
>>    both "strictly less than" the other.
>> - The comment says "Prefer builtin packages" but the code checks
>>   `package-disabled-p`.
>>
>> And now that I look more carefully, maybe this `sort` is the culprit
>> after all, because it operates directly on the list contained in
>> `packages-alist`, modifying it in-place :-(
>>
>> I think we should *not* sort here: we should instead sort when we
>> populate `package-alist` (like we already do).  So the "Prefer VC" part
>> of the sorting should be moved to `package-process-define-package`
>> (and the `package-disabled-p` can be dropped because it's already taken
>> care of by the loop that follows the above code).
>>
>>
>>         Stefan
>>
>>
>
> --
> __________________________
>
> С уважением,
> Бойков Евгений Алексеевич
> сот. 8-924-202-25-65
> e-mail: artscan@list.ru
>


-- 
__________________________

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

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

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

[Eli Zaretskii]

> From: Евгений Бойков <artscan@list.ru>
> Date: Sun, 4 Jun 2023 15:37:31 +1000
> Cc: Philip Kaludercic <philipk@posteo.net>, Eli Zaretskii <eliz@gnu.org>, 63757@debbugs.gnu.org
> 
> Excuse me for extra messages, I've already found a bug in the previous patch.
> I need a review, especially with in-place operations.
> 
> move "Prefer VC packages" to `package-process-define-package`
> drop `package-disabled-p` and "Prefer builtin packages" comment

Thanks, but I'm not really comfortable with such non-trivial changes
in package.el at this late stage of the pretest.

Is this problem new in Emacs 29, or did it exist in Emacs 28 as well?
If it is new, which change(s) caused the incorrect ordering of
versions in Emacs 29?

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

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Евгений Бойков <artscan@list.ru>
>> Date: Sun, 4 Jun 2023 15:37:31 +1000
>> Cc: Philip Kaludercic <philipk@posteo.net>, Eli Zaretskii <eliz@gnu.org>, 63757@debbugs.gnu.org
>> 
>> Excuse me for extra messages, I've already found a bug in the previous patch.
>> I need a review, especially with in-place operations.
>> 
>> move "Prefer VC packages" to `package-process-define-package`
>> drop `package-disabled-p` and "Prefer builtin packages" comment
>
> Thanks, but I'm not really comfortable with such non-trivial changes
> in package.el at this late stage of the pretest.
>
> Is this problem new in Emacs 29, or did it exist in Emacs 28 as well?
> If it is new, which change(s) caused the incorrect ordering of
> versions in Emacs 29?

It is new, and the simplest solution that should revert the issue could
be just to have `package-load-all-descriptors' sort the files when
loading descriptors.  I am not sure if the issue that Stefan raised,
with `sort' in `package--get-activatable-pkg' destructively modifying
`package-alist', but if anything that has been an issue before Emacs 29
as well.

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

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: Евгений Бойков <artscan@list.ru>,
>   monnier@iro.umontreal.ca,
>   63757@debbugs.gnu.org
> Date: Sun, 04 Jun 2023 07:47:50 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Is this problem new in Emacs 29, or did it exist in Emacs 28 as well?
> > If it is new, which change(s) caused the incorrect ordering of
> > versions in Emacs 29?
> 
> It is new, and the simplest solution that should revert the issue could
> be just to have `package-load-all-descriptors' sort the files when
> loading descriptors.

Can you show a patch for this, please?

> I am not sure if the issue that Stefan raised,
> with `sort' in `package--get-activatable-pkg' destructively modifying
> `package-alist', but if anything that has been an issue before Emacs 29
> as well.

Евгений says that the call to 'sort' in package--get-activatable-pkg
didn't exist before commit fb87d500.

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

[Philip Kaludercic]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: Евгений Бойков <artscan@list.ru>,
>>   monnier@iro.umontreal.ca,
>>   63757@debbugs.gnu.org
>> Date: Sun, 04 Jun 2023 07:47:50 +0000
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Is this problem new in Emacs 29, or did it exist in Emacs 28 as well?
>> > If it is new, which change(s) caused the incorrect ordering of
>> > versions in Emacs 29?
>> 
>> It is new, and the simplest solution that should revert the issue could
>> be just to have `package-load-all-descriptors' sort the files when
>> loading descriptors.
>
> Can you show a patch for this, please?

Whoops, I forgot to attach a diff:


[-- Attachment #2: Type: text/plain, Size: 1479 bytes --]

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 1ee39f87529..0145306dc4e 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -898,14 +898,22 @@ package-activate-1
 
 (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'.
     (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)))
 

[-- Attachment #3: Type: text/plain, Size: 414 bytes --]


>> I am not sure if the issue that Stefan raised,
>> with `sort' in `package--get-activatable-pkg' destructively modifying
>> `package-alist', but if anything that has been an issue before Emacs 29
>> as well.
>
> Евгений says that the call to 'sort' in package--get-activatable-pkg
> didn't exist before commit fb87d500.

That is true, and perhaps that could be reverted to some degree as well.

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

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: artscan@list.ru,  monnier@iro.umontreal.ca,  63757@debbugs.gnu.org
> Date: Sun, 04 Jun 2023 09:19:28 +0000
> 
> >> It is new, and the simplest solution that should revert the issue could
> >> be just to have `package-load-all-descriptors' sort the files when
> >> loading descriptors.
> >
> > Can you show a patch for this, please?
> 
> Whoops, I forgot to attach a diff:

Is that a reverse diff or something?  The code we have now already
does sort the list...

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

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: artscan@list.ru,  monnier@iro.umontreal.ca,  63757@debbugs.gnu.org
>> Date: Sun, 04 Jun 2023 09:19:28 +0000
>> 
>> >> It is new, and the simplest solution that should revert the issue could
>> >> be just to have `package-load-all-descriptors' sort the files when
>> >> loading descriptors.
>> >
>> > Can you show a patch for this, please?
>> 
>> Whoops, I forgot to attach a diff:
>
> 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?

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

[Eli Zaretskii]

> 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?

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

[Philip Kaludercic]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> 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 sorry, that was the wrong patch -.-


[-- Attachment #2: Type: text/plain, Size: 561 bytes --]

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index de5108430aa..fdc09a939be 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -739,7 +739,7 @@ package-load-all-descriptors
 updates `package-alist'."
   (dolist (dir (cons package-user-dir package-directory-list))
     (when (file-directory-p dir)
-      (dolist (pkg-dir (directory-files dir t "\\`[^.]" t))
+      (dolist (pkg-dir (directory-files dir t "\\`[^.]"))
         (when (file-directory-p pkg-dir)
           (package-load-descriptor pkg-dir))))))
 

[-- Attachment #3: Type: text/plain, Size: 173 bytes --]


(I attached the *vc-diff* buffer, but before sending the message I
invoked vc-diff again which modified the buffer contents.  Should have
just copied the diff manually...)

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

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: artscan@list.ru,  monnier@iro.umontreal.ca,  63757@debbugs.gnu.org
> Date: Sun, 04 Jun 2023 12:12:16 +0000
> 
> I'm sorry, that was the wrong patch -.-
> 
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index de5108430aa..fdc09a939be 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -739,7 +739,7 @@ package-load-all-descriptors
>  updates `package-alist'."
>    (dolist (dir (cons package-user-dir package-directory-list))
>      (when (file-directory-p dir)
> -      (dolist (pkg-dir (directory-files dir t "\\`[^.]" t))
> +      (dolist (pkg-dir (directory-files dir t "\\`[^.]"))
>          (when (file-directory-p pkg-dir)
>            (package-load-descriptor pkg-dir))))))
>  
> (I attached the *vc-diff* buffer, but before sending the message I
> invoked vc-diff again which modified the buffer contents.  Should have
> just copied the diff manually...)

Thanks, now it at least makes sense.

But why does it matter whether the package directory names are sorted
or not?  Is sorting package versions the same as sorting the names of
their directories?

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

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: artscan@list.ru,  monnier@iro.umontreal.ca,  63757@debbugs.gnu.org
>> Date: Sun, 04 Jun 2023 12:12:16 +0000
>> 
>> I'm sorry, that was the wrong patch -.-
>> 
>> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
>> index de5108430aa..fdc09a939be 100644
>> --- a/lisp/emacs-lisp/package.el
>> +++ b/lisp/emacs-lisp/package.el
>> @@ -739,7 +739,7 @@ package-load-all-descriptors
>>  updates `package-alist'."
>>    (dolist (dir (cons package-user-dir package-directory-list))
>>      (when (file-directory-p dir)
>> -      (dolist (pkg-dir (directory-files dir t "\\`[^.]" t))
>> +      (dolist (pkg-dir (directory-files dir t "\\`[^.]"))
>>          (when (file-directory-p pkg-dir)
>>            (package-load-descriptor pkg-dir))))))
>>  
>> (I attached the *vc-diff* buffer, but before sending the message I
>> invoked vc-diff again which modified the buffer contents.  Should have
>> just copied the diff manually...)
>
> Thanks, now it at least makes sense.
>
> But why does it matter whether the package directory names are sorted
> or not?  Is sorting package versions the same as sorting the names of
> their directories?

No, because directories are sorted by their names and by that metric,
"foo-10" is less than "foo-9".  But since package archives like MELPA
use ISO 8601-like release-dates as version numbers, the order "works".
If we disable sorting, the files returned in any order the operating
system might have arbitrary stored the directory entries in which might
even change depending on the file system.  So at the very least we can
argue that while sorting does not solve the issue, it provides a quick
hack to prevent sudden breakage from Emacs 28 to Emacs 29, due to users
relying on this kind of behaviour.  A proper solution, that takes
version numbers into account should be prepared on the master branch.

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

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: artscan@list.ru,  monnier@iro.umontreal.ca,  63757@debbugs.gnu.org
> Date: Sun, 04 Jun 2023 12:32:17 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> -      (dolist (pkg-dir (directory-files dir t "\\`[^.]" t))
> >> +      (dolist (pkg-dir (directory-files dir t "\\`[^.]"))
> >>          (when (file-directory-p pkg-dir)
> >>            (package-load-descriptor pkg-dir))))))
> >>  
> >> (I attached the *vc-diff* buffer, but before sending the message I
> >> invoked vc-diff again which modified the buffer contents.  Should have
> >> just copied the diff manually...)
> >
> > Thanks, now it at least makes sense.
> >
> > But why does it matter whether the package directory names are sorted
> > or not?  Is sorting package versions the same as sorting the names of
> > their directories?
> 
> No, because directories are sorted by their names and by that metric,
> "foo-10" is less than "foo-9".  But since package archives like MELPA
> use ISO 8601-like release-dates as version numbers, the order "works".
> If we disable sorting, the files returned in any order the operating
> system might have arbitrary stored the directory entries in which might
> even change depending on the file system.  So at the very least we can
> argue that while sorting does not solve the issue, it provides a quick
> hack to prevent sudden breakage from Emacs 28 to Emacs 29, due to users
> relying on this kind of behaviour.  A proper solution, that takes
> version numbers into account should be prepared on the master branch.

OK, thanks.  Please install on emacs-29, and let's hope this doesn't
cause new problems.

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

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: artscan@list.ru,  monnier@iro.umontreal.ca,  63757@debbugs.gnu.org
>> Date: Sun, 04 Jun 2023 12:32:17 +0000
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> -      (dolist (pkg-dir (directory-files dir t "\\`[^.]" t))
>> >> +      (dolist (pkg-dir (directory-files dir t "\\`[^.]"))
>> >>          (when (file-directory-p pkg-dir)
>> >>            (package-load-descriptor pkg-dir))))))
>> >>  
>> >> (I attached the *vc-diff* buffer, but before sending the message I
>> >> invoked vc-diff again which modified the buffer contents.  Should have
>> >> just copied the diff manually...)
>> >
>> > Thanks, now it at least makes sense.
>> >
>> > But why does it matter whether the package directory names are sorted
>> > or not?  Is sorting package versions the same as sorting the names of
>> > their directories?
>> 
>> No, because directories are sorted by their names and by that metric,
>> "foo-10" is less than "foo-9".  But since package archives like MELPA
>> use ISO 8601-like release-dates as version numbers, the order "works".
>> If we disable sorting, the files returned in any order the operating
>> system might have arbitrary stored the directory entries in which might
>> even change depending on the file system.  So at the very least we can
>> argue that while sorting does not solve the issue, it provides a quick
>> hack to prevent sudden breakage from Emacs 28 to Emacs 29, due to users
>> relying on this kind of behaviour.  A proper solution, that takes
>> version numbers into account should be prepared on the master branch.
>
> OK, thanks.  Please install on emacs-29, and let's hope this doesn't
> cause new problems.

Done.  Should we close the issue then, or wait for more feedback?

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]

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

> Done.  Should we close the issue then, or wait for more feedback?

fa8135f8916 * emacs-29 origin/emacs-29 Revert changes to the order in which
package descs are loaded
-      (dolist (pkg-dir (directory-files dir t "\\`[^.]" t))
+      (dolist (pkg-dir (directory-files dir t "\\`[^.]"))

it doesn't fix the initial problem: `load-path` and `package-alist` are
still broken in my test.

Test:
To reproduce the case we need emacs 29.0.91 + fa8135f8916 fix  and
    ~/.emacs.d/elpa/go-mode-20170726.555/...
    ~/.emacs.d/elpa/go-mode-20220114.2239/...
Run `emacs -Q` and `M-x package-initialize`. After that `package-alist` and
`load-path` contain the 2017 version as the first item in the list (instead
of 2022).

It can be fixed like this:

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

But if we leave it until 29.2, I will not insist :)


On Sun, Jun 4, 2023 at 11:21 PM Philip Kaludercic <philipk@posteo.net>
wrote:

> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> From: Philip Kaludercic <philipk@posteo.net>
> >> Cc: artscan@list.ru,  monnier@iro.umontreal.ca,  63757@debbugs.gnu.org
> >> Date: Sun, 04 Jun 2023 12:32:17 +0000
> >>
> >> Eli Zaretskii <eliz@gnu.org> writes:
> >>
> >> >> -      (dolist (pkg-dir (directory-files dir t "\\`[^.]" t))
> >> >> +      (dolist (pkg-dir (directory-files dir t "\\`[^.]"))
> >> >>          (when (file-directory-p pkg-dir)
> >> >>            (package-load-descriptor pkg-dir))))))
> >> >>
> >> >> (I attached the *vc-diff* buffer, but before sending the message I
> >> >> invoked vc-diff again which modified the buffer contents.  Should
> have
> >> >> just copied the diff manually...)
> >> >
> >> > Thanks, now it at least makes sense.
> >> >
> >> > But why does it matter whether the package directory names are sorted
> >> > or not?  Is sorting package versions the same as sorting the names of
> >> > their directories?
> >>
> >> No, because directories are sorted by their names and by that metric,
> >> "foo-10" is less than "foo-9".  But since package archives like MELPA
> >> use ISO 8601-like release-dates as version numbers, the order "works".
> >> If we disable sorting, the files returned in any order the operating
> >> system might have arbitrary stored the directory entries in which might
> >> even change depending on the file system.  So at the very least we can
> >> argue that while sorting does not solve the issue, it provides a quick
> >> hack to prevent sudden breakage from Emacs 28 to Emacs 29, due to users
> >> relying on this kind of behaviour.  A proper solution, that takes
> >> version numbers into account should be prepared on the master branch.
> >
> > OK, thanks.  Please install on emacs-29, and let's hope this doesn't
> > cause new problems.
>
> Done.  Should we close the issue then, or wait for more feedback?
>


-- 
__________________________

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

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

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

[Philip Kaludercic]

Евгений Бойков <artscan@list.ru> writes:

>> Done.  Should we close the issue then, or wait for more feedback?
>
> fa8135f8916 * emacs-29 origin/emacs-29 Revert changes to the order in which
> package descs are loaded
> -      (dolist (pkg-dir (directory-files dir t "\\`[^.]" t))
> +      (dolist (pkg-dir (directory-files dir t "\\`[^.]"))
>
> it doesn't fix the initial problem: `load-path` and `package-alist` are
> still broken in my test.

What does "broken" mean in this case?  I fear that your patch just
changes what is necessary to resolve the issue in your case, but the
underlying problem (the destructive modification of `package-list')
still persists?

> Test:
> To reproduce the case we need emacs 29.0.91 + fa8135f8916 fix  and
>     ~/.emacs.d/elpa/go-mode-20170726.555/...
>     ~/.emacs.d/elpa/go-mode-20220114.2239/...
> Run `emacs -Q` and `M-x package-initialize`. After that `package-alist` and
> `load-path` contain the 2017 version as the first item in the list (instead
> of 2022).
>
> It can be fixed like this:
>
> 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
>
> But if we leave it until 29.2, I will not insist :)
>
>
> On Sun, Jun 4, 2023 at 11:21 PM Philip Kaludercic <philipk@posteo.net>
> wrote:
>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>> >> From: Philip Kaludercic <philipk@posteo.net>
>> >> Cc: artscan@list.ru,  monnier@iro.umontreal.ca,  63757@debbugs.gnu.org
>> >> Date: Sun, 04 Jun 2023 12:32:17 +0000
>> >>
>> >> Eli Zaretskii <eliz@gnu.org> writes:
>> >>
>> >> >> -      (dolist (pkg-dir (directory-files dir t "\\`[^.]" t))
>> >> >> +      (dolist (pkg-dir (directory-files dir t "\\`[^.]"))
>> >> >>          (when (file-directory-p pkg-dir)
>> >> >>            (package-load-descriptor pkg-dir))))))
>> >> >>
>> >> >> (I attached the *vc-diff* buffer, but before sending the message I
>> >> >> invoked vc-diff again which modified the buffer contents.  Should
>> have
>> >> >> just copied the diff manually...)
>> >> >
>> >> > Thanks, now it at least makes sense.
>> >> >
>> >> > But why does it matter whether the package directory names are sorted
>> >> > or not?  Is sorting package versions the same as sorting the names of
>> >> > their directories?
>> >>
>> >> No, because directories are sorted by their names and by that metric,
>> >> "foo-10" is less than "foo-9".  But since package archives like MELPA
>> >> use ISO 8601-like release-dates as version numbers, the order "works".
>> >> If we disable sorting, the files returned in any order the operating
>> >> system might have arbitrary stored the directory entries in which might
>> >> even change depending on the file system.  So at the very least we can
>> >> argue that while sorting does not solve the issue, it provides a quick
>> >> hack to prevent sudden breakage from Emacs 28 to Emacs 29, due to users
>> >> relying on this kind of behaviour.  A proper solution, that takes
>> >> version numbers into account should be prepared on the master branch.
>> >
>> > OK, thanks.  Please install on emacs-29, and let's hope this doesn't
>> > cause new problems.
>>
>> Done.  Should we close the issue then, or wait for more feedback?
>>

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

[Eli Zaretskii]

> From: Евгений Бойков <artscan@list.ru>
> Date: Mon, 5 Jun 2023 00:03:48 +1000
> Cc: Eli Zaretskii <eliz@gnu.org>, monnier@iro.umontreal.ca, 63757@debbugs.gnu.org
> 
> > Done.  Should we close the issue then, or wait for more feedback?
> 
> fa8135f8916 * emacs-29 origin/emacs-29 Revert changes to the order in which package descs are
> loaded
> -      (dolist (pkg-dir (directory-files dir t "\\`[^.]" t))
> +      (dolist (pkg-dir (directory-files dir t "\\`[^.]"))
> 
> it doesn't fix the initial problem: `load-path` and `package-alist` are still broken in my test.
> 
> Test:
> To reproduce the case we need emacs 29.0.91 + fa8135f8916 fix  and
>     ~/.emacs.d/elpa/go-mode-20170726.555/...
>     ~/.emacs.d/elpa/go-mode-20220114.2239/...
> Run `emacs -Q` and `M-x package-initialize`. After that `package-alist` and `load-path` contain the
> 2017 version as the first item in the list (instead of 2022).

I guess we need to reverse the list of directories, since
directory-files sorts using string-lessp as the predicate?

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]

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

> I guess we need to reverse the list of directories, since
> directory-files sorts using string-lessp as the predicate?
Unfortunately, `directory-files` order in `package-load-all-descriptors` is
later overlapped by `package--get-activatable-pkg` order which sorts
`package-alist` in-place.

> What does "broken" mean in this case?  I fear that your patch just
> changes what is necessary to resolve the issue in your case, but the
> underlying problem (the destructive modification of `package-list')
> still persists?

"Broken" means wrong order of versions.
As you can see in `package-process-define-package`

           (old-pkgs (assq name package-alist)))
...
              (push new-pkg-desc (cdr old-pkgs))

gives destructive modification of `package-alist` too.
We can put this destructive modification in one place, as Stefan noted,
like this

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 Mon, Jun 5, 2023 at 12:29 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Евгений Бойков <artscan@list.ru>
> > Date: Mon, 5 Jun 2023 00:03:48 +1000
> > Cc: Eli Zaretskii <eliz@gnu.org>, monnier@iro.umontreal.ca,
> 63757@debbugs.gnu.org
> >
> > > Done.  Should we close the issue then, or wait for more feedback?
> >
> > fa8135f8916 * emacs-29 origin/emacs-29 Revert changes to the order in
> which package descs are
> > loaded
> > -      (dolist (pkg-dir (directory-files dir t "\\`[^.]" t))
> > +      (dolist (pkg-dir (directory-files dir t "\\`[^.]"))
> >
> > it doesn't fix the initial problem: `load-path` and `package-alist` are
> still broken in my test.
> >
> > Test:
> > To reproduce the case we need emacs 29.0.91 + fa8135f8916 fix  and
> >     ~/.emacs.d/elpa/go-mode-20170726.555/...
> >     ~/.emacs.d/elpa/go-mode-20220114.2239/...
> > Run `emacs -Q` and `M-x package-initialize`. After that `package-alist`
> and `load-path` contain the
> > 2017 version as the first item in the list (instead of 2022).
>
> I guess we need to reverse the list of directories, since
> directory-files sorts using string-lessp as the predicate?
>


-- 
__________________________

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

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

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

[Eli Zaretskii]

> From: Евгений Бойков <artscan@list.ru>
> Date: Mon, 5 Jun 2023 00:47:43 +1000
> Cc: philipk@posteo.net, monnier@iro.umontreal.ca, 63757@debbugs.gnu.org
> 
> > I guess we need to reverse the list of directories, since
> > directory-files sorts using string-lessp as the predicate?
> Unfortunately, `directory-files` order in `package-load-all-descriptors` is later overlapped by
> `package--get-activatable-pkg` order which sorts `package-alist` in-place.

And why is that sorting a problem?

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]

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

> Why 2 loops?
Because I just incrementally improved my variants and mechanically moved
the code from `package--get-activatable-pkg` to
`package-process-define-package` in hope of next improvements.

> And why is that sorting a problem?
Maybe putting it to docs as a "known bug" would be better.
It is just unintuitive behaviour.
I understand that tested code is the priority.
On the other hand, if the fb87d500 feature is little enough, why not revert
and pretest without it? Or fix it and pretest fixed version.

> Евгений, does this change fix your use case?
Yes, reverted fb87d500 fixes the case too.

On Mon, Jun 5, 2023 at 12:55 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Евгений Бойков <artscan@list.ru>
> > Date: Mon, 5 Jun 2023 00:47:43 +1000
> > Cc: philipk@posteo.net, monnier@iro.umontreal.ca, 63757@debbugs.gnu.org
> >
> > > I guess we need to reverse the list of directories, since
> > > directory-files sorts using string-lessp as the predicate?
> > Unfortunately, `directory-files` order in `package-load-all-descriptors`
> is later overlapped by
> > `package--get-activatable-pkg` order which sorts `package-alist`
> in-place.
>
> And why is that sorting a problem?
>


-- 
__________________________

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

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

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> It can be fixed like this:
>
> 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

Sounds about right.  Except:

- We still need a `copy-sequence` to avoid `sort` side-effecting the
  `package-alist` contents.
- I'm not sure the change of `package-disabled-p` =>
  `package-built-in-p` is right: it makes the code obey the comment, but
  I don't think we want to prefer built-in packages.

With these two tweaks, I suspect it might actually really fix the bug in
a way that should be safe enough for `emacs-29`.


        Stefan

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

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Philip Kaludercic <philipk@posteo.net>,  Eli Zaretskii <eliz@gnu.org>,
>   63757@debbugs.gnu.org
> Date: Sun, 04 Jun 2023 11:12:31 -0400
> 
> Sounds about right.  Except:
> 
> - We still need a `copy-sequence` to avoid `sort` side-effecting the
>   `package-alist` contents.
> - I'm not sure the change of `package-disabled-p` =>
>   `package-built-in-p` is right: it makes the code obey the comment, but
>   I don't think we want to prefer built-in packages.
> 
> With these two tweaks, I suspect it might actually really fix the bug in
> a way that should be safe enough for `emacs-29`.

I'd need to see the actual patch, but right now I'm inclined to leave
this problem unsolved in Emacs 29.  It only happens for people who
don't uninstall older versions of packages they install, and that's
not the workflow we support in Emacs 29.  The offending code was with
us since Aug 2022, so replacing it with something at the last moment
needs a very good reason.

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> I'd need to see the actual patch, but right now I'm inclined to leave
> this problem unsolved in Emacs 29.  It only happens for people who
> don't uninstall older versions of packages they install, and that's
> not the workflow we support in Emacs 29.  The offending code was with
> us since Aug 2022, so replacing it with something at the last moment
> needs a very good reason.

Actually, here's the patch that I'd suggest.
It reverts the code to its Jan 2021 state, which, in my book, makes it
safe enough.

The current sort predicate is quite severely broken, so "no sort" can't
be worse (especially since the list is supposed to be sorted already).
Also it re-instates the "prefer a builtin package" which was lost along
the way.


        Stefan


diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 8f266186d5e..7d5946fbfa9 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -924,22 +924,14 @@ package-activate-1
 
 (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
           (let ((available-version (package-desc-version (car pkg-descs))))
-            (package-disabled-p pkg-name available-version)))
+            (or (package-disabled-p pkg-name available-version)
+                ;; Prefer a builtin package.
+                (package-built-in-p pkg-name available-version))))
       (setq pkg-descs (cdr pkg-descs)))
     (car pkg-descs)))
 

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

[Philip Kaludercic]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I'd need to see the actual patch, but right now I'm inclined to leave
>> this problem unsolved in Emacs 29.  It only happens for people who
>> don't uninstall older versions of packages they install, and that's
>> not the workflow we support in Emacs 29.  The offending code was with
>> us since Aug 2022, so replacing it with something at the last moment
>> needs a very good reason.
>
> Actually, here's the patch that I'd suggest.
> It reverts the code to its Jan 2021 state, which, in my book, makes it
> safe enough.
>
> The current sort predicate is quite severely broken, so "no sort" can't
> be worse (especially since the list is supposed to be sorted already).
> Also it re-instates the "prefer a builtin package" which was lost along
> the way.

Together with the sorting patch from above, it seems this would also
work for VC packages, since "foo" is always listed before "foo-...".

>         Stefan
>
>
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index 8f266186d5e..7d5946fbfa9 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -924,22 +924,14 @@ package-activate-1
>  
>  (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
>            (let ((available-version (package-desc-version (car pkg-descs))))
> -            (package-disabled-p pkg-name available-version)))
> +            (or (package-disabled-p pkg-name available-version)
> +                ;; Prefer a builtin package.
> +                (package-built-in-p pkg-name available-version))))
>        (setq pkg-descs (cdr pkg-descs)))
>      (car pkg-descs)))

-- 
Philip Kaludercic

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

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  artscan@list.ru,  63757@debbugs.gnu.org
> Date: Sun, 04 Jun 2023 16:15:10 +0000
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> > Actually, here's the patch that I'd suggest.
> > It reverts the code to its Jan 2021 state, which, in my book, makes it
> > safe enough.
> >
> > The current sort predicate is quite severely broken, so "no sort" can't
> > be worse (especially since the list is supposed to be sorted already).
> > Also it re-instates the "prefer a builtin package" which was lost along
> > the way.
> 
> Together with the sorting patch from above, it seems this would also
> work for VC packages, since "foo" is always listed before "foo-...".

So what exactly was the change installed back in Aug 2022 supposed to
improve?

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> So what exactly was the change installed back in Aug 2022 supposed to
> improve?

I believe its intention was only to give preference to the VC packages
(I still don't know why it moved the "prefer built-in packages" part
(and then changed it to test `package-disabled-p`); I presume it was an
accident).

So my patch *may* be a regression for users of `package-vc`, but since
it's a new feature in Emacs-29, I think it's perfectly acceptable.
Especially because the sort it implemented is wrong (because the sort
predicate fails to define a partial order, so the output will depend on
arbitrary details of how `sort` happens to pass elements to the
predicate), so it's not at all clear that it does what it intends to
do anyway.
Furthermore I believe that it is mostly redundant: VC packages should
almost always already be sorted first by virtue of the version being
almost always higher than non-VC packages.


        Stefan

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

[Philip Kaludercic]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> So what exactly was the change installed back in Aug 2022 supposed to
>> improve?
>
> I believe its intention was only to give preference to the VC packages
> (I still don't know why it moved the "prefer built-in packages" part
> (and then changed it to test `package-disabled-p`); I presume it was an
> accident).

This is true, it was my mistake made when refactoring package-vc.

> So my patch *may* be a regression for users of `package-vc`, but since
> it's a new feature in Emacs-29, I think it's perfectly acceptable.
> Especially because the sort it implemented is wrong (because the sort
> predicate fails to define a partial order, so the output will depend on
> arbitrary details of how `sort` happens to pass elements to the
> predicate), so it's not at all clear that it does what it intends to
> do anyway.
> Furthermore I believe that it is mostly redundant: VC packages should
> almost always already be sorted first by virtue of the version being
> almost always higher than non-VC packages.

I'll do some more testing to see if everything still works, but from
what I see it seems like it is doing the right thing.  The issue I was
trying to solve back then did not exist.

-- 
Philip Kaludercic

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

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  artscan@list.ru,  63757@debbugs.gnu.org
> Date: Sun, 04 Jun 2023 17:37:16 +0000
> 
> > So my patch *may* be a regression for users of `package-vc`, but since
> > it's a new feature in Emacs-29, I think it's perfectly acceptable.
> > Especially because the sort it implemented is wrong (because the sort
> > predicate fails to define a partial order, so the output will depend on
> > arbitrary details of how `sort` happens to pass elements to the
> > predicate), so it's not at all clear that it does what it intends to
> > do anyway.
> > Furthermore I believe that it is mostly redundant: VC packages should
> > almost always already be sorted first by virtue of the version being
> > almost always higher than non-VC packages.
> 
> I'll do some more testing to see if everything still works, but from
> what I see it seems like it is doing the right thing.  The issue I was
> trying to solve back then did not exist.

OK, please ack when you think it is okay to revert that change.

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Together with the sorting patch from above, it seems this would also
> work for VC packages, since "foo" is always listed before "foo-...".

The `directory-files` sort should make no difference since its output is
fed to the sort of `package-process-define-package`.


        Stefan

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

[Philip Kaludercic]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Together with the sorting patch from above, it seems this would also
>> work for VC packages, since "foo" is always listed before "foo-...".
>
> The `directory-files` sort should make no difference since its output is
> fed to the sort of `package-process-define-package`.

Just to make sure I am not missing something in that case:
package-load-all-descriptors reads package descriptors either in
alphabetical order or in the order the operating system will list them
in the elpa/ directory.  Via package-load-descriptor, the descriptor
expression is passed to the package-process-define-package, which will
insert them into package-alist according to the version number.

If this is so, then if the package developer does not bump the commit to
something else after releasing the package on GNU ELPA, then order of
the VC package vs. the tarball package would depend on order in which
they are returned from via `directory-files', since their version
according to `version-list-=' would be the same?
                         
-- 
Philip Kaludercic

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Just to make sure I am not missing something in that case:
> package-load-all-descriptors reads package descriptors either in
> alphabetical order or in the order the operating system will list them
> in the elpa/ directory.  Via package-load-descriptor, the descriptor
> expression is passed to the package-process-define-package, which will
> insert them into package-alist according to the version number.

That's right.

> If this is so, then if the package developer does not bump the commit to
> something else after releasing the package on GNU ELPA, then order of
> the VC package vs. the tarball package would depend on order in which
> they are returned from via `directory-files', since their version
> according to `version-list-=' would be the same?

If two installed packages have the same version, then indeed the sort
order between them is "arbitrary" (it currently happens to put in first
position the first package encountered, which may depend on the
directory-files sort order but also on the order the packages are found
along `package-directory-list` if they're not in the same dir).

So, to better handle the scenario you describe, we'd need/want something
like the 100% untested patch below.


        Stefan


diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ba0e3618f28..01936a4fe0a 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -698,8 +698,10 @@ package-process-define-package
         ;; 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))))
+                     (let ((oversion (package-desc-version (cadr old-pkgs)))))
+                     (or (version-list-< version oversion)
+                         (and (equal version oversion)
+                              (package-vc-p new-pkg-desc))))
                 (setq old-pkgs (cdr old-pkgs))
               (push new-pkg-desc (cdr old-pkgs))
               nil)))

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

[Philip Kaludercic]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Just to make sure I am not missing something in that case:
>> package-load-all-descriptors reads package descriptors either in
>> alphabetical order or in the order the operating system will list them
>> in the elpa/ directory.  Via package-load-descriptor, the descriptor
>> expression is passed to the package-process-define-package, which will
>> insert them into package-alist according to the version number.
>
> That's right.
>
>> If this is so, then if the package developer does not bump the commit to
>> something else after releasing the package on GNU ELPA, then order of
>> the VC package vs. the tarball package would depend on order in which
>> they are returned from via `directory-files', since their version
>> according to `version-list-=' would be the same?
>
> If two installed packages have the same version, then indeed the sort
> order between them is "arbitrary" (it currently happens to put in first
> position the first package encountered, which may depend on the
> directory-files sort order but also on the order the packages are found
> along `package-directory-list` if they're not in the same dir).

Ok.

> So, to better handle the scenario you describe, we'd need/want something
> like the 100% untested patch below.
>
>
>         Stefan
>
>
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index ba0e3618f28..01936a4fe0a 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -698,8 +698,10 @@ package-process-define-package
>          ;; 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))))
> +                     (let ((oversion (package-desc-version (cadr old-pkgs)))))
> +                     (or (version-list-< version oversion)
> +                         (and (equal version oversion)
> +                              (package-vc-p new-pkg-desc))))

But in this case, if a regular package is upgraded, and a VC package
weren't (even though the user might have written some patch they are
relying on), it would be suddenly switch when starting up Emacs at some
point.

>                  (setq old-pkgs (cdr old-pkgs))
>                (push new-pkg-desc (cdr old-pkgs))
>                nil)))

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> But in this case, if a regular package is upgraded, and a VC package
> weren't (even though the user might have written some patch they are
> relying on), it would be suddenly switch when starting up Emacs at some
> point.

Yup.  AFAIK both behaviors (staying with the VC package, and
switching to the non-VC package) can be "incorrect", depending on
the circumstances.  So I don't see this as "a bug".

Note also that in the session where we perform the install of the
upgraded package, the upgraded package will take precedence over the VC
package, so it's mildly more consistent to do the same at startup.

If the user wants to keep using the older VC package, there's always to
option to use `package-pinned-packages` or to remove the non-VC package.

If we want to do better, rather than forcing VC packages to the top we
should detect when the ordering between VC and non-VC packages changes
and emit a warning or prompt the user.


        Stefan

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

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  artscan@list.ru,  63757@debbugs.gnu.org
> Date: Mon, 05 Jun 2023 10:55:50 -0400
> 
> > But in this case, if a regular package is upgraded, and a VC package
> > weren't (even though the user might have written some patch they are
> > relying on), it would be suddenly switch when starting up Emacs at some
> > point.
> 
> Yup.  AFAIK both behaviors (staying with the VC package, and
> switching to the non-VC package) can be "incorrect", depending on
> the circumstances.  So I don't see this as "a bug".
> 
> Note also that in the session where we perform the install of the
> upgraded package, the upgraded package will take precedence over the VC
> package, so it's mildly more consistent to do the same at startup.
> 
> If the user wants to keep using the older VC package, there's always to
> option to use `package-pinned-packages` or to remove the non-VC package.
> 
> If we want to do better, rather than forcing VC packages to the top we
> should detect when the ordering between VC and non-VC packages changes
> and emit a warning or prompt the user.

So what's the bottom line here?  Do we have an agreed-upon patch that
should be installed on the emacs-29 branch?  If so, can it be
installed soon, please?  Because I'd like to put out another Emacs 29
pretest in a couple of days.

If there's no agreement yet, can someone please spell out what is yet
to be done here?

Thanks.

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

[Philip Kaludercic]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: Eli Zaretskii <eliz@gnu.org>,  artscan@list.ru,  63757@debbugs.gnu.org
>> Date: Mon, 05 Jun 2023 10:55:50 -0400
>> 
>> > But in this case, if a regular package is upgraded, and a VC package
>> > weren't (even though the user might have written some patch they are
>> > relying on), it would be suddenly switch when starting up Emacs at some
>> > point.
>> 
>> Yup.  AFAIK both behaviors (staying with the VC package, and
>> switching to the non-VC package) can be "incorrect", depending on
>> the circumstances.  So I don't see this as "a bug".
>> 
>> Note also that in the session where we perform the install of the
>> upgraded package, the upgraded package will take precedence over the VC
>> package, so it's mildly more consistent to do the same at startup.
>> 
>> If the user wants to keep using the older VC package, there's always to
>> option to use `package-pinned-packages` or to remove the non-VC package.
>> 
>> If we want to do better, rather than forcing VC packages to the top we
>> should detect when the ordering between VC and non-VC packages changes
>> and emit a warning or prompt the user.
>
> So what's the bottom line here?  Do we have an agreed-upon patch that
> should be installed on the emacs-29 branch?  If so, can it be
> installed soon, please?  Because I'd like to put out another Emacs 29
> pretest in a couple of days.

I think that Stefan's suggestion is the best way to go forward for now:


[-- Attachment #2: Type: text/plain, Size: 1912 bytes --]

From f47c9cdfa4f55a168246bd4fc19ad6c897e8906b Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Wed, 7 Jun 2023 20:37:13 +0200
Subject: [PATCH] ; Restore old behaviour of 'package--get-activatable-pkg'

* lisp/emacs-lisp/package.el (package--get-activatable-pkg): Do not
sort 'package-alist' to prefer VC packages.  (Bug#63757)
---
 lisp/emacs-lisp/package.el | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 43368b59522..3e6acd9b388 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -924,22 +924,14 @@ package-activate-1
 
 (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
           (let ((available-version (package-desc-version (car pkg-descs))))
-            (package-disabled-p pkg-name available-version)))
+            (or (package-disabled-p pkg-name available-version)
+                ;; Prefer a builtin package.
+                (package-built-in-p pkg-name available-version))))
       (setq pkg-descs (cdr pkg-descs)))
     (car pkg-descs)))
 
-- 
2.39.2


[-- Attachment #3: Type: text/plain, Size: 104 bytes --]


> If there's no agreement yet, can someone please spell out what is yet
> to be done here?
>
> Thanks.

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> I think that Stefan's suggestion is the best way to go forward for now:

Indeed, I think the rest of the discussion is for `master` not for `emacs-29`.


        Stefan

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

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  artscan@list.ru,  63757@debbugs.gnu.org
> Date: Wed, 07 Jun 2023 15:20:56 -0400
> 
> > I think that Stefan's suggestion is the best way to go forward for now:
> 
> Indeed, I think the rest of the discussion is for `master` not for `emacs-29`.

OK, so I've now reverted the offending commit, and I'm marking this
bug done.

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

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: artscan@list.ru,  philipk@posteo.net,  63757@debbugs.gnu.org
> Date: Sun, 04 Jun 2023 11:47:15 -0400
> 
> > I'd need to see the actual patch, but right now I'm inclined to leave
> > this problem unsolved in Emacs 29.  It only happens for people who
> > don't uninstall older versions of packages they install, and that's
> > not the workflow we support in Emacs 29.  The offending code was with
> > us since Aug 2022, so replacing it with something at the last moment
> > needs a very good reason.
> 
> Actually, here's the patch that I'd suggest.
> It reverts the code to its Jan 2021 state, which, in my book, makes it
> safe enough.
> 
> The current sort predicate is quite severely broken, so "no sort" can't
> be worse (especially since the list is supposed to be sorted already).
> Also it re-instates the "prefer a builtin package" which was lost along
> the way.

Thanks, Stefan.

Philip, you installed the change that Stefan suggests to revert.  What
will this mean for the use cases which your change was supposed to
improve?  Will anything be worse or broken?

Евгений, does this change fix your use case?

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> - I'm not sure the change of `package-disabled-p` =>
>   `package-built-in-p` is right: it makes the code obey the comment, but
>   I don't think we want to prefer built-in packages.

Scratch that, I see I was misremembering how `package-built-in-p` works,
this part of the change might be OK, actually.


        Stefan

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> OK, thanks.  Please install on emacs-29, and let's hope this doesn't
> cause new problems.

No please don't.  It doesn't fix the problem, it just hides it a bit
further but will still bite some users.


        Stefan

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

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Philip Kaludercic <philipk@posteo.net>,  artscan@list.ru,
>   63757@debbugs.gnu.org
> Date: Sun, 04 Jun 2023 10:46:05 -0400
> 
> > OK, thanks.  Please install on emacs-29, and let's hope this doesn't
> > cause new problems.
> 
> No please don't.  It doesn't fix the problem, it just hides it a bit
> further but will still bite some users.

What do you suggest instead, that is safe enough for emacs-29?  The
only alternative I see is leave this unsolved, since the issue happens
only if someone installs new versions of packages by hand without
deleting the old versions.  Are there any other alternatives that you
see for the release branch?

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> OK, thanks.  Please install on emacs-29, and let's hope this doesn't
>> cause new problems.
> No please don't.  It doesn't fix the problem, it just hides it a bit
> further but will still bite some users.

Actually, I believe this patch will have no effect at all (because its
sorting is overridden by the subsequent sorting that takes place in
`package-process-define-package`).

This said if this dir-sort usually puts older versions first, then maybe
it's a good change: it increases the chance that the "sorted insertion"
used in `package-process-define-package` the can be done in O(1), so it
might speed this up a tiny-teeny bit, sometimes.


        Stefan

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]

[-- 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 --]

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]

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

Sort didn't exist before fb87d500

-  (let ((pkg-descs (cdr (assq pkg-name package-alist))))
+  (let ((pkg-descs (sort (cdr (assq pkg-name package-alist))

We can fix incorrect sorting

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ba0e3618f28..228d77b54e7 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -916,17 +916,18 @@ 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
+         ;; Prefer VC packages
+         (apply 'append
+                (cl-reduce
+                 (lambda (p1 p2)
+                   (if (package-vc-p p1)
+                       (push p1 (cl-first p2))
+                     (push p1 (cl-second p2)))
+                   p2)
+                 (cdr (assq pkg-name package-alist))
+                 :initial-value (list nil nil)
+                 :from-end t))))
     ;; Check if PACKAGE is available in `package-alist'.
     (while
         (when pkg-descs

or just revert fb87d500

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

> > From: Евгений Бойков <artscan@list.ru>
> > Date: Sun, 4 Jun 2023 15:37:31 +1000
> > Cc: Philip Kaludercic <philipk@posteo.net>, Eli Zaretskii <eliz@gnu.org>,
> 63757@debbugs.gnu.org
> >
> > Excuse me for extra messages, I've already found a bug in the previous
> patch.
> > I need a review, especially with in-place operations.
> >
> > move "Prefer VC packages" to `package-process-define-package`
> > drop `package-disabled-p` and "Prefer builtin packages" comment
>
> Thanks, but I'm not really comfortable with such non-trivial changes
> in package.el at this late stage of the pretest.
>
> Is this problem new in Emacs 29, or did it exist in Emacs 28 as well?
> If it is new, which change(s) caused the incorrect ordering of
> versions in Emacs 29?
>


-- 
__________________________

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

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

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

[Eli Zaretskii]

> From: Евгений Бойков <artscan@list.ru>
> Date: Sun, 4 Jun 2023 17:52:57 +1000
> Cc: monnier@iro.umontreal.ca, philipk@posteo.net, 63757@debbugs.gnu.org
> 
> Sort didn't exist before fb87d500
> 
> -  (let ((pkg-descs (cdr (assq pkg-name package-alist))))
> +  (let ((pkg-descs (sort (cdr (assq pkg-name package-alist))
> 
> We can fix incorrect sorting 

I wonder how come this issue never came up until now, in all the time
since commit fb87d500, 9 months ago?  What are you, Евгений, do that
bumps into this issue, that other people don't do?  Or does this
affect only very specific packages?

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]

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

> I wonder how come this issue never came up until now, in all the time
> since commit fb87d500, 9 months ago?  What are you, Евгений, do that
> bumps into this issue, that other people don't do?  Or does this
> affect only very specific packages?

I moved my everyday working emacs instance to 29.0.91 and got almost 2%
completely broken packages.
I rarely delete really old versions from my ~/.emacs.d/elpa.
If your package-user-dir is cleaned more often, you may not notice that
your loaded packages have slightly different old versions.
This can affect any package if it has few versions.


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

> > From: Евгений Бойков <artscan@list.ru>
> > Date: Sun, 4 Jun 2023 17:52:57 +1000
> > Cc: monnier@iro.umontreal.ca, philipk@posteo.net, 63757@debbugs.gnu.org
> >
> > Sort didn't exist before fb87d500
> >
> > -  (let ((pkg-descs (cdr (assq pkg-name package-alist))))
> > +  (let ((pkg-descs (sort (cdr (assq pkg-name package-alist))
> >
> > We can fix incorrect sorting
>
> I wonder how come this issue never came up until now, in all the time
> since commit fb87d500, 9 months ago?  What are you, Евгений, do that
> bumps into this issue, that other people don't do?  Or does this
> affect only very specific packages?
>


-- 
__________________________

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

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

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

[Eli Zaretskii]

> From: Евгений Бойков <artscan@list.ru>
> Date: Sun, 4 Jun 2023 18:43:13 +1000
> Cc: monnier@iro.umontreal.ca, philipk@posteo.net, 63757@debbugs.gnu.org
> 
> > I wonder how come this issue never came up until now, in all the time
> > since commit fb87d500, 9 months ago?  What are you, Евгений, do that
> > bumps into this issue, that other people don't do?  Or does this
> > affect only very specific packages?
> 
> I moved my everyday working emacs instance to 29.0.91 and got almost 2% completely broken
> packages.
> I rarely delete really old versions from my ~/.emacs.d/elpa.
> If your package-user-dir is cleaned more often, you may not notice that your loaded packages have
> slightly different old versions.
> This can affect any package if it has few versions.

So you are saying that other people mostly delete old versions from
~/.emacs.d/elpa/, and that's why this problem was not reported until
now, is that right?

What are our recommendations, if any, for cleaning up old versions of
ELPA packages?

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

[Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Евгений Бойков <artscan@list.ru>
>> Date: Sun, 4 Jun 2023 18:43:13 +1000
>> Cc: monnier@iro.umontreal.ca, philipk@posteo.net, 63757@debbugs.gnu.org
>> 
>> > I wonder how come this issue never came up until now, in all the time
>> > since commit fb87d500, 9 months ago?  What are you, Евгений, do that
>> > bumps into this issue, that other people don't do?  Or does this
>> > affect only very specific packages?
>> 
>> I moved my everyday working emacs instance to 29.0.91 and got almost 2% completely broken
>> packages.
>> I rarely delete really old versions from my ~/.emacs.d/elpa.
>> If your package-user-dir is cleaned more often, you may not notice that your loaded packages have
>> slightly different old versions.
>> This can affect any package if it has few versions.
>
> So you are saying that other people mostly delete old versions from
> ~/.emacs.d/elpa/, and that's why this problem was not reported until
> now, is that right?

IIRC, from another bug report, someone shared that pressing U in `M-x
list-packages' is equivalent to: "for each upgradable pacakge, install
the new version and uninstall the old version".

So, I think what Евгений did was go to the package list, find the new
version of a package of interest and install it -- _without_ also
deleting the old version.

> What are our recommendations, if any, for cleaning up old versions of
> ELPA packages?


-- 
Best,


RY

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

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Евгений Бойков <artscan@list.ru>
>> Date: Sun, 4 Jun 2023 18:43:13 +1000
>> Cc: monnier@iro.umontreal.ca, philipk@posteo.net, 63757@debbugs.gnu.org
>> 
>> > I wonder how come this issue never came up until now, in all the time
>> > since commit fb87d500, 9 months ago?  What are you, Евгений, do that
>> > bumps into this issue, that other people don't do?  Or does this
>> > affect only very specific packages?
>> 
>> I moved my everyday working emacs instance to 29.0.91 and got almost 2% completely broken
>> packages.
>> I rarely delete really old versions from my ~/.emacs.d/elpa.
>> If your package-user-dir is cleaned more often, you may not notice that your loaded packages have
>> slightly different old versions.
>> This can affect any package if it has few versions.
>
> So you are saying that other people mostly delete old versions from
> ~/.emacs.d/elpa/, and that's why this problem was not reported until
> now, is that right?

If you use regular package-* functions, upgrading always deletes and old
version when the new one is installed, so IIRC having two separate
versions of a package installed has to be the result of some manual
intervention.

> What are our recommendations, if any, for cleaning up old versions of
> ELPA packages?

M-x package-autoremove should take care of them.

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

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: Евгений Бойков <artscan@list.ru>,
>   monnier@iro.umontreal.ca,
>   63757@debbugs.gnu.org
> Date: Sun, 04 Jun 2023 09:17:46 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > So you are saying that other people mostly delete old versions from
> > ~/.emacs.d/elpa/, and that's why this problem was not reported until
> > now, is that right?
> 
> If you use regular package-* functions, upgrading always deletes and old
> version when the new one is installed, so IIRC having two separate
> versions of a package installed has to be the result of some manual
> intervention.
> 
> > What are our recommendations, if any, for cleaning up old versions of
> > ELPA packages?
> 
> M-x package-autoremove should take care of them.

So maybe we don't have to fix this on the emacs-29 branch, but can
instead install the changes on master?  And maybe consider backporting
them later for Emacs 29.2?

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]

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

> So you are saying that other people mostly delete old versions from
> ~/.emacs.d/elpa/, and that's why this problem was not reported until
> now, is that right?
Right.

> What are our recommendations, if any, for cleaning up old versions of
> ELPA packages?
I had old versions of packages mostly because I had problems with moving
between emacs versions in the past.
For example, I had to roll back some package versions along with emacs
version downgrading (after unsuccessful upgrade).
But now my config is stabilized and I just deleted old versions manually a
few days ago.

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

> > From: Евгений Бойков <artscan@list.ru>
> > Date: Sun, 4 Jun 2023 18:43:13 +1000
> > Cc: monnier@iro.umontreal.ca, philipk@posteo.net, 63757@debbugs.gnu.org
> >
> > > I wonder how come this issue never came up until now, in all the time
> > > since commit fb87d500, 9 months ago?  What are you, Евгений, do that
> > > bumps into this issue, that other people don't do?  Or does this
> > > affect only very specific packages?
> >
> > I moved my everyday working emacs instance to 29.0.91 and got almost 2%
> completely broken
> > packages.
> > I rarely delete really old versions from my ~/.emacs.d/elpa.
> > If your package-user-dir is cleaned more often, you may not notice that
> your loaded packages have
> > slightly different old versions.
> > This can affect any package if it has few versions.
>
> So you are saying that other people mostly delete old versions from
> ~/.emacs.d/elpa/, and that's why this problem was not reported until
> now, is that right?
>
> What are our recommendations, if any, for cleaning up old versions of
> ELPA packages?
>


-- 
__________________________

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

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

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> -        (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))
> +          ;; Prefer VC packages
> +          (let* ((pkg-desc0 (cdr (assq name package-alist)))
> +                 (pkg-desc
> +                  (apply 'append
> +                         (cl-reduce
> +                          (lambda (p1 p2)
> +                            (if (package-vc-p p1)
> +                                (push p1 (cl-first p2))
> +                              (push p1 (cl-second p2)))
> +                            p2)
> +                          pkg-desc0
> +                          :initial-value (list nil nil)
> +                          :from-end t))))
> +            (setf pkg-desc0 pkg-desc)
> +            (setq new-pkg-desc (car pkg-desc)))))

Why 2 loops?  The first loop is a kind of insertion sort, so what we want is
simply to replace the

    (version-list-< version (package-desc-version (cadr old-pkgs)))

test with one which places the VC packages first (tho I'm not sure why
one would need that: presumably the version in the VC package will
already be higher).

[ BTW the speed of this code can be fairly important for people who don't use
  `package-quickstart`.  I don't know if the added cost of the
  additional loop would be significant, but it's another reason to merge
  the two loops, I think.  ]


        Stefan

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> In your commit 04c4c578, package directories became NOSORT due to (fourth t
>> argument)
>>     (directory-files dir)
>> replaced by
>>     (directory-files dir t "^[^.]" t)
>>
>> It affects package-alist forming.

Can you show how `package-alist` changes for `-go-mode` for you?

>> For example, there are old and new
>> versions of a package in my elpa directory:
>>
>> go-mode-20170726.555
>> go-mode-20220114.2239
>>
>> After (package-initialize)
>> in emacs 28 (require 'go-mode) gives 2022 version, e.g. the latest version
>> of a package in most cases, directory-files default sorting works.
>> In emacs 29.0.91 I get old 2017

That shouldn't be the case (unless you specifically asked for it via
something like `package-pinned-packages`).

>> version because of NOSORT unpredictable version order in
>> package-alist and hence random paths in load-path

In its, this shouldn't make a difference, because it just changes the
order in which we consult the `<PKG>-pkg.el` descriptors to populate
`package-alist` whereas the *activation* of the `go-mode` package is the
one that should choose the one with the higher version number.

> No, this is not a feature but I think it might have uncovered a bug.
> Relying on the ordering of file names (which IIRC is lexiographic not
> numeric) is not a robust way to decide what packages should be preferred
> when loading descriptors.  Could you submit a bug report to
> bug-gnu-emacs@gnu.org so that this issue can be discussed with the other
> maintainers?

Barring bugs, `package-alist` can be filled in any order (so the `nosort`
option shouldn't make a difference): we apply the proper sorting
(i.e. newest version at the front) in `package-process-define-package`.


        Stefan

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]

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

> Can you show how `package-alist` changes for `-go-mode` for you?

To reproduce the case we need emacs 29.0.91 and

    ~/.emacs.d/elpa/go-mode-20170726.555/...
    ~/.emacs.d/elpa/go-mode-20220114.2239/...

Run `emacs -Q` and `M-x package-initialize`.

After that `package-alist` contains

   (go-mode #s(package-desc :name go-mode :version
                            (20170726 555)
                            :summary "Major mode for the Go programming
language" :reqs nil :kind nil :archive nil :dir
"/home/eab/.emacs.d/elpa/go-mode-20170726.555" :extras
                            ((:keywords "languages" "go")
                             (:url . "https://github.com/dominikh/go-mode.el
")
                             (:commit .
"99b06da201afb91e8db0c525c0f3c3590fd92fa6"))
                            :signed nil)
            #s(package-desc :name go-mode :version
                            (20220114 2239)
                            :summary "Major mode for the Go programming
language" :reqs
                            ((emacs
                              (26 1)))
                            :kind nil :archive nil :dir
"/home/eab/.emacs.d/elpa/go-mode-20220114.2239" :extras
                            ((:url . "https://github.com/dominikh/go-mode.el
")
                             (:keywords "languages" "go")
                             (:maintainer "The go-mode Authors")
                             (:authors
                              ("The go-mode Authors"))
                             (:commit .
"fa2693278637f56759480d2bf203bb8aad107230"))
                            :signed nil))

and `load-path` contains

    "/home/eab/.emacs.d/elpa/go-mode-20170726.555"

On Mon, May 29, 2023 at 11:25 PM Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> >> In your commit 04c4c578, package directories became NOSORT due to
> (fourth t
> >> argument)
> >>     (directory-files dir)
> >> replaced by
> >>     (directory-files dir t "^[^.]" t)
> >>
> >> It affects package-alist forming.
>
> Can you show how `package-alist` changes for `-go-mode` for you?
>
> >> For example, there are old and new
> >> versions of a package in my elpa directory:
> >>
> >> go-mode-20170726.555
> >> go-mode-20220114.2239
> >>
> >> After (package-initialize)
> >> in emacs 28 (require 'go-mode) gives 2022 version, e.g. the latest
> version
> >> of a package in most cases, directory-files default sorting works.
> >> In emacs 29.0.91 I get old 2017
>
> That shouldn't be the case (unless you specifically asked for it via
> something like `package-pinned-packages`).
>
> >> version because of NOSORT unpredictable version order in
> >> package-alist and hence random paths in load-path
>
> In its, this shouldn't make a difference, because it just changes the
> order in which we consult the `<PKG>-pkg.el` descriptors to populate
> `package-alist` whereas the *activation* of the `go-mode` package is the
> one that should choose the one with the higher version number.
>
> > No, this is not a feature but I think it might have uncovered a bug.
> > Relying on the ordering of file names (which IIRC is lexiographic not
> > numeric) is not a robust way to decide what packages should be preferred
> > when loading descriptors.  Could you submit a bug report to
> > bug-gnu-emacs@gnu.org so that this issue can be discussed with the other
> > maintainers?
>
> Barring bugs, `package-alist` can be filled in any order (so the `nosort`
> option shouldn't make a difference): we apply the proper sorting
> (i.e. newest version at the front) in `package-process-define-package`.
>
>
>         Stefan
>
>

-- 
__________________________

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

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

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Run `emacs -Q` and `M-x package-initialize`.
>
> After that `package-alist` contains
>
>    (go-mode #s(package-desc :name go-mode :version
>                             (20170726 555)
>                             :summary "Major mode for the Go programming language"
>                             :reqs nil :kind nil :archive nil
>                             :dir "/home/eab/.emacs.d/elpa/go-mode-20170726.555"
>                             :extras
>                             ((:keywords "languages" "go")
>                              (:url . "https://github.com/dominikh/go-mode.el")
>                              (:commit . "99b06da201afb91e8db0c525c0f3c3590fd92fa6"))
>                             :signed nil)
>             #s(package-desc :name go-mode :version
>                             (20220114 2239)
>                             :summary "Major mode for the Go programming language"
>                             :reqs ((emacs (26 1)))
>                             :kind nil :archive nil
>                             :dir "/home/eab/.emacs.d/elpa/go-mode-20220114.2239"
>                             :extras
>                             ((:url . "https://github.com/dominikh/go-mode.el")
>                              (:keywords "languages" "go")
>                              (:maintainer "The go-mode Authors")
>                              (:authors
>                               ("The go-mode Authors"))
>                              (:commit . "fa2693278637f56759480d2bf203bb8aad107230"))
>                             :signed nil))

Hmm... that definitely looks wrong.  The 2022 version should come first
and the 2017 second.  Either there's a bug in
`package-process-define-package` or `package-alist` is being
(incorrectly) populated by some other function.


        Stefan

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

[Eli Zaretskii]

> Cc: 63757@debbugs.gnu.org
> Date: Mon, 29 May 2023 22:44:37 -0400
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> > Run `emacs -Q` and `M-x package-initialize`.
> >
> > After that `package-alist` contains
> >
> >    (go-mode #s(package-desc :name go-mode :version
> >                             (20170726 555)
> >                             :summary "Major mode for the Go programming language"
> >                             :reqs nil :kind nil :archive nil
> >                             :dir "/home/eab/.emacs.d/elpa/go-mode-20170726.555"
> >                             :extras
> >                             ((:keywords "languages" "go")
> >                              (:url . "https://github.com/dominikh/go-mode.el")
> >                              (:commit . "99b06da201afb91e8db0c525c0f3c3590fd92fa6"))
> >                             :signed nil)
> >             #s(package-desc :name go-mode :version
> >                             (20220114 2239)
> >                             :summary "Major mode for the Go programming language"
> >                             :reqs ((emacs (26 1)))
> >                             :kind nil :archive nil
> >                             :dir "/home/eab/.emacs.d/elpa/go-mode-20220114.2239"
> >                             :extras
> >                             ((:url . "https://github.com/dominikh/go-mode.el")
> >                              (:keywords "languages" "go")
> >                              (:maintainer "The go-mode Authors")
> >                              (:authors
> >                               ("The go-mode Authors"))
> >                              (:commit . "fa2693278637f56759480d2bf203bb8aad107230"))
> >                             :signed nil))
> 
> Hmm... that definitely looks wrong.  The 2022 version should come first
> and the 2017 second.  Either there's a bug in
> `package-process-define-package` or `package-alist` is being
> (incorrectly) populated by some other function.

Any suggestions for how to fix this?  I'd like this to be fixed before
the next pretest, please.