Re: emacs-31 effafd92e05 1/2: Load all dependencies when isolating a package

Sean Whitton <[email protected]> writes:

> Philip Kaludercic [09/Aug  4:23am -04] wrote:
>> branch: emacs-31
>> commit effafd92e05a595b32d2fba41aadd7229f0947ce
>> Author: Philip Kaludercic <[email protected]>
>> Commit: Philip Kaludercic <[email protected]>
>>
>>     Load all dependencies when isolating a package
>>
>>     * lisp/emacs-lisp/package.el (package-isolate): Replace call to
>>     'package-compute-transaction', which omits already inferred
>>     dependencies, with a manual dependency resolution.
>> ---
>>  lisp/emacs-lisp/package.el | 18 ++++++++++++++++--
>>  1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
>> index 7e52e6decea..74e46392f84 100644
>> --- a/lisp/emacs-lisp/package.el
>> +++ b/lisp/emacs-lisp/package.el
>> @@ -2652,8 +2652,22 @@ intended for testing Emacs and/or the packages in a clean environment."
>>               current-prefix-arg)))
>>    (let* ((name (concat "package-isolate-"
>>                         (mapconcat #'package-desc-full-name packages ",")))
>> -         (dependencies (apply #'append (mapcar #'package-desc-reqs packages)))
>> -         (all-packages (package-compute-transaction packages dependencies))
>> +         (all-packages
>> +          (delete-dups
>> +           (nconc
>> +            (mapcan
>> +             (pcase-lambda (`(,name ,vers))
>> +               (and (not (eq name 'emacs))
>> +                    (not (cl-find name packages :key #'package-desc-name))
>> +                    (if-let* ((desc (package-get-descriptor
>> +                                     name t
>> +                                     (lambda (desc)
>> +                                       (version-list-<= vers (package-desc-version desc))))))
>> +                        (list desc)
>> +                      (error "Failed to find package: (%s %S)" name vers))))
>> +             (package--dependencies packages))
>> +            packages)))
>> +         (all-packages (seq-remove #'package-built-in-p all-packages))
>>           (package-alist (copy-tree package-alist t))
>>           (temp-install-dir nil) initial-scratch-message load-list)
>>      (when-let* ((missing (seq-remove #'package-installed-p all-packages))
>
> Although I appreciate you are fixing a bug here, this is a large code
> change to a core part of emacs-31, so it would have been good to have a
> chance for someone else to look it over before installing

I should have made it a while back since it is just a consequence of a
sloppy change I made a few months before we forked off emacs-31.  That
being said, I still invite any comments or bug reports.

(Also, just a clarification: What do you mean by "core part of
emacs-31"?  Since we split of package-activate and package,
package-isolate will not be loaded unless the user loads package.el so
in my mental model, this is not a "core feature", and more of a
standalone feature that we could have added to package-x before it was
obsoleted.)

> (I did the same for VC recently..)