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

[Stefan Monnier]

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