Re: Proposal for 'package-isolate' command

[Thierry Volpiatto <thievol@posteo.net>]

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


Thierry Volpiatto <thievol@posteo.net> writes:

> Hello Philip,
>
> Philip Kaludercic <philipk@posteo.net> writes:
>
>> How about this patch, that will use a temporary directory when
>> `package-isolate' is invoked with a prefix argument (not sure what the
>> default should be, I guess reusing `user-emacs-directory' is less
>> surprising):
>>
>> [2. text/x-diff; 0001-Add-command-to-start-Emacs-with-specific-packages.patch]...
>
> I was reading the code of the new version of `package--dependencies` and
> had some questions:
>
> --8<---------------cut here---------------start------------->8---
>     (named-let more ((pkg-desc desc))
>       (let (deps)
>         (dolist (req (package-desc-reqs pkg-desc))
>           (setq deps (nconc
>                       (catch 'found
>                         (dolist (p (apply #'append (mapcar #'cdr (package--alist))))
>                           (when (and (string= (car req) (package-desc-name p))
>                                      (version-list-<= (cadr req) (package-desc-version p)))
>                             (throw 'found (more p)))))
>                       deps)))
>         (delete-dups (cons pkg-desc deps))))
> --8<---------------cut here---------------end--------------->8---
>
> Why are you using `string=` to compare (car req) 
> with (package-desc-name p)?
>
> Isn't (apply #'append (mapcar #'cdr (package--alist))) same as
> (mapcar #'cadr (package--alist))?
>
> I am not a fan of named-let but isn't using deps as a second arg of
> 'more' more in the named-let spirit? (fully not tested)
>
> --8<---------------cut here---------------start------------->8---
>     (named-let more ((pkg-desc desc)
>                      (deps nil))
>       (dolist (req (package-desc-reqs pkg-desc))
>         (setq deps (nconc
>                     (catch 'found
>                       (dolist (p (apply #'append (mapcar #'cdr (package--alist))))
>                         (when (and (string= (car req) (package-desc-name p))
>                                    (version-list-<= (cadr req) (package-desc-version p)))
>                           (throw 'found (more p deps)))))
>                     deps)))
>       (delete-dups (cons pkg-desc deps)))
> --8<---------------cut here---------------end--------------->8---

After some tests this create circular lists, so forget it.

Also now from Emacs 29 (magit is NOT installed):

(package-initialize)
(package--dependencies 'magit)
=> (emacs compat dash git-commit magit-section seq transient with-editor)

Now if I eval your new package--dependencies from Emacs 30:
(package-initialize)
(package--dependencies 'magit)
=> (compat)

What is wrong is you are looping in package-alist instead of
package-archive-contents, in fact you don't need to loop at all, just
using assq or package-get-descriptor:

--8<---------------cut here---------------start------------->8---
    (defun package--dependencies (pkg)
      "Return a list of all transitive dependencies of PKG.
    If PKG is a package descriptor, the return value is a list of
    package descriptors.  If PKG is a symbol designating a package,
    the return value is a list of symbols designating packages."
      (when-let* ((desc (if (package-desc-p pkg) pkg
                          (cadr (assq pkg package-archive-contents)))))
        ;; Can we have circular dependencies?  Assume "nope".
        (let ((all (cl-labels ((more (pkg-desc)
                                 (let (deps)
                                   (dolist (req (package-desc-reqs pkg-desc))
                                     (setq deps (nconc
                                                 (when-let* ((matched (package-get-descriptor (car req)))
                                                             (version<= (version-list-<=
                                                                         (cadr req)
                                                                         (package-desc-version matched))))
                                                   (more matched))
                                                 deps)))
                                   (delete-dups (cons pkg-desc deps)))))
                     (more desc))))
          (remq pkg (mapcar (if (package-desc-p pkg) #'identity #'package-desc-name) all)))))
--8<---------------cut here---------------end--------------->8---

PS: Sorry I used cl-labels to test as I am not easy with named-let.


-- 
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 686 bytes --]