unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* package.el dependencies
@ 2015-01-23 13:37 Thierry Volpiatto
  2015-01-23 13:46 ` Dmitry Gutov
  2015-01-23 20:40 ` Stefan Monnier
  0 siblings, 2 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-23 13:37 UTC (permalink / raw)
  To: emacs-devel

Hi,
is there something planned to handle dependencies in package.el?

1) Prevent (or warn) deleting a package if it is already used by another
package as dependency.

2) Store a list of packages installed explicitely (not as dependency)
and provide an autoremove function such as apt-get autoremove.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 




^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-23 13:37 package.el dependencies Thierry Volpiatto
@ 2015-01-23 13:46 ` Dmitry Gutov
  2015-01-23 14:12   ` Ivan Shmakov
  2015-01-23 20:40 ` Stefan Monnier
  1 sibling, 1 reply; 79+ messages in thread
From: Dmitry Gutov @ 2015-01-23 13:46 UTC (permalink / raw)
  To: Thierry Volpiatto, emacs-devel

On 01/23/2015 03:37 PM, Thierry Volpiatto wrote:

> is there something planned to handle dependencies in package.el?

Only in the sense of "would be cool if someone worked on that", AFAIK.

> 1) Prevent (or warn) deleting a package if it is already used by another
> package as dependency.
>
> 2) Store a list of packages installed explicitely (not as dependency)
> and provide an autoremove function such as apt-get autoremove.

#2 was discussed a bit not too long ago.



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-23 13:46 ` Dmitry Gutov
@ 2015-01-23 14:12   ` Ivan Shmakov
  0 siblings, 0 replies; 79+ messages in thread
From: Ivan Shmakov @ 2015-01-23 14:12 UTC (permalink / raw)
  To: emacs-devel

>>>>> Dmitry Gutov <dgutov@yandex.ru> writes:
>>>>> On 01/23/2015 03:37 PM, Thierry Volpiatto wrote:

[…]

 >> 1) Prevent (or warn) deleting a package if it is already used by
 >> another package as dependency.

 >> 2) Store a list of packages installed explicitely (not as
 >> dependency) and provide an autoremove function such as apt-get
 >> autoremove.

 > #2 was discussed a bit not too long ago.

	Could someone please file the respective bug report (or two)?
	FWIW, I see nothing relevant at [1].

	TIA.

[1] http://debbugs.gnu.org/cgi/pkgreport.cgi?package=emacs;include=subject%3Apackage

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-23 13:37 package.el dependencies Thierry Volpiatto
  2015-01-23 13:46 ` Dmitry Gutov
@ 2015-01-23 20:40 ` Stefan Monnier
  2015-01-23 21:02   ` Thierry Volpiatto
                     ` (4 more replies)
  1 sibling, 5 replies; 79+ messages in thread
From: Stefan Monnier @ 2015-01-23 20:40 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: emacs-devel

> 1) Prevent (or warn) deleting a package if it is already used by another
> package as dependency.

That'd be nice.

> 2) Store a list of packages installed explicitely (not as dependency)
> and provide an autoremove function such as apt-get autoremove.

I already asked for help writing that (not much luck so far, tho), so
yes, that'd be very welcome,


        Stefan



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-23 20:40 ` Stefan Monnier
@ 2015-01-23 21:02   ` Thierry Volpiatto
  2015-01-24  0:50     ` Artur Malabarba
  2015-01-25  9:18   ` Thierry Volpiatto
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-23 21:02 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel


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

>> 1) Prevent (or warn) deleting a package if it is already used by another
>> package as dependency.
>
> That'd be nice.

Ok, just need to run a predicate before deinstalling.

>> 2) Store a list of packages installed explicitely (not as dependency)
>> and provide an autoremove function such as apt-get autoremove.
>
> I already asked for help writing that (not much luck so far, tho), so
> yes, that'd be very welcome,

If I find the time I could do that in next weeks.
The problem is we need to feed an alist of packages explicitely
installed with their dependencies, so of course all packages installed
prior this change will have to be reinstalled.

The alist would looks like this:

(setq packages-installed-explicitely
      '((foo . (a b c d h))
        (bar . (b d e))
        (baz . (e f g))))
where a b c... are dependencies.

when deinstalling e.g foo, the starting list of dependencies to add to
the package autoremove list is (a b c d h), then looping in
packages-installed-explicitely and removing the packages used as
dependencies (and directly installed of course) would end in a final
list like (a c h).

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-23 21:02   ` Thierry Volpiatto
@ 2015-01-24  0:50     ` Artur Malabarba
  2015-01-24  4:55       ` Stefan Monnier
  0 siblings, 1 reply; 79+ messages in thread
From: Artur Malabarba @ 2015-01-24  0:50 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

> The problem is we need to feed an alist of packages explicitely
> installed with their dependencies, so of course all packages installed
> prior this change will have to be reinstalled.
>
> The alist would looks like this:
>
> (setq packages-installed-explicitely
>       '((foo . (a b c d h))
>         (bar . (b d e))
>         (baz . (e f g))))
> where a b c... are dependencies.
>

I don't think we need an alist for that. Simply marking which packages
were installed explicitly in a plain list should be enough. The
dependency information is already available in `package-alist`,
there's no need to replicate it in this other variable. (In fact,
maybe the "instaleed explicitly" flag could be stored in
`package-alist' too, instead of its own list.)

Thus, when `foo' is removed, we loop through it's depedencies and
remove each package which is neither `installed-explicitly' nor the
dependency of something else.



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-24  0:50     ` Artur Malabarba
@ 2015-01-24  4:55       ` Stefan Monnier
  2015-01-25  6:51         ` Thierry Volpiatto
  2015-01-26  7:17         ` Thierry Volpiatto
  0 siblings, 2 replies; 79+ messages in thread
From: Stefan Monnier @ 2015-01-24  4:55 UTC (permalink / raw)
  To: Artur Malabarba; +Cc: emacs-devel, Thierry Volpiatto

> I don't think we need an alist for that. Simply marking which packages
> were installed explicitly in a plain list should be enough.

Agreed.

> (In fact, maybe the "instaleed explicitly" flag could be stored in
> `package-alist' too, instead of its own list.)

You can integrate the data into package-alist, but it needs to be stored
elsewhere since it needs to be stored into a file (my preference is to
store it into a Custom var and rely on Custom to save that into a file).


        Stefan



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-24  4:55       ` Stefan Monnier
@ 2015-01-25  6:51         ` Thierry Volpiatto
  2015-01-26  7:17         ` Thierry Volpiatto
  1 sibling, 0 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-25  6:51 UTC (permalink / raw)
  To: emacs-devel

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> I don't think we need an alist for that. Simply marking which packages
>> were installed explicitly in a plain list should be enough.
>
> Agreed.

Agreed too.

>> (In fact, maybe the "instaleed explicitly" flag could be stored in
>> `package-alist' too, instead of its own list.)
>
> You can integrate the data into package-alist, but it needs to be stored
> elsewhere since it needs to be stored into a file (my preference is to
> store it into a Custom var and rely on Custom to save that into a file).

I think too it is the best option.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 




^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-23 20:40 ` Stefan Monnier
  2015-01-23 21:02   ` Thierry Volpiatto
@ 2015-01-25  9:18   ` Thierry Volpiatto
  2015-01-25 14:54     ` Stefan Monnier
  2015-01-26 15:22   ` Thierry Volpiatto
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-25  9:18 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel


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

>> 1) Prevent (or warn) deleting a package if it is already used by another
>> package as dependency.
>
> That'd be nice.

Fixed, assuming that when upgrading a package, deletion of old version
is made AFTER installing new version.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-25  9:18   ` Thierry Volpiatto
@ 2015-01-25 14:54     ` Stefan Monnier
  2015-01-25 15:48       ` Thierry Volpiatto
  0 siblings, 1 reply; 79+ messages in thread
From: Stefan Monnier @ 2015-01-25 14:54 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: emacs-devel

>>> 1) Prevent (or warn) deleting a package if it is already used by another
>>> package as dependency.
>> That'd be nice.
> Fixed,

Thanks Thierry.

> assuming that when upgrading a package, deletion of old version
> is made AFTER installing new version.

Good,


        Stefan



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-25 14:54     ` Stefan Monnier
@ 2015-01-25 15:48       ` Thierry Volpiatto
  2015-01-25 17:10         ` Dmitry Gutov
                           ` (2 more replies)
  0 siblings, 3 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-25 15:48 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel


Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>>> 1) Prevent (or warn) deleting a package if it is already used by another
>>>> package as dependency.
>>> That'd be nice.
>> Fixed,
>
> Thanks Thierry.
>
>> assuming that when upgrading a package, deletion of old version
>> is made AFTER installing new version.
>
> Good,

Here the patch:

--8<---------------cut here---------------start------------->8---
20c8e5d05f8e8e8996cf80f03f35335b328fdd94 HEAD package_autoremove
Author: Thierry Volpiatto <thierry.volpiatto@gmail.com>
Date:   Sun Jan 25 10:13:28 2015 +0100

    Prevent deleting a package needed as dependency by another package.
    
    * lisp/emacs-lisp/package.el (package-used-elsewhere-p): New.
    (package-delete): Use it.

1 file changed, 30 insertions(+), 18 deletions(-)
 lisp/emacs-lisp/package.el | 48 +++++++++++++++++++++++++++++-----------------

	Modified   lisp/emacs-lisp/package.el
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 88fc950..6923de5 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1388,26 +1388,38 @@ The file can either be a tar file or an Emacs Lisp file."
       (when (string-match "\\.tar\\'" file) (tar-mode)))
     (package-install-from-buffer)))
 
+(defun package-used-elsewhere-p (pkg)
+  "Check if PKG is used elsewhere as dependency.
+Argument PKG is a symbol."
+  (cl-loop with alist = (remove (assoc pkg package-alist) package-alist)
+           for p in alist thereis
+           (member pkg (mapcar 'car (package-desc-reqs (cadr p))))))
+
 (defun package-delete (pkg-desc)
   (let ((dir (package-desc-dir pkg-desc)))
-    (if (not (string-prefix-p (file-name-as-directory
-                               (expand-file-name package-user-dir))
-                              (expand-file-name dir)))
-        ;; Don't delete "system" packages.
-	(error "Package `%s' is a system package, not deleting"
-               (package-desc-full-name pkg-desc))
-      (delete-directory dir t t)
-      ;; Remove NAME-VERSION.signed file.
-      (let ((signed-file (concat dir ".signed")))
-	(if (file-exists-p signed-file)
-	    (delete-file signed-file)))
-      ;; Update package-alist.
-      (let* ((name (package-desc-name pkg-desc))
-             (pkgs (assq name package-alist)))
-        (delete pkg-desc pkgs)
-        (unless (cdr pkgs)
-          (setq package-alist (delq pkgs package-alist))))
-      (message "Package `%s' deleted." (package-desc-full-name pkg-desc)))))
+    (cond ((not (string-prefix-p (file-name-as-directory
+                                  (expand-file-name package-user-dir))
+                                 (expand-file-name dir)))
+           ;; Don't delete "system" packages.
+           (error "Package `%s' is a system package, not deleting"
+                  (package-desc-full-name pkg-desc)))
+          ((package-used-elsewhere-p (elt pkg-desc 1))
+           ;; Don't delete packages used as dependency elsewhere.
+           (error "Package `%s' is used elsewhere as dependency, not deleting"
+                  (package-desc-full-name pkg-desc)))
+          (t 
+           (delete-directory dir t t)
+           ;; Remove NAME-VERSION.signed file.
+           (let ((signed-file (concat dir ".signed")))
+             (if (file-exists-p signed-file)
+                 (delete-file signed-file)))
+           ;; Update package-alist.
+           (let* ((name (package-desc-name pkg-desc))
+                  (pkgs (assq name package-alist)))
+             (delete pkg-desc pkgs)
+             (unless (cdr pkgs)
+               (setq package-alist (delq pkgs package-alist))))
+           (message "Package `%s' deleted." (package-desc-full-name pkg-desc))))))
 
 (defun package-archive-base (desc)
   "Return the archive containing the package NAME."
--8<---------------cut here---------------end--------------->8---

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply related	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-25 15:48       ` Thierry Volpiatto
@ 2015-01-25 17:10         ` Dmitry Gutov
  2015-01-25 18:32           ` Stephen Leake
  2015-01-25 18:21         ` Artur Malabarba
  2015-01-26 12:53         ` Artur Malabarba
  2 siblings, 1 reply; 79+ messages in thread
From: Dmitry Gutov @ 2015-01-25 17:10 UTC (permalink / raw)
  To: Thierry Volpiatto, Stefan Monnier; +Cc: emacs-devel

On 01/25/2015 05:48 PM, Thierry Volpiatto wrote:

> +           ;; Don't delete packages used as dependency elsewhere.
> +           (error "Package `%s' is used elsewhere as dependency, not deleting"
> +                  (package-desc-full-name pkg-desc)))

Please include the exact dependant in the message. Otherwise, deleting 
packages will sometime become a lot more frustrating.



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-25 15:48       ` Thierry Volpiatto
  2015-01-25 17:10         ` Dmitry Gutov
@ 2015-01-25 18:21         ` Artur Malabarba
  2015-01-26  4:48           ` Thierry Volpiatto
  2015-01-26 12:53         ` Artur Malabarba
  2 siblings, 1 reply; 79+ messages in thread
From: Artur Malabarba @ 2015-01-25 18:21 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

> +(defun package-used-elsewhere-p (pkg)
> +  "Check if PKG is used elsewhere as dependency.
> +Argument PKG is a symbol."
> +  (cl-loop with alist = (remove (assoc pkg package-alist) package-alist)
> +           for p in alist thereis
> +           (member pkg (mapcar 'car (package-desc-reqs (cadr p))))))

`package-used-elsewhere-p' might as well return a list of the
conflicts (and say that in the docstring). I'm not entirely familiar
with the `thereis' in `cl-loop' so maybe that's already being done
here?
In any case, that would certainly be useful below.

>  (defun package-delete (pkg-desc)

Just a suggestion.
I think it would be useful to add an optional SOFT or NOERROR argument
now, which would silently not-delete packages instead of erroring. It
will probably be useful for automatic dependency removal.

> +          ((package-used-elsewhere-p (elt pkg-desc 1))

Why not use the package-desc getters?

> +           ;; Don't delete packages used as dependency elsewhere.
> +           (error "Package `%s' is used elsewhere as dependency, not deleting"
> +                  (package-desc-full-name pkg-desc)))

As already said, please say where the conflicts occurred. If
`package-used-elsewhere-p' returned a list, you could use it here to
report the conflicts.



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-25 17:10         ` Dmitry Gutov
@ 2015-01-25 18:32           ` Stephen Leake
  0 siblings, 0 replies; 79+ messages in thread
From: Stephen Leake @ 2015-01-25 18:32 UTC (permalink / raw)
  To: emacs-devel

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 01/25/2015 05:48 PM, Thierry Volpiatto wrote:
>
>> +           ;; Don't delete packages used as dependency elsewhere.
>> +           (error "Package `%s' is used elsewhere as dependency, not deleting"
>> +                  (package-desc-full-name pkg-desc)))
>
> Please include the exact dependant in the message. Otherwise, deleting
> packages will sometime become a lot more frustrating.

+1

-- 
-- Stephe



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-25 18:21         ` Artur Malabarba
@ 2015-01-26  4:48           ` Thierry Volpiatto
  2015-01-26 12:35             ` Artur Malabarba
  0 siblings, 1 reply; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-26  4:48 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


Artur Malabarba <bruce.connor.am@gmail.com> writes:

>> +(defun package-used-elsewhere-p (pkg)
>> +  "Check if PKG is used elsewhere as dependency.
>> +Argument PKG is a symbol."
>> +  (cl-loop with alist = (remove (assoc pkg package-alist) package-alist)
>> +           for p in alist thereis
>> +           (member pkg (mapcar 'car (package-desc-reqs (cadr p))))))
>
> `package-used-elsewhere-p' might as well return a list of the
> conflicts (and say that in the docstring). I'm not entirely familiar
> with the `thereis' in `cl-loop' so maybe that's already being done
> here?
> In any case, that would certainly be useful below.

That would be maybe useful for the next step of this work, but for now I
did like this to not loop further as soon a conflict is detected, IOW
stop at first conflict.

>>  (defun package-delete (pkg-desc)
>
> Just a suggestion.
> I think it would be useful to add an optional SOFT or NOERROR argument
> now, which would silently not-delete packages instead of erroring. It
> will probably be useful for automatic dependency removal.

Yes agree, I was not sure what to do here, so I just throw an error as a
starting point, will see later this, probably we will need also a
reinstall command, which run delete with noerror.

> Why not use the package-desc getters?

Because package-desc-reqs returns only the direct dependencies of a
package, we have to check all dependencies, for example the package
"jedi" have 3 direct dependencies and 4 indirect dependencies (or may be 4
and 3).

>> +           ;; Don't delete packages used as dependency elsewhere.
>> +           (error "Package `%s' is used elsewhere as dependency, not deleting"
>> +                  (package-desc-full-name pkg-desc)))
>
> As already said, please say where the conflicts occurred. If
> `package-used-elsewhere-p' returned a list, you could use it here to
> report the conflicts.

Unless I really need it for something else, no, I will report only the
first conflict encountered.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-24  4:55       ` Stefan Monnier
  2015-01-25  6:51         ` Thierry Volpiatto
@ 2015-01-26  7:17         ` Thierry Volpiatto
  2015-01-26  9:19           ` Artur Malabarba
  2015-01-26 14:52           ` Stefan Monnier
  1 sibling, 2 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-26  7:17 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Artur Malabarba, emacs-devel


Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> I don't think we need an alist for that. Simply marking which packages
>> were installed explicitly in a plain list should be enough.
>
> Agreed.
>
>> (In fact, maybe the "instaleed explicitly" flag could be stored in
>> `package-alist' too, instead of its own list.)
>
> You can integrate the data into package-alist, but it needs to be stored
> elsewhere since it needs to be stored into a file (my preference is to
> store it into a Custom var and rely on Custom to save that into a file).

I don't think it will be enough, we will have to store also a plain list
of the package to autoremove because the info about the dependencies in
package-alist will not be anymore available when deinstalling a package
unless autoremove is launched right now before quitting emacs, which is
not what we want.

1) install a package, it is added to the package-installed-directly
list.
2) store this list.
3) delete the package it is removed from package-alist, and we should
also remove it from package-installed-directly list.
4) quit emacs
5) restart emacs, even if we kept the package in
packages-installed-directly (why) we will not have the dependencies in
package-alist, so we will have to fetch these infos again, recompute the
autoremove list etc... IMO it is simpler to delete the package from the
packages-installed-directly list as soon it is removed and store also the
autoremove list.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-26  7:17         ` Thierry Volpiatto
@ 2015-01-26  9:19           ` Artur Malabarba
  2015-01-26  9:54             ` Thierry Volpiatto
  2015-01-26 14:52           ` Stefan Monnier
  1 sibling, 1 reply; 79+ messages in thread
From: Artur Malabarba @ 2015-01-26  9:19 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

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

> I don't think it will be enough, we will have to store also a plain list
> of the package to autoremove because the info about the dependencies in
> package-alist will not be anymore available when deinstalling a package
> unless autoremove is launched right now before quitting emacs, which is
> not what we want.

I think the orphan dependencies should be autoremoved immediately after the
package is deleted, so we don't need to save a list of packages to be
autoremoved. Here are two ways I can see it happen.

1. When the user marks packages for deletion and then hits x, the package
transaction is performed as usual. Afterwards, we run a function which
loops through all packages and detect the ones that are not needed by
anything nor installed explicitly (the orphans) and offer to delete them
too.

2. Alternatively, this could be done as part of the same delete transaction
which was manually requested by the user. But this would be a lot more
complicated.

> 1) install a package, it is added to the package-installed-directly
> list.
> 2) store this list.
> 3) delete the package it is removed from package-alist, and we should
> also remove it from package-installed-directly list.
> 4) quit emacs
> 5) restart emacs, even if we kept the package in
> packages-installed-directly (why) we will not have the dependencies in

I agree. Why would we have kept it in this list? If a package is deleted
(without a newer version installed) it is removed from the list.

> package-alist, so we will have to fetch these infos again, recompute the
> autoremove list etc... IMO it is simpler to delete the package from the
> packages-installed-directly list as soon it is removed and store also the
> autoremove list.

Why does this imply the need for an autoremove list? Dependencies should be
autoremoved as soon as they're no longer necessary, not later. So we
generally won't need this list.
If we do need it, it's safer to just calculate it than to try and keep it
up to date between emacs sessions. I can provide a function for this if
you'd like.

I hope I don't sound like I'm trying to make this difficult. I'm just
trying to understand why we would need this extra autoremove list.

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

^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-26  9:19           ` Artur Malabarba
@ 2015-01-26  9:54             ` Thierry Volpiatto
  2015-01-26 12:46               ` Artur Malabarba
  0 siblings, 1 reply; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-26  9:54 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


Artur Malabarba <bruce.connor.am@gmail.com> writes:

>> I don't think it will be enough, we will have to store also a plain list
>> of the package to autoremove because the info about the dependencies in
>> package-alist will not be anymore available when deinstalling a package
>> unless autoremove is launched right now before quitting emacs, which is
>> not what we want.
>
> I think the orphan dependencies should be autoremoved immediately
> after the package is deleted,

No, not necessarily, it should not be an obligation, one can delete a
package and make a cleanup later, it is what apt-get does.  

> so we don't need to save a list of packages to be autoremoved. Here
> are two ways I can see it happen.
>
> 1. When the user marks packages for deletion and then hits x, the
> package transaction is performed as usual. Afterwards, we run a
> function which loops through all packages and detect the ones that are
> not needed by anything nor installed explicitly (the orphans) and
> offer to delete them too.

No, see above.

> I agree. Why would we have kept it in this list? If a package is
> deleted (without a newer version installed) it is removed from the
> list.

You assume the dependencies unneeded are removed straight after
deleting, which is not the case.

>> package-alist, so we will have to fetch these infos again, recompute the
>> autoremove list etc... IMO it is simpler to delete the package from the
>> packages-installed-directly list as soon it is removed and store also the
>> autoremove list.
>
> Why does this imply the need for an autoremove list?

Same see above.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-26  4:48           ` Thierry Volpiatto
@ 2015-01-26 12:35             ` Artur Malabarba
  0 siblings, 0 replies; 79+ messages in thread
From: Artur Malabarba @ 2015-01-26 12:35 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

>> Why not use the package-desc getters?
>
> Because package-desc-reqs returns only the direct dependencies of a
> package, we have to check all dependencies, for example the package
> "jedi" have 3 direct dependencies and 4 indirect dependencies (or may be 4
> and 3).

I think we had a failure in communication here. What I meant was: "Why
not use `(package-desc-name pkg)' instead of `(elt pkg 1)'?



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-26  9:54             ` Thierry Volpiatto
@ 2015-01-26 12:46               ` Artur Malabarba
  0 siblings, 0 replies; 79+ messages in thread
From: Artur Malabarba @ 2015-01-26 12:46 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

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

> > I think the orphan dependencies should be autoremoved immediately
> > after the package is deleted,
>
> No, not necessarily, it should not be an obligation, one can delete a
> package and make a cleanup later, it is what apt-get does.

Though, I'd prefer doing the cleanup immediately (it's what I've always
done with arch linux's pacman), this can be discussed some other time.
Getting this feature out is more important, so I'm ok with whatever you
chose here.

Still, that doesn't say why you'd need an `autoremove-list'. That's
something that's easy to calculate (I can write the implementation if you'd
like).


> > I agree. Why would we have kept it in this list? If a package is
> > deleted (without a newer version installed) it is removed from the
> > list.
>
> You assume the dependencies unneeded are removed straight after
> deleting, which is not the case.

Not really. The package-installed-explicitly is a list of installed
packages which the user manually requested, why would it ever contain
deleted packages?

We don't need this list to calculate unneeded dependencies. All we need is
the list of currently installed packages (package-alist) and the dependency
tree of currently installed packages (available inside the entries of
package-alist).


Maybe I just didn't quite understand what's the implementation you have in
mind, and maybe we're just talking different things. But, from what I
understand, we don't need to (and probably shouldn't) keep a list of
unneeded packages. Just calculating it with a function would be less code,
and would be safer (you never know what might have happened between an
Emacs restart).

Once your first patch is applied, I can provide an example implementation
for this if you'd like (just to show I'm trying to help, and not get in the
way :-)).

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

^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-25 15:48       ` Thierry Volpiatto
  2015-01-25 17:10         ` Dmitry Gutov
  2015-01-25 18:21         ` Artur Malabarba
@ 2015-01-26 12:53         ` Artur Malabarba
  2 siblings, 0 replies; 79+ messages in thread
From: Artur Malabarba @ 2015-01-26 12:53 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

2015-01-25 13:48 GMT-02:00 Thierry Volpiatto <thierry.volpiatto@gmail.com>:
>
> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>
>>>>> 1) Prevent (or warn) deleting a package if it is already used by another
>>>>> package as dependency.
>>>> That'd be nice.
>>> Fixed,
>>
>> Thanks Thierry.
>>
>>> assuming that when upgrading a package, deletion of old version
>>> is made AFTER installing new version.
>>
>> Good,
>
> Here the patch:
>
> --8<---------------cut here---------------start------------->8---
> 20c8e5d05f8e8e8996cf80f03f35335b328fdd94 HEAD package_autoremove
> Author: Thierry Volpiatto <thierry.volpiatto@gmail.com>
> Date:   Sun Jan 25 10:13:28 2015 +0100
>
>     Prevent deleting a package needed as dependency by another package.
>
>     * lisp/emacs-lisp/package.el (package-used-elsewhere-p): New.
>     (package-delete): Use it.
>
> 1 file changed, 30 insertions(+), 18 deletions(-)
> +(defun package-used-elsewhere-p (pkg)
> +  "Check if PKG is used elsewhere as dependency.
> +Argument PKG is a symbol."
> +  (cl-loop with alist = (remove (assoc pkg package-alist) package-alist)
> +           for p in alist thereis
> +           (member pkg (mapcar 'car (package-desc-reqs (cadr p))))))
> [...]
> +
> +          ((package-used-elsewhere-p (elt pkg-desc 1))
> +           ;; Don't delete packages used as dependency elsewhere.
> +           (error "Package `%s' is used elsewhere as dependency, not deleting"
> +                  (package-desc-full-name pkg-desc)))

I was reading this again now and I have a question. How does this work
if `package-used-elsequere-p' only takes the function name? For
instance, let's say I upgrade `dash', which is a dependency to some
other packages. Here's what I think would happen:
1. Emacs installs the new version.
2. Emacs tries to delete the old version.
3. `package-delete' calls `(package-used-elsewhere-p 'dash)', which
returns non-nil.
4. `package-delete' throws an error, because `dash' is used as
dependency, even though Emacs was just trying to delete the old
version.



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-26  7:17         ` Thierry Volpiatto
  2015-01-26  9:19           ` Artur Malabarba
@ 2015-01-26 14:52           ` Stefan Monnier
  2015-01-27  6:10             ` Thierry Volpiatto
  1 sibling, 1 reply; 79+ messages in thread
From: Stefan Monnier @ 2015-01-26 14:52 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Artur Malabarba, emacs-devel

>> You can integrate the data into package-alist, but it needs to be stored
>> elsewhere since it needs to be stored into a file (my preference is to
>> store it into a Custom var and rely on Custom to save that into a file).
> I don't think it will be enough, we will have to store also a plain list
> of the package to autoremove because the info about the dependencies in

No, that's not needed.  We can do a GC-like traversal to compute which
members of package-alist aren't (transitively) required by the manually
installed packages (and should hence be auto-removed).


        Stefan



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-23 20:40 ` Stefan Monnier
  2015-01-23 21:02   ` Thierry Volpiatto
  2015-01-25  9:18   ` Thierry Volpiatto
@ 2015-01-26 15:22   ` Thierry Volpiatto
  2015-01-26 15:44     ` Stefan Monnier
  2015-01-26 16:34     ` Artur Malabarba
  2015-01-28  7:30   ` Thierry Volpiatto
  2015-01-30  5:38   ` Thierry Volpiatto
  4 siblings, 2 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-26 15:22 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel


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

>> 1) Prevent (or warn) deleting a package if it is already used by another
>> package as dependency.
>
> That'd be nice.

Fixed with improved message showing the first conflict encountered which
help deletion.

>> 2) Store a list of packages installed explicitely (not as dependency)
>> and provide an autoremove function such as apt-get autoremove.
>
> I already asked for help writing that (not much luck so far, tho), so
> yes, that'd be very welcome,

Done also, I only fail now to store variables with
customize-set-variable, don't know why yet.

I will give you more details and patch later, oops! I have no more time.

BTW I use a local branch for this, what's the best way actually to show
this branch for easier communication ?

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-26 15:22   ` Thierry Volpiatto
@ 2015-01-26 15:44     ` Stefan Monnier
  2015-01-27  6:08       ` Thierry Volpiatto
  2015-01-26 16:34     ` Artur Malabarba
  1 sibling, 1 reply; 79+ messages in thread
From: Stefan Monnier @ 2015-01-26 15:44 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: emacs-devel

> BTW I use a local branch for this, what's the best way actually to show
> this branch for easier communication ?

You can push them to the main repository under the name
"scratch/<yourchoicehere>".


        Stefan



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-26 15:22   ` Thierry Volpiatto
  2015-01-26 15:44     ` Stefan Monnier
@ 2015-01-26 16:34     ` Artur Malabarba
  1 sibling, 0 replies; 79+ messages in thread
From: Artur Malabarba @ 2015-01-26 16:34 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

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

> BTW I use a local branch for this, what's the best way actually to show
> this branch for easier communication ?

You can push it to the remote, to a branch whose name starts with
"scratch/". That indicates a branch is up for discussion, and its commits
and commit-messages aren't final.

Before actually merging the branch, remember to clean it up, in case some
of the commits don't follow the style.

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

^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-26 15:44     ` Stefan Monnier
@ 2015-01-27  6:08       ` Thierry Volpiatto
  0 siblings, 0 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-27  6:08 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel


Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> BTW I use a local branch for this, what's the best way actually to show
>> this branch for easier communication ?
>
> You can push them to the main repository under the name
> "scratch/<yourchoicehere>".

Ok thanks.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-26 14:52           ` Stefan Monnier
@ 2015-01-27  6:10             ` Thierry Volpiatto
  2015-01-27 11:52               ` Artur Malabarba
  0 siblings, 1 reply; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-27  6:10 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Artur Malabarba, emacs-devel


Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>> You can integrate the data into package-alist, but it needs to be stored
>>> elsewhere since it needs to be stored into a file (my preference is to
>>> store it into a Custom var and rely on Custom to save that into a file).
>> I don't think it will be enough, we will have to store also a plain list
>> of the package to autoremove because the info about the dependencies in
>
> No, that's not needed.  We can do a GC-like traversal to compute which
> members of package-alist aren't (transitively) required by the manually
> installed packages (and should hence be auto-removed).

Ok done, still not able to customize-set-variable though, for some
reason it doesn't write to my custom file.

Thanks.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-27  6:10             ` Thierry Volpiatto
@ 2015-01-27 11:52               ` Artur Malabarba
  0 siblings, 0 replies; 79+ messages in thread
From: Artur Malabarba @ 2015-01-27 11:52 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

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

> Ok done, still not able to customize-set-variable though, for some
> reason it doesn't write to my custom file.
>

It might be *customize-save-variable* that you want.

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

^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-23 20:40 ` Stefan Monnier
                     ` (2 preceding siblings ...)
  2015-01-26 15:22   ` Thierry Volpiatto
@ 2015-01-28  7:30   ` Thierry Volpiatto
  2015-01-28  8:55     ` Thierry Volpiatto
  2015-01-28 10:47     ` Artur Malabarba
  2015-01-30  5:38   ` Thierry Volpiatto
  4 siblings, 2 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-28  7:30 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel


I will not have a good connection until next week, so I am unable to
push a branch, so I am just attaching patch of all my changes against
package.el here.

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 88fc950..df3a108 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -333,6 +333,15 @@ contents of the archive."
   :group 'package
   :version "24.4")
 
+(defcustom packages-installed-directly nil
+  "Store here packages installed explicitely by user.
+This variable will be feeded automaticaly by emacs,
+so you should not modify it yourself.
+This variable will be used by `package-autoremove' to decide
+which packages are no more needed."
+  :group 'package
+  :type '(repeat (choice symbol)))
+
 (defvar package--default-summary "No description available.")
 
 (cl-defstruct (package-desc
@@ -1187,7 +1196,7 @@ using `package-compute-transaction'."
   (mapc #'package-install-from-archive packages))
 
 ;;;###autoload
-(defun package-install (pkg)
+(defun package-install (pkg &optional arg)
   "Install the package PKG.
 PKG can be a package-desc or the package name of one the available packages
 in an archive in `package-archives'.  Interactively, prompt for its name."
@@ -1206,7 +1215,11 @@ in an archive in `package-archives'.  Interactively, prompt for its name."
                                     (unless (package-installed-p (car elt))
                                       (symbol-name (car elt))))
                                   package-archive-contents))
-                    nil t)))))
+                    nil t))
+           "\p")))
+  (when (and arg (not (memq pkg packages-installed-directly)))
+    (customize-save-variable 'packages-installed-directly
+                            (cons pkg packages-installed-directly)))
   (package-download-transaction
    (if (package-desc-p pkg)
        (package-compute-transaction (list pkg)
@@ -1388,26 +1401,105 @@ The file can either be a tar file or an Emacs Lisp file."
       (when (string-match "\\.tar\\'" file) (tar-mode)))
     (package-install-from-buffer)))
 
-(defun package-delete (pkg-desc)
-  (let ((dir (package-desc-dir pkg-desc)))
-    (if (not (string-prefix-p (file-name-as-directory
-                               (expand-file-name package-user-dir))
-                              (expand-file-name dir)))
-        ;; Don't delete "system" packages.
-	(error "Package `%s' is a system package, not deleting"
-               (package-desc-full-name pkg-desc))
-      (delete-directory dir t t)
-      ;; Remove NAME-VERSION.signed file.
-      (let ((signed-file (concat dir ".signed")))
-	(if (file-exists-p signed-file)
-	    (delete-file signed-file)))
-      ;; Update package-alist.
-      (let* ((name (package-desc-name pkg-desc))
-             (pkgs (assq name package-alist)))
-        (delete pkg-desc pkgs)
-        (unless (cdr pkgs)
-          (setq package-alist (delq pkgs package-alist))))
-      (message "Package `%s' deleted." (package-desc-full-name pkg-desc)))))
+(defun package--get-deps (pkg &optional only)
+  (let* ((pkg-desc (cadr (assq pkg package-alist)))
+         (direct-deps (cl-loop for p in (package-desc-reqs pkg-desc)
+                               for name = (car p)
+                               when (assq name package-alist)
+                               collect name))
+         (indirect-deps (unless (eq only 'direct)
+                          (cl-loop for p in direct-deps
+                                   for dep = (cadr (assq p package-alist))
+                                   when (and dep (assq p package-alist))
+                                   append (mapcar 'car
+                                                  (package-desc-reqs
+                                                   dep))))))
+    (cl-case only
+      (direct   direct-deps)
+      (separate (list direct-deps indirect-deps))
+      (indirect indirect-deps)
+      (t        (append direct-deps indirect-deps)))))
+
+(defun package-used-elsewhere-p (pkg &optional pkg-list)
+  "Check in PKG-LIST if PKG is used elsewhere as dependency.
+When not specified, PKG-LIST default to `package-alist' with PKG entry removed.
+Argument PKG is a symbol.
+Returns the first package found in PKG-LIST where PKG is used as dependency."
+  (cl-loop with alist = (or pkg-list
+                            (remove (assq pkg package-alist)
+                                    package-alist))
+           for p in alist thereis
+           (and (memq pkg (mapcar 'car (package-desc-reqs (cadr p))))
+                (car p))))
+
+(defun package-delete (pkg-desc &optional force)
+  "Delete package PKG-DESC.
+
+Argument PKG-DESC is a full description of package as vector.
+When package is used elsewhere as dependency of another package,
+refuse deleting it and return an error.
+If FORCE is non--nil package will be deleted even if it is used
+elsewhere."
+  (let ((dir (package-desc-dir pkg-desc))
+        (name (package-desc-name pkg-desc))
+        pkg-used-elsewhere-by)
+    (cond ((not (string-prefix-p (file-name-as-directory
+                                  (expand-file-name package-user-dir))
+                                 (expand-file-name dir)))
+           ;; Don't delete "system" packages.
+           (error "Package `%s' is a system package, not deleting"
+                  (package-desc-full-name pkg-desc)))
+          ((and (null force)
+                (setq pkg-used-elsewhere-by
+                      (package-used-elsewhere-p name)))
+           ;; Don't delete packages used as dependency elsewhere.
+           (error "Package `%s' is used by `%s' as dependency, not deleting"
+                  (package-desc-full-name pkg-desc)
+                  pkg-used-elsewhere-by))
+          (t 
+           (delete-directory dir t t)
+           ;; Remove NAME-VERSION.signed file.
+           (let ((signed-file (concat dir ".signed")))
+             (if (file-exists-p signed-file)
+                 (delete-file signed-file)))
+           ;; Update package-alist.
+           (let ((pkgs (assq name package-alist)))
+             (delete pkg-desc pkgs)
+             (unless (cdr pkgs)
+               (setq package-alist (delq pkgs package-alist))))
+           (message "Package `%s' deleted." (package-desc-full-name pkg-desc))))))
+
+;;;###autoload
+(defun package-autoremove ()
+  "Remove packages that are no more needed.
+
+Packages that are no more needed by other packages in
+`packages-installed-directly' and their dependencies
+will be deleted."
+  (interactive)
+  (let* (old-direct
+         (needed (cl-loop for p in packages-installed-directly
+                          if (assq p package-alist)
+                          append (package--get-deps p) into lst
+                          else do (push p old-direct)
+                          finally return lst)))
+    (cl-loop for p in (mapcar 'car package-alist)
+             unless (or (memq p needed)
+                        (memq p packages-installed-directly))
+             collect p into lst
+             finally (if lst
+                         (when (y-or-n-p (format "%s packages will be deleted:\n%s, proceed? "
+                                                 (length lst)
+                                                 (mapconcat 'symbol-name lst ", ")))
+                           (mapc (lambda (p)
+                                   (package-delete (cadr (assq p package-alist)) t))
+                                 lst)
+                           (customize-save-variable
+                            'packages-installed-directly
+                            (cl-loop for p in packages-installed-directly
+                                     unless (memq p old-direct)
+                                     collect p)))
+                       (message "Nothing to autoremove")))))
 
 (defun package-archive-base (desc)
   "Return the archive containing the package NAME."
@@ -2178,7 +2270,7 @@ Optional argument NOQUERY non-nil means do not ask the user to confirm."
                       (length install-list)
                       (mapconcat #'package-desc-full-name
                                  install-list ", ")))))
-	  (mapc 'package-install install-list)))
+	  (mapc (lambda (p) (package-install p 1)) install-list)))
     ;; Delete packages, prompting if necessary.
     (when delete-list
       (if (or


-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply related	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-28  7:30   ` Thierry Volpiatto
@ 2015-01-28  8:55     ` Thierry Volpiatto
  2015-01-28 12:42       ` Thierry Volpiatto
  2015-01-28 10:47     ` Artur Malabarba
  1 sibling, 1 reply; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-28  8:55 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel


Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:

> I will not have a good connection until next week, so I am unable to
> push a branch, so I am just attaching patch of all my changes against
> package.el here.

So to summarize what this patch does:

1) Returns an error when trying to delete a package already used as
dependency by another package.  The first package already using the
package we are trying to delete is returned in error message.

2) When installing a package explicitely (interactively) record this
package in a variable named `packages-installed-directly'.
(NB: In this patch `package-install-from-buffer' is not doing this,
I think it should, I did this on my local branch, 
it cover also `package-install-file' which reuse
`package-install-from-buffer')

3) Provide an autoremove command that remove all unneeded packages, i.e
the packages that are not needed as dependency (directly or indirectly)
by one of `packages-installed-directly'.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-28  7:30   ` Thierry Volpiatto
  2015-01-28  8:55     ` Thierry Volpiatto
@ 2015-01-28 10:47     ` Artur Malabarba
  2015-01-28 11:58       ` Thierry Volpiatto
  2015-01-28 19:33       ` Stefan Monnier
  1 sibling, 2 replies; 79+ messages in thread
From: Artur Malabarba @ 2015-01-28 10:47 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

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

Hi,
I'll read the rest later, but I just wanted to say one thing before leaving
the house.

On 28 Jan 2015 05:30, "Thierry Volpiatto" <thierry.volpiatto@gmail.com>
wrote:
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index 88fc950..df3a108 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -333,6 +333,15 @@ contents of the archive."
>    :group 'package
>    :version "24.4")
>
> +(defcustom packages-installed-directly nil
> +  "Store here packages installed explicitely by user.
> +This variable will be feeded automaticaly by emacs,
> +so you should not modify it yourself.

Then don't make it a defcustom :-). `customize-save-variable' will work on
any variable (even unbound ones), so a defvar is enough here.

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

^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-28 10:47     ` Artur Malabarba
@ 2015-01-28 11:58       ` Thierry Volpiatto
  2015-01-28 19:33       ` Stefan Monnier
  1 sibling, 0 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-28 11:58 UTC (permalink / raw)
  To: Artur Malabarba; +Cc: Stefan Monnier, emacs-devel


Artur Malabarba <arturmalabarba@gmail.com> writes:

> Hi,
> I'll read the rest later, but I just wanted to say one thing before leaving the house.
>
> On 28 Jan 2015 05:30, "Thierry Volpiatto" <thierry.volpiatto@gmail.com> wrote:
>> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
>> index 88fc950..df3a108 100644
>> --- a/lisp/emacs-lisp/package.el
>> +++ b/lisp/emacs-lisp/package.el
>> @@ -333,6 +333,15 @@ contents of the archive."
>>  :group 'package
>>  :version "24.4")
>>
>> +(defcustom packages-installed-directly nil
>> + "Store here packages installed explicitely by user.
>> +This variable will be feeded automaticaly by emacs,
>> +so you should not modify it yourself.
>
> Then don't make it a defcustom :-). `customize-save-variable' will
> work on any variable (even unbound ones), so a defvar is enough here.

In fact the variable stored will not be loaded at initialization if it
is not a defcustom, so I leave it like this for now as it is working
fine like this.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-28  8:55     ` Thierry Volpiatto
@ 2015-01-28 12:42       ` Thierry Volpiatto
  2015-01-28 13:17         ` Artur Malabarba
  2015-01-28 13:40         ` Dmitry Gutov
  0 siblings, 2 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-28 12:42 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel


Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:

> Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:
>
>> I will not have a good connection until next week, so I am unable to
>> push a branch, so I am just attaching patch of all my changes against
>> package.el here.
>
> So to summarize what this patch does:
>
> 1) Returns an error when trying to delete a package already used as
> dependency by another package.  The first package already using the
> package we are trying to delete is returned in error message.
>
> 2) When installing a package explicitely (interactively) record this
> package in a variable named `packages-installed-directly'.
>
> 3) Provide an autoremove command that remove all unneeded packages, i.e
> the packages that are not needed as dependency (directly or indirectly)
> by one of `packages-installed-directly'.

So here the last version, with tiny changes since the last, tested and
working fine here, please review, possibly install it so that I can make
easily corrections if needed, thanks.

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 88fc950..c80ea4d 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -333,6 +333,15 @@ contents of the archive."
   :group 'package
   :version "24.4")
 
+(defcustom packages-installed-directly nil
+  "Store here packages installed explicitely by user.
+This variable will be feeded automaticaly by emacs,
+so you should not modify it yourself.
+This variable will be used by `package-autoremove' to decide
+which packages are no more needed."
+  :group 'package
+  :type '(repeat (choice symbol)))
+
 (defvar package--default-summary "No description available.")
 
 (cl-defstruct (package-desc
@@ -1187,10 +1196,13 @@ using `package-compute-transaction'."
   (mapc #'package-install-from-archive packages))
 
 ;;;###autoload
-(defun package-install (pkg)
+(defun package-install (pkg &optional arg)
   "Install the package PKG.
 PKG can be a package-desc or the package name of one the available packages
-in an archive in `package-archives'.  Interactively, prompt for its name."
+in an archive in `package-archives'.  Interactively, prompt for its name
+and add PKG to `packages-installed-directly'.
+When called from lisp you will have to use ARG if you want to
+simulate an interactive call to add PKG to `packages-installed-directly'."
   (interactive
    (progn
      ;; Initialize the package system to get the list of package
@@ -1206,7 +1218,11 @@ in an archive in `package-archives'.  Interactively, prompt for its name."
                                     (unless (package-installed-p (car elt))
                                       (symbol-name (car elt))))
                                   package-archive-contents))
-                    nil t)))))
+                    nil t))
+           "\p")))
+  (when (and arg (not (memq pkg packages-installed-directly)))
+    (customize-save-variable 'packages-installed-directly
+                            (cons pkg packages-installed-directly)))
   (package-download-transaction
    (if (package-desc-p pkg)
        (package-compute-transaction (list pkg)
@@ -1368,10 +1384,15 @@ Downloads and installs required packages as needed."
            (package-buffer-info)))))
     ;; Download and install the dependencies.
     (let* ((requires (package-desc-reqs pkg-desc))
+           (name (package-desc-name pkg-desc))
            (transaction (package-compute-transaction nil requires)))
       (package-download-transaction transaction))
     ;; Install the package itself.
     (package-unpack pkg-desc)
+    (unless (memq name packages-installed-directly)
+      (push name packages-installed-directly)
+      (customize-save-variable 'packages-installed-directly
+                               packages-installed-directly))
     pkg-desc))
 
 ;;;###autoload
@@ -1388,26 +1409,105 @@ The file can either be a tar file or an Emacs Lisp file."
       (when (string-match "\\.tar\\'" file) (tar-mode)))
     (package-install-from-buffer)))
 
-(defun package-delete (pkg-desc)
-  (let ((dir (package-desc-dir pkg-desc)))
-    (if (not (string-prefix-p (file-name-as-directory
-                               (expand-file-name package-user-dir))
-                              (expand-file-name dir)))
-        ;; Don't delete "system" packages.
-	(error "Package `%s' is a system package, not deleting"
-               (package-desc-full-name pkg-desc))
-      (delete-directory dir t t)
-      ;; Remove NAME-VERSION.signed file.
-      (let ((signed-file (concat dir ".signed")))
-	(if (file-exists-p signed-file)
-	    (delete-file signed-file)))
-      ;; Update package-alist.
-      (let* ((name (package-desc-name pkg-desc))
-             (pkgs (assq name package-alist)))
-        (delete pkg-desc pkgs)
-        (unless (cdr pkgs)
-          (setq package-alist (delq pkgs package-alist))))
-      (message "Package `%s' deleted." (package-desc-full-name pkg-desc)))))
+(defun package--get-deps (pkg &optional only)
+  (let* ((pkg-desc (cadr (assq pkg package-alist)))
+         (direct-deps (cl-loop for p in (package-desc-reqs pkg-desc)
+                               for name = (car p)
+                               when (assq name package-alist)
+                               collect name))
+         (indirect-deps (unless (eq only 'direct)
+                          (cl-loop for p in direct-deps
+                                   for dep = (cadr (assq p package-alist))
+                                   when (and dep (assq p package-alist))
+                                   append (mapcar 'car
+                                                  (package-desc-reqs
+                                                   dep))))))
+    (cl-case only
+      (direct   direct-deps)
+      (separate (list direct-deps indirect-deps))
+      (indirect indirect-deps)
+      (t        (append direct-deps indirect-deps)))))
+
+(defun package-used-elsewhere-p (pkg &optional pkg-list)
+  "Check in PKG-LIST if PKG is used elsewhere as dependency.
+When not specified, PKG-LIST default to `package-alist' with PKG entry removed.
+Argument PKG is a symbol.
+Returns the first package found in PKG-LIST where PKG is used as dependency."
+  (cl-loop with alist = (or pkg-list
+                            (remove (assq pkg package-alist)
+                                    package-alist))
+           for p in alist thereis
+           (and (memq pkg (mapcar 'car (package-desc-reqs (cadr p))))
+                (car p))))
+
+(defun package-delete (pkg-desc &optional force)
+  "Delete package PKG-DESC.
+
+Argument PKG-DESC is a full description of package as vector.
+When package is used elsewhere as dependency of another package,
+refuse deleting it and return an error.
+If FORCE is non--nil package will be deleted even if it is used
+elsewhere."
+  (let ((dir (package-desc-dir pkg-desc))
+        (name (package-desc-name pkg-desc))
+        pkg-used-elsewhere-by)
+    (cond ((not (string-prefix-p (file-name-as-directory
+                                  (expand-file-name package-user-dir))
+                                 (expand-file-name dir)))
+           ;; Don't delete "system" packages.
+           (error "Package `%s' is a system package, not deleting"
+                  (package-desc-full-name pkg-desc)))
+          ((and (null force)
+                (setq pkg-used-elsewhere-by
+                      (package-used-elsewhere-p name)))
+           ;; Don't delete packages used as dependency elsewhere.
+           (error "Package `%s' is used by `%s' as dependency, not deleting"
+                  (package-desc-full-name pkg-desc)
+                  pkg-used-elsewhere-by))
+          (t 
+           (delete-directory dir t t)
+           ;; Remove NAME-VERSION.signed file.
+           (let ((signed-file (concat dir ".signed")))
+             (if (file-exists-p signed-file)
+                 (delete-file signed-file)))
+           ;; Update package-alist.
+           (let ((pkgs (assq name package-alist)))
+             (delete pkg-desc pkgs)
+             (unless (cdr pkgs)
+               (setq package-alist (delq pkgs package-alist))))
+           (message "Package `%s' deleted." (package-desc-full-name pkg-desc))))))
+
+;;;###autoload
+(defun package-autoremove ()
+  "Remove packages that are no more needed.
+
+Packages that are no more needed by other packages in
+`packages-installed-directly' and their dependencies
+will be deleted."
+  (interactive)
+  (let* (old-direct
+         (needed (cl-loop for p in packages-installed-directly
+                          if (assq p package-alist)
+                          append (package--get-deps p) into lst
+                          else do (push p old-direct)
+                          finally return lst)))
+    (cl-loop for p in (mapcar 'car package-alist)
+             unless (or (memq p needed)
+                        (memq p packages-installed-directly))
+             collect p into lst
+             finally (if lst
+                         (when (y-or-n-p (format "%s packages will be deleted:\n%s, proceed? "
+                                                 (length lst)
+                                                 (mapconcat 'symbol-name lst ", ")))
+                           (mapc (lambda (p)
+                                   (package-delete (cadr (assq p package-alist)) t))
+                                 lst)
+                           (customize-save-variable
+                            'packages-installed-directly
+                            (cl-loop for p in packages-installed-directly
+                                     unless (memq p old-direct)
+                                     collect p)))
+                       (message "Nothing to autoremove")))))
 
 (defun package-archive-base (desc)
   "Return the archive containing the package NAME."
@@ -1721,7 +1821,7 @@ If optional arg NO-ACTIVATE is non-nil, don't activate packages."
   (let ((pkg-desc (button-get button 'package-desc)))
     (when (y-or-n-p (format "Install package `%s'? "
                             (package-desc-full-name pkg-desc)))
-      (package-install pkg-desc)
+      (package-install pkg-desc 1)
       (revert-buffer nil t)
       (goto-char (point-min)))))
 
@@ -2178,7 +2278,7 @@ Optional argument NOQUERY non-nil means do not ask the user to confirm."
                       (length install-list)
                       (mapconcat #'package-desc-full-name
                                  install-list ", ")))))
-	  (mapc 'package-install install-list)))
+	  (mapc (lambda (p) (package-install p 1)) install-list)))
     ;; Delete packages, prompting if necessary.
     (when delete-list
       (if (or

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply related	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-28 12:42       ` Thierry Volpiatto
@ 2015-01-28 13:17         ` Artur Malabarba
  2015-01-28 14:32           ` Thierry Volpiatto
  2015-01-28 13:40         ` Dmitry Gutov
  1 sibling, 1 reply; 79+ messages in thread
From: Artur Malabarba @ 2015-01-28 13:17 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

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

> @@ -2178,7 +2278,7 @@ Optional argument NOQUERY non-nil means do not ask
the user to confirm."
>                        (length install-list)
>                        (mapconcat #'package-desc-full-name
>                                   install-list ", ")))))
> -         (mapc 'package-install install-list)))
> +         (mapc (lambda (p) (package-install p 1)) install-list)))

IIUC, this will mark dependencies as installed directly when the user
performs an upgrade (`U x' in the package list).

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

^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-28 12:42       ` Thierry Volpiatto
  2015-01-28 13:17         ` Artur Malabarba
@ 2015-01-28 13:40         ` Dmitry Gutov
  1 sibling, 0 replies; 79+ messages in thread
From: Dmitry Gutov @ 2015-01-28 13:40 UTC (permalink / raw)
  To: Thierry Volpiatto, Stefan Monnier; +Cc: emacs-devel

On 01/28/2015 02:42 PM, Thierry Volpiatto wrote:

> So here the last version, with tiny changes since the last, tested and
> working fine here, please review, possibly install it so that I can make
> easily corrections if needed, thanks.

This patch will need ChangeLog and NEWS entries.

Could you add a couple of tests, too?



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-28 13:17         ` Artur Malabarba
@ 2015-01-28 14:32           ` Thierry Volpiatto
  0 siblings, 0 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-28 14:32 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


Artur Malabarba <bruce.connor.am@gmail.com> writes:

>> @@ -2178,7 +2278,7 @@ Optional argument NOQUERY non-nil means do not ask the user to confirm."
>>            (length install-list)
>>            (mapconcat #'package-desc-full-name
>>                  install-list ", ")))))
>> -    (mapc 'package-install install-list)))
>> +    (mapc (lambda (p) (package-install p 1)) install-list)))
>
> IIUC, this will mark dependencies as installed directly when the user performs an upgrade (`U x' in the package list).

Indeed, thanks, this should be enough (fully not tested):

-	  (mapc (lambda (p) (package-install p 1)) install-list)))
+	  (mapc (lambda (p)
+                  (package-install p (and (null (package-installed-p p)) 1)))
+                install-list)))

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-28 10:47     ` Artur Malabarba
  2015-01-28 11:58       ` Thierry Volpiatto
@ 2015-01-28 19:33       ` Stefan Monnier
  2015-01-28 19:50         ` Ivan Shmakov
  2015-01-29  5:31         ` Thierry Volpiatto
  1 sibling, 2 replies; 79+ messages in thread
From: Stefan Monnier @ 2015-01-28 19:33 UTC (permalink / raw)
  To: Artur Malabarba; +Cc: emacs-devel, Thierry Volpiatto

>> +(defcustom packages-installed-directly nil
>> +  "Store here packages installed explicitely by user.
>> +This variable will be feeded automaticaly by emacs,
>> +so you should not modify it yourself.
> Then don't make it a defcustom :-).

I agree with Arthur's reaction to the docstring's message.  But I think
in reality the problem is in the docstring: there's nothing wrong with
setting this variable via Custom (or via setq).
We should very much support such usage.

E.g. we should provide a command which makes sure that all packages in
packages-installed-directly are indeed installed.  So a user can copy
her init file to a new computer and then use this command to
auto-install all the packages she likes.

Of course, this also hints at the fact that the name is not right.
The name should be more along the lines of "user selected packages", or
something like that.


        Stefan



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-28 19:33       ` Stefan Monnier
@ 2015-01-28 19:50         ` Ivan Shmakov
  2015-01-28 20:12           ` Artur Malabarba
  2015-01-28 22:20           ` Stefan Monnier
  2015-01-29  5:31         ` Thierry Volpiatto
  1 sibling, 2 replies; 79+ messages in thread
From: Ivan Shmakov @ 2015-01-28 19:50 UTC (permalink / raw)
  To: emacs-devel

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

[…]

 > E. g. we should provide a command which makes sure that all packages
 > in packages-installed-directly are indeed installed.  So a user can
 > copy her init file to a new computer and then use this command to
 > auto-install all the packages she likes.

 > Of course, this also hints at the fact that the name is not right.
 > The name should be more along the lines of "user selected packages",
 > or something like that.

	How about the following?

(defcustom package-selected-packages nil
  "List of packages to keep installed.
Packages may also be installed or removed as part of dependency
resolution, or via `package-autoremove'.  Emacs will try to ensure that
the packages listed here, along with all their dependencies, are kept
installed."
  :group 'package
  :type '(repeat (choice symbol)))

-- 
FSF associate member #7257  np. The Call of Ktulu — Metallica  B6A0 230E 334A



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-28 19:50         ` Ivan Shmakov
@ 2015-01-28 20:12           ` Artur Malabarba
  2015-01-28 22:20           ` Stefan Monnier
  1 sibling, 0 replies; 79+ messages in thread
From: Artur Malabarba @ 2015-01-28 20:12 UTC (permalink / raw)
  To: emacs-devel

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

> (defcustom package-selected-packages nil
>   "List of packages to keep installed.
> Packages may also be installed or removed as part of dependency
> resolution, or via `package-autoremove'.  Emacs will try to ensure that
> the packages listed here, along with all their dependencies, are kept
> installed."
>   :group 'package
>   :type '(repeat (choice symbol)))

I like this. Just 2 things:
1. Also say packages are auto added to this list when explicitly installed
(or something like this).
2. Isn't (choice symbol) the same as symbol?

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

^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-28 19:50         ` Ivan Shmakov
  2015-01-28 20:12           ` Artur Malabarba
@ 2015-01-28 22:20           ` Stefan Monnier
  1 sibling, 0 replies; 79+ messages in thread
From: Stefan Monnier @ 2015-01-28 22:20 UTC (permalink / raw)
  To: emacs-devel

> 	How about the following?

> (defcustom package-selected-packages nil

Good,


        Stefan



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-28 19:33       ` Stefan Monnier
  2015-01-28 19:50         ` Ivan Shmakov
@ 2015-01-29  5:31         ` Thierry Volpiatto
  2015-01-29  7:22           ` Thierry Volpiatto
  1 sibling, 1 reply; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-29  5:31 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Artur Malabarba, emacs-devel


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

>>> +(defcustom packages-installed-directly nil
>>> +  "Store here packages installed explicitely by user.
>>> +This variable will be feeded automaticaly by emacs,
>>> +so you should not modify it yourself.
>> Then don't make it a defcustom :-).
>
> I agree with Arthur's reaction to the docstring's message.  But I think
> in reality the problem is in the docstring: there's nothing wrong with
> setting this variable via Custom (or via setq).
> We should very much support such usage.
>
> E.g. we should provide a command which makes sure that all packages in
> packages-installed-directly are indeed installed.  So a user can copy
> her init file to a new computer and then use this command to
> auto-install all the packages she likes.
>
> Of course, this also hints at the fact that the name is not right.
> The name should be more along the lines of "user selected packages", or
> something like that.

Ok done, I haven't done the rename of `packages-installed-directly'
though, I leave you the choice of the name.

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ba9fcd2..92bcbf1 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -335,10 +335,12 @@ contents of the archive."
 
 (defcustom packages-installed-directly nil
   "Store here packages installed explicitely by user.
-This variable will be feeded automaticaly by emacs,
-so you should not modify it yourself.
+This variable will be feeded automatically by emacs,
+when installing a new package.
 This variable will be used by `package-autoremove' to decide
-which packages are no more needed."
+which packages are no more needed.
+You can use it to (re)install packages on other machines
+by running `package-user-selected-packages-install'."
   :group 'package
   :type '(repeat (choice symbol)))
 
@@ -1428,6 +1430,27 @@ The file can either be a tar file or an Emacs Lisp file."
       (indirect indirect-deps)
       (t        (append direct-deps indirect-deps)))))
 
+(defun package-user-selected-packages-installed-p ()
+  (cl-loop for p in packages-installed-directly
+           always (package-installed-p p)))
+
+;;;###autoload
+(defun package-user-selected-packages-install ()
+  "Ensure packages in `packages-installed-directly' are installed.
+If some packages are not installed propose to install them."
+  (interactive)
+  (cl-loop for p in packages-installed-directly
+           unless (package-installed-p p)
+           collect p into lst
+           finally
+           (if lst
+               (when (y-or-n-p
+                      (format "%s packages will be installed:\n%s, proceed?"
+                              (length lst)
+                              (mapconcat 'symbol-name lst ", ")))
+                 (mapc 'package-install lst))
+               (message "All your packages are already installed"))))
+
 (defun package-used-elsewhere-p (pkg &optional pkg-list)
   "Check in PKG-LIST if PKG is used elsewhere as dependency.
 When not specified, PKG-LIST default to `package-alist' with PKG entry removed.
@@ -1618,6 +1641,9 @@ If optional arg NO-ACTIVATE is non-nil, don't activate packages."
   (unless no-activate
     (dolist (elt package-alist)
       (package-activate (car elt))))
+  (unless (package-user-selected-packages-installed-p)
+    (when (y-or-n-p "It seems some user-installed packages are missing, update now? ")
+      (package-user-selected-packages-install)))
   (setq package--initialized t))
 
 \f

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply related	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-29  5:31         ` Thierry Volpiatto
@ 2015-01-29  7:22           ` Thierry Volpiatto
  0 siblings, 0 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-29  7:22 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Artur Malabarba, emacs-devel


Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>>> +(defcustom packages-installed-directly nil
>>>> +  "Store here packages installed explicitely by user.
>>>> +This variable will be feeded automaticaly by emacs,
>>>> +so you should not modify it yourself.
>>> Then don't make it a defcustom :-).
>>
>> I agree with Arthur's reaction to the docstring's message.  But I think
>> in reality the problem is in the docstring: there's nothing wrong with
>> setting this variable via Custom (or via setq).
>> We should very much support such usage.
>>
>> E.g. we should provide a command which makes sure that all packages in
>> packages-installed-directly are indeed installed.  So a user can copy
>> her init file to a new computer and then use this command to
>> auto-install all the packages she likes.
>>
>> Of course, this also hints at the fact that the name is not right.
>> The name should be more along the lines of "user selected packages", or
>> something like that.
>
> Ok done, I haven't done the rename of `packages-installed-directly'
> though, I leave you the choice of the name.

I have also added a reinstall command which is handy when one want to
reinstall a package which is not directly installed.

> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index ba9fcd2..92bcbf1 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -335,10 +335,12 @@ contents of the archive."
>  
>  (defcustom packages-installed-directly nil
>    "Store here packages installed explicitely by user.
> -This variable will be feeded automaticaly by emacs,
> -so you should not modify it yourself.
> +This variable will be feeded automatically by emacs,
> +when installing a new package.
>  This variable will be used by `package-autoremove' to decide
> -which packages are no more needed."
> +which packages are no more needed.
> +You can use it to (re)install packages on other machines
> +by running `package-user-selected-packages-install'."
>    :group 'package
>    :type '(repeat (choice symbol)))
>  
> @@ -1428,6 +1430,27 @@ The file can either be a tar file or an Emacs Lisp file."
>        (indirect indirect-deps)
>        (t        (append direct-deps indirect-deps)))))
>  
> +(defun package-user-selected-packages-installed-p ()
> +  (cl-loop for p in packages-installed-directly
> +           always (package-installed-p p)))
> +
> +;;;###autoload
> +(defun package-user-selected-packages-install ()
> +  "Ensure packages in `packages-installed-directly' are installed.
> +If some packages are not installed propose to install them."
> +  (interactive)
> +  (cl-loop for p in packages-installed-directly
> +           unless (package-installed-p p)
> +           collect p into lst
> +           finally
> +           (if lst
> +               (when (y-or-n-p
> +                      (format "%s packages will be installed:\n%s, proceed?"
> +                              (length lst)
> +                              (mapconcat 'symbol-name lst ", ")))
> +                 (mapc 'package-install lst))
> +               (message "All your packages are already installed"))))
> +
>  (defun package-used-elsewhere-p (pkg &optional pkg-list)
>    "Check in PKG-LIST if PKG is used elsewhere as dependency.
>  When not specified, PKG-LIST default to `package-alist' with PKG entry removed.
> @@ -1618,6 +1641,9 @@ If optional arg NO-ACTIVATE is non-nil, don't activate packages."
>    (unless no-activate
>      (dolist (elt package-alist)
>        (package-activate (car elt))))
> +  (unless (package-user-selected-packages-installed-p)
> +    (when (y-or-n-p "It seems some user-installed packages are missing, update now? ")
> +      (package-user-selected-packages-install)))
>    (setq package--initialized t))
>  
>  \f

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-23 20:40 ` Stefan Monnier
                     ` (3 preceding siblings ...)
  2015-01-28  7:30   ` Thierry Volpiatto
@ 2015-01-30  5:38   ` Thierry Volpiatto
  2015-01-30 16:43     ` Artur Malabarba
  4 siblings, 1 reply; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-30  5:38 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel


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

>> 1) Prevent (or warn) deleting a package if it is already used by another
>> package as dependency.
>
> That'd be nice.
>
>> 2) Store a list of packages installed explicitely (not as dependency)
>> and provide an autoremove function such as apt-get autoremove.
>
> I already asked for help writing that (not much luck so far, tho), so
> yes, that'd be very welcome,

So find here my final patch, I haven't renamed
packages-installed-directly, feel free to rename it as needed.


Changes from master to working tree
2 files changed, 193 insertions(+), 37 deletions(-)
 lisp/ChangeLog             |  25 ++++++
 lisp/emacs-lisp/package.el | 205 +++++++++++++++++++++++++++++++++++++--------

	Modified   lisp/ChangeLog
diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index eb6ef6b..06753eb 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,28 @@
+2015-01-30  Thierry Volpiatto  <thierry.volpiatto@gmail.com>
+
+	* lisp/emacs-lisp/package.el:
+
+	Package should not allow deleting packages used elsewhere.
+	(package-used-elsewhere-p): New.
+	(package-delete): Use it, return now an error when trying to delete
+	a package used as dependency by another package.
+
+	Add a reinstall package command.
+	(package-reinstall): New.
+
+	Add a package-autoremove command.
+	(packages-installed-directly): New user var.
+	(package-install): Add an optional arg to notify interactive use.
+	Fix docstring. Save installed package to packages-installed-directly.
+	(package-install-from-buffer): Same.
+	(package-user-selected-packages-install): Allow installing all
+	packages in packages-installed-directly at once.
+	(package--get-deps): New.
+	(package-autoremove): New
+	(package-install-button-action): Call package-install with interactive arg.
+	(package-menu-execute): Same but only for only for not installed packages.
+
+
 2015-01-26  Fabián Ezequiel Gallina  <fgallina@gnu.org>
 
 	python.el: New non-global state dependent indentation engine.
	Modified   lisp/emacs-lisp/package.el
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 88fc950..df27be6 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -333,6 +333,17 @@ contents of the archive."
   :group 'package
   :version "24.4")
 
+(defcustom packages-installed-directly nil
+  "Store here packages installed explicitely by user.
+This variable will be feeded automatically by emacs,
+when installing a new package.
+This variable will be used by `package-autoremove' to decide
+which packages are no more needed.
+You can use it to (re)install packages on other machines
+by running `package-user-selected-packages-install'."
+  :group 'package
+  :type '(repeat (choice symbol)))
+
 (defvar package--default-summary "No description available.")
 
 (cl-defstruct (package-desc
@@ -1187,10 +1198,13 @@ using `package-compute-transaction'."
   (mapc #'package-install-from-archive packages))
 
 ;;;###autoload
-(defun package-install (pkg)
+(defun package-install (pkg &optional arg)
   "Install the package PKG.
 PKG can be a package-desc or the package name of one the available packages
-in an archive in `package-archives'.  Interactively, prompt for its name."
+in an archive in `package-archives'.  Interactively, prompt for its name
+and add PKG to `packages-installed-directly'.
+When called from lisp you will have to use ARG if you want to
+simulate an interactive call to add PKG to `packages-installed-directly'."
   (interactive
    (progn
      ;; Initialize the package system to get the list of package
@@ -1206,7 +1220,11 @@ in an archive in `package-archives'.  Interactively, prompt for its name."
                                     (unless (package-installed-p (car elt))
                                       (symbol-name (car elt))))
                                   package-archive-contents))
-                    nil t)))))
+                    nil t))
+           "\p")))
+  (when (and arg (not (memq pkg packages-installed-directly)))
+    (customize-save-variable 'packages-installed-directly
+                            (cons pkg packages-installed-directly)))
   (package-download-transaction
    (if (package-desc-p pkg)
        (package-compute-transaction (list pkg)
@@ -1214,6 +1232,16 @@ in an archive in `package-archives'.  Interactively, prompt for its name."
      (package-compute-transaction ()
                                   (list (list pkg))))))
 
+;;;###autoload
+(defun package-reinstall (pkg)
+  "Reinstall package PKG."
+  (interactive (list (intern (completing-read
+                              "Reinstall package: "
+                              (mapcar 'symbol-name
+                                      (mapcar 'car package-alist))))))
+  (package-delete (cadr (assq pkg package-alist)) t)
+  (package-install pkg))
+
 (defun package-strip-rcs-id (str)
   "Strip RCS version ID from the version string STR.
 If the result looks like a dotted numeric version, return it.
@@ -1354,24 +1382,29 @@ is derived from the main .el file in the directory.
 
 Downloads and installs required packages as needed."
   (interactive)
-  (let ((pkg-desc
-         (cond
-          ((derived-mode-p 'dired-mode)
-           ;; This is the only way a package-desc object with a `dir'
-           ;; desc-kind can be created.  Such packages can't be
-           ;; uploaded or installed from archives, they can only be
-           ;; installed from local buffers or directories.
-           (package-dir-info))
-          ((derived-mode-p 'tar-mode)
-           (package-tar-file-info))
-          (t
-           (package-buffer-info)))))
+  (let* ((pkg-desc
+          (cond
+            ((derived-mode-p 'dired-mode)
+             ;; This is the only way a package-desc object with a `dir'
+             ;; desc-kind can be created.  Such packages can't be
+             ;; uploaded or installed from archives, they can only be
+             ;; installed from local buffers or directories.
+             (package-dir-info))
+            ((derived-mode-p 'tar-mode)
+             (package-tar-file-info))
+            (t
+             (package-buffer-info))))
+         (name (package-desc-name pkg-desc)))
     ;; Download and install the dependencies.
     (let* ((requires (package-desc-reqs pkg-desc))
            (transaction (package-compute-transaction nil requires)))
       (package-download-transaction transaction))
     ;; Install the package itself.
     (package-unpack pkg-desc)
+    (unless (memq name packages-installed-directly)
+      (push name packages-installed-directly)
+      (customize-save-variable 'packages-installed-directly
+                               packages-installed-directly))
     pkg-desc))
 
 ;;;###autoload
@@ -1388,26 +1421,122 @@ The file can either be a tar file or an Emacs Lisp file."
       (when (string-match "\\.tar\\'" file) (tar-mode)))
     (package-install-from-buffer)))
 
-(defun package-delete (pkg-desc)
-  (let ((dir (package-desc-dir pkg-desc)))
-    (if (not (string-prefix-p (file-name-as-directory
-                               (expand-file-name package-user-dir))
-                              (expand-file-name dir)))
-        ;; Don't delete "system" packages.
-	(error "Package `%s' is a system package, not deleting"
-               (package-desc-full-name pkg-desc))
-      (delete-directory dir t t)
-      ;; Remove NAME-VERSION.signed file.
-      (let ((signed-file (concat dir ".signed")))
-	(if (file-exists-p signed-file)
-	    (delete-file signed-file)))
-      ;; Update package-alist.
-      (let* ((name (package-desc-name pkg-desc))
-             (pkgs (assq name package-alist)))
-        (delete pkg-desc pkgs)
-        (unless (cdr pkgs)
-          (setq package-alist (delq pkgs package-alist))))
-      (message "Package `%s' deleted." (package-desc-full-name pkg-desc)))))
+(defun package--get-deps (pkg &optional only)
+  (let* ((pkg-desc (cadr (assq pkg package-alist)))
+         (direct-deps (cl-loop for p in (package-desc-reqs pkg-desc)
+                               for name = (car p)
+                               when (assq name package-alist)
+                               collect name))
+         (indirect-deps (unless (eq only 'direct)
+                          (cl-loop for p in direct-deps
+                                   for dep = (cadr (assq p package-alist))
+                                   when (and dep (assq p package-alist))
+                                   append (mapcar 'car
+                                                  (package-desc-reqs
+                                                   dep))))))
+    (cl-case only
+      (direct   direct-deps)
+      (separate (list direct-deps indirect-deps))
+      (indirect indirect-deps)
+      (t        (append direct-deps indirect-deps)))))
+
+;;;###autoload
+(defun package-user-selected-packages-install ()
+  "Ensure packages in `packages-installed-directly' are installed.
+If some packages are not installed propose to install them."
+  (interactive)
+  (cl-loop for p in packages-installed-directly
+           unless (package-installed-p p)
+           collect p into lst
+           finally
+           (if lst
+               (when (y-or-n-p
+                      (format "%s packages will be installed:\n%s, proceed?"
+                              (length lst)
+                              (mapconcat 'symbol-name lst ", ")))
+                 (mapc 'package-install lst))
+               (message "All your packages are already installed"))))
+
+(defun package-used-elsewhere-p (pkg &optional pkg-list)
+  "Check in PKG-LIST if PKG is used elsewhere as dependency.
+When not specified, PKG-LIST default to `package-alist' with PKG entry removed.
+Argument PKG is a symbol.
+Returns the first package found in PKG-LIST where PKG is used as dependency."
+  (cl-loop with alist = (or pkg-list
+                            (remove (assq pkg package-alist)
+                                    package-alist))
+           for p in alist thereis
+           (and (memq pkg (mapcar 'car (package-desc-reqs (cadr p))))
+                (car p))))
+
+(defun package-delete (pkg-desc &optional force)
+  "Delete package PKG-DESC.
+
+Argument PKG-DESC is a full description of package as vector.
+When package is used elsewhere as dependency of another package,
+refuse deleting it and return an error.
+If FORCE is non--nil package will be deleted even if it is used
+elsewhere."
+  (let ((dir (package-desc-dir pkg-desc))
+        (name (package-desc-name pkg-desc))
+        pkg-used-elsewhere-by)
+    (cond ((not (string-prefix-p (file-name-as-directory
+                                  (expand-file-name package-user-dir))
+                                 (expand-file-name dir)))
+           ;; Don't delete "system" packages.
+           (error "Package `%s' is a system package, not deleting"
+                  (package-desc-full-name pkg-desc)))
+          ((and (null force)
+                (setq pkg-used-elsewhere-by
+                      (package-used-elsewhere-p name)))
+           ;; Don't delete packages used as dependency elsewhere.
+           (error "Package `%s' is used by `%s' as dependency, not deleting"
+                  (package-desc-full-name pkg-desc)
+                  pkg-used-elsewhere-by))
+          (t 
+           (delete-directory dir t t)
+           ;; Remove NAME-VERSION.signed file.
+           (let ((signed-file (concat dir ".signed")))
+             (if (file-exists-p signed-file)
+                 (delete-file signed-file)))
+           ;; Update package-alist.
+           (let ((pkgs (assq name package-alist)))
+             (delete pkg-desc pkgs)
+             (unless (cdr pkgs)
+               (setq package-alist (delq pkgs package-alist))))
+           (message "Package `%s' deleted." (package-desc-full-name pkg-desc))))))
+
+;;;###autoload
+(defun package-autoremove ()
+  "Remove packages that are no more needed.
+
+Packages that are no more needed by other packages in
+`packages-installed-directly' and their dependencies
+will be deleted."
+  (interactive)
+  (let* (old-direct
+         (needed (cl-loop for p in packages-installed-directly
+                          if (assq p package-alist)
+                          append (package--get-deps p) into lst
+                          else do (push p old-direct)
+                          finally return lst)))
+    (cl-loop for p in (mapcar 'car package-alist)
+             unless (or (memq p needed)
+                        (memq p packages-installed-directly))
+             collect p into lst
+             finally (if lst
+                         (when (y-or-n-p (format "%s packages will be deleted:\n%s, proceed? "
+                                                 (length lst)
+                                                 (mapconcat 'symbol-name lst ", ")))
+                           (mapc (lambda (p)
+                                   (package-delete (cadr (assq p package-alist)) t))
+                                 lst)
+                           (customize-save-variable
+                            'packages-installed-directly
+                            (cl-loop for p in packages-installed-directly
+                                     unless (memq p old-direct)
+                                     collect p)))
+                       (message "Nothing to autoremove")))))
 
 (defun package-archive-base (desc)
   "Return the archive containing the package NAME."
@@ -1721,7 +1850,7 @@ If optional arg NO-ACTIVATE is non-nil, don't activate packages."
   (let ((pkg-desc (button-get button 'package-desc)))
     (when (y-or-n-p (format "Install package `%s'? "
                             (package-desc-full-name pkg-desc)))
-      (package-install pkg-desc)
+      (package-install pkg-desc 1)
       (revert-buffer nil t)
       (goto-char (point-min)))))
 
@@ -2178,7 +2307,9 @@ Optional argument NOQUERY non-nil means do not ask the user to confirm."
                       (length install-list)
                       (mapconcat #'package-desc-full-name
                                  install-list ", ")))))
-	  (mapc 'package-install install-list)))
+	  (mapc (lambda (p)
+                  (package-install p (and (null (package-installed-p p)) 1)))
+                install-list)))
     ;; Delete packages, prompting if necessary.
     (when delete-list
       (if (or


-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply related	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-30  5:38   ` Thierry Volpiatto
@ 2015-01-30 16:43     ` Artur Malabarba
  2015-01-30 17:13       ` Thierry Volpiatto
                         ` (2 more replies)
  0 siblings, 3 replies; 79+ messages in thread
From: Artur Malabarba @ 2015-01-30 16:43 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

There are several small style changes I would suggest, but I'll
refrain from mentioning them now as they can be made later.

There are, though, two important issues I see, so I'd like to raise them now.

> +(defun package-used-elsewhere-p (pkg &optional pkg-list)
> +  "Check in PKG-LIST if PKG is used elsewhere as dependency.
> +When not specified, PKG-LIST default to `package-alist' with PKG entry removed.
> +Argument PKG is a symbol.
> +Returns the first package found in PKG-LIST where PKG is used as dependency."
> +  (cl-loop with alist = (or pkg-list
> +                            (remove (assq pkg package-alist)
> +                                    package-alist))
> +           for p in alist thereis
> +           (and (memq pkg (mapcar 'car (package-desc-reqs (cadr p))))
> +                (car p))))

This will prevent deletion of obsolete depencies (packages which are
dependencies but have a more recent version installed). To fix that,
PKG in this function needs to be a package-desc object, and the
function needs to be altered as follows.

(defun package-used-elsewhere-p (pkg &optional pkg-list)
  "Check in PKG-LIST if PKG is used elsewhere as dependency.
When not specified, PKG-LIST default to `package-alist' with PKG entry removed.
Argument PKG is a package-desc.
Returns name of the first package found in PKG-LIST where PKG is
used as dependency."
  (let ((name (package-desc-name pkg))
        (version (package-desc-version pkg))
        (most-recent-version (package-desc-version (assq name package-alist))))
    (unless (version< version most-recent-version)
      (cl-loop with alist = (or pkg-list
                                (remove (assq name package-alist)
                                        package-alist))
               for p in alist thereis
               (and (memq name (mapcar 'car (package-desc-reqs (cadr p))))
                    (car p)))))


> +;;;###autoload
> +(defun package-autoremove ()
> +  "Remove packages that are no more needed.
> +
> +Packages that are no more needed by other packages in
> +`packages-installed-directly' and their dependencies
> +will be deleted."
> +  (interactive)
> +  (let* (old-direct
> +         (needed (cl-loop for p in packages-installed-directly
> +                          if (assq p package-alist)
> +                          append (package--get-deps p) into lst
> +                          else do (push p old-direct)
> +                          finally return lst)))
> +    (cl-loop for p in (mapcar 'car package-alist)
> +             unless (or (memq p needed)
> +                        (memq p packages-installed-directly))
> +             collect p into lst
> +             finally (if lst
> +                         (when (y-or-n-p (format "%s packages will be deleted:\n%s, proceed? "
> +                                                 (length lst)
> +                                                 (mapconcat 'symbol-name lst ", ")))
> +                           (mapc (lambda (p)
> +                                   (package-delete (cadr (assq p package-alist)) t))
> +                                 lst)
> +                           (customize-save-variable
> +                            'packages-installed-directly
> +                            (cl-loop for p in packages-installed-directly
> +                                     unless (memq p old-direct)
> +                                     collect p)))

IIUC, this will remove a package from packages-installed-directly if
it is not currently installed. That seems undesirable, and would be
fixed by simply removing this call to `customize-save-variable'.



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-30 16:43     ` Artur Malabarba
@ 2015-01-30 17:13       ` Thierry Volpiatto
  2015-01-31  6:01       ` Thierry Volpiatto
  2015-01-31  6:51       ` Thierry Volpiatto
  2 siblings, 0 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-30 17:13 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


Artur Malabarba <bruce.connor.am@gmail.com> writes:

> There are several small style changes I would suggest, but I'll
> refrain from mentioning them now as they can be made later.
>
> There are, though, two important issues I see, so I'd like to raise them now.

Ok, thanks to catch this, however I don't want now to create and
recreate patch indefinitely, so if you want to see this change
happening, just install this patch and I will send a new patch on top of
this (or you can fix it yourself, I don't care).
I will not have the time to fix this until next week though.

Thanks.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-30 16:43     ` Artur Malabarba
  2015-01-30 17:13       ` Thierry Volpiatto
@ 2015-01-31  6:01       ` Thierry Volpiatto
  2015-01-31 10:58         ` Artur Malabarba
  2015-01-31 20:26         ` Stefan Monnier
  2015-01-31  6:51       ` Thierry Volpiatto
  2 siblings, 2 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-31  6:01 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


Artur Malabarba <bruce.connor.am@gmail.com> writes:

> There are, though, two important issues I see, so I'd like to raise
> them now.

After reading again this mail, I am not sure I agree with you on both
points.

> This will prevent deletion of obsolete depencies (packages which are
> dependencies but have a more recent version installed). To fix that,
> PKG in this function needs to be a package-desc object, and the
> function needs to be altered as follows.

Can you explain this with a real life example ?

> IIUC, this will remove a package from packages-installed-directly if
> it is not currently installed. That seems undesirable, and would be
> fixed by simply removing this call to `customize-save-variable'.

And I disagree on this one.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-30 16:43     ` Artur Malabarba
  2015-01-30 17:13       ` Thierry Volpiatto
  2015-01-31  6:01       ` Thierry Volpiatto
@ 2015-01-31  6:51       ` Thierry Volpiatto
  2015-01-31 20:30         ` Stefan Monnier
  2 siblings, 1 reply; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-31  6:51 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


Artur Malabarba <bruce.connor.am@gmail.com> writes:

> This will prevent deletion of obsolete depencies (packages which are
> dependencies but have a more recent version installed). To fix that,
> PKG in this function needs to be a package-desc object, and the
> function needs to be altered as follows.
>
> (defun package-used-elsewhere-p (pkg &optional pkg-list)
>   "Check in PKG-LIST if PKG is used elsewhere as dependency.
> When not specified, PKG-LIST default to `package-alist' with PKG entry removed.
> Argument PKG is a package-desc.
> Returns name of the first package found in PKG-LIST where PKG is
> used as dependency."
>   (let ((name (package-desc-name pkg))
>         (version (package-desc-version pkg))
>         (most-recent-version (package-desc-version (assq name package-alist))))
>     (unless (version< version most-recent-version)
>       (cl-loop with alist = (or pkg-list
>                                 (remove (assq name package-alist)
>                                         package-alist))
>                for p in alist thereis
>                (and (memq name (mapcar 'car (package-desc-reqs (cadr p))))
>                     (car p)))))

Anyway, this is not working, and need to be fixed differently, will do
next week.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-31  6:01       ` Thierry Volpiatto
@ 2015-01-31 10:58         ` Artur Malabarba
  2015-01-31 20:26         ` Stefan Monnier
  1 sibling, 0 replies; 79+ messages in thread
From: Artur Malabarba @ 2015-01-31 10:58 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

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

> > This will prevent deletion of obsolete depencies (packages which are
> > dependencies but have a more recent version installed). To fix that,
> > PKG in this function needs to be a package-desc object, and the
> > function needs to be altered as follows.
>
> Can you explain this with a real life example ?

The user hits U on the package menu, and a dependency is marked for
upgrade. The user hits x to execute the transactions. The new version of
the package is installed, then the old version is deleted. When
package-delete is called on the old package, you get an error because this
package is used elsewhere, even though it's fine to delete it (because
there's a newer version installed).

Maybe I'm just reading it wrong, but that's what it looks like to me.
Again, package-used-elsewhere-p needs to take a package object, not a
symbol.
Note, the fix I proposed only works if you also change package-delete to
actually pass an object to the function when it is called.

> > IIUC, this will remove a package from packages-installed-directly if
> > it is not currently installed. That seems undesirable, and would be
> > fixed by simply removing this call to `customize-save-variable'.
>
> And I disagree on this one.

Do you disagree that it's undesirable, or do you disagree that it well
happen?

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

^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-31  6:01       ` Thierry Volpiatto
  2015-01-31 10:58         ` Artur Malabarba
@ 2015-01-31 20:26         ` Stefan Monnier
       [not found]           ` <874mr67gjb.fsf@gmail.com>
  1 sibling, 1 reply; 79+ messages in thread
From: Stefan Monnier @ 2015-01-31 20:26 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: bruce.connor.am, emacs-devel

>> IIUC, this will remove a package from packages-installed-directly if
>> it is not currently installed. That seems undesirable, and would be
>> fixed by simply removing this call to `customize-save-variable'.
> And I disagree on this one.

Like Arthur, I'm not I understand what you're disagreeing with, but to
me the fact that package-autoremove calls customize-save-variable is
an error.
When the user runs package-autoremove, it doesn't mean she wants to
change the set of selected packages, only the set of installed packages.


        Stefan



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-31  6:51       ` Thierry Volpiatto
@ 2015-01-31 20:30         ` Stefan Monnier
  2015-01-31 22:10           ` Thierry Volpiatto
                             ` (2 more replies)
  0 siblings, 3 replies; 79+ messages in thread
From: Stefan Monnier @ 2015-01-31 20:30 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: bruce.connor.am, emacs-devel

> Anyway, this is not working, and need to be fixed differently, will do
> next week.

While you're at it: if package-selected-packages (or whatever name you
end up using) is not yet set (e.g. still nil), you could guess an
initial value: take the list of packages that are installed and are not
required by other installed packages.  I.e. choose as initial value of
package-selected-packages the smallest set of packages such that
package-autoremove won't have anything to remove.


        Stefan



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-31 20:30         ` Stefan Monnier
@ 2015-01-31 22:10           ` Thierry Volpiatto
  2015-01-31 23:26           ` Artur Malabarba
  2015-02-02 12:00           ` Artur Malabarba
  2 siblings, 0 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-01-31 22:10 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: bruce.connor.am, emacs-devel


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

>> Anyway, this is not working, and need to be fixed differently, will do
>> next week.
>
> While you're at it: if package-selected-packages (or whatever name you
> end up using) is not yet set (e.g. still nil), you could guess an
> initial value: take the list of packages that are installed and are not
> required by other installed packages.  I.e. choose as initial value of
> package-selected-packages the smallest set of packages such that
> package-autoremove won't have anything to remove.

I will fix the issue in package-used-elsewhere-p raised by Arthur (It is
done) and when we agree on why keeping a package you have deleted in the 
packages-installed-directly list, I will send a last patch, at this
point I would like you install this patch so that I continue working on
this.

Thanks.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-31 20:30         ` Stefan Monnier
  2015-01-31 22:10           ` Thierry Volpiatto
@ 2015-01-31 23:26           ` Artur Malabarba
  2015-02-01  6:29             ` Thierry Volpiatto
                               ` (2 more replies)
  2015-02-02 12:00           ` Artur Malabarba
  2 siblings, 3 replies; 79+ messages in thread
From: Artur Malabarba @ 2015-01-31 23:26 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel, Thierry Volpiatto

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

> While you're at it: if package-selected-packages (or whatever name you
> end up using) is not yet set (e.g. still nil), you could guess an
> initial value: take the list of packages that are installed and are not
> required by other installed packages.  I.e. choose as initial value of
> package-selected-packages the smallest set of packages such that
> package-autoremove won't have anything to remove.

I'll work on this, and then I'll add it in once this patch is applied.

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

^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-31 23:26           ` Artur Malabarba
@ 2015-02-01  6:29             ` Thierry Volpiatto
  2015-02-01  7:02             ` Thierry Volpiatto
  2015-02-01 15:55             ` Thierry Volpiatto
  2 siblings, 0 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-02-01  6:29 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


Artur Malabarba <bruce.connor.am@gmail.com> writes:

>> While you're at it: if package-selected-packages (or whatever name you
>> end up using) is not yet set (e.g. still nil), you could guess an
>> initial value: take the list of packages that are installed and are not
>> required by other installed packages. I.e. choose as initial value of
>> package-selected-packages the smallest set of packages such that
>> package-autoremove won't have anything to remove.
>
> I'll work on this, and then I'll add it in once this patch is applied.

Great ;-) thanks.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-31 23:26           ` Artur Malabarba
  2015-02-01  6:29             ` Thierry Volpiatto
@ 2015-02-01  7:02             ` Thierry Volpiatto
  2015-02-01 15:55             ` Thierry Volpiatto
  2 siblings, 0 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-02-01  7:02 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


Artur Malabarba <bruce.connor.am@gmail.com> writes:

>> While you're at it: if package-selected-packages (or whatever name you
>> end up using) is not yet set (e.g. still nil), you could guess an
>> initial value: take the list of packages that are installed and are not
>> required by other installed packages. I.e. choose as initial value of
>> package-selected-packages the smallest set of packages such that
>> package-autoremove won't have anything to remove.
>
> I'll work on this, and then I'll add it in once this patch is applied.

So here again my final patch:
- packages-installed-directly renamed to package-selected-packages
- remove customize call in package-autoremove.

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 88fc950..74a441b 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -333,6 +333,17 @@ contents of the archive."
   :group 'package
   :version "24.4")
 
+(defcustom package-selected-packages nil
+  "Store here packages installed explicitely by user.
+This variable will be feeded automatically by emacs,
+when installing a new package.
+This variable will be used by `package-autoremove' to decide
+which packages are no more needed.
+You can use it to (re)install packages on other machines
+by running `package-user-selected-packages-install'."
+  :group 'package
+  :type '(repeat (choice symbol)))
+
 (defvar package--default-summary "No description available.")
 
 (cl-defstruct (package-desc
@@ -1187,10 +1198,13 @@ using `package-compute-transaction'."
   (mapc #'package-install-from-archive packages))
 
 ;;;###autoload
-(defun package-install (pkg)
+(defun package-install (pkg &optional arg)
   "Install the package PKG.
 PKG can be a package-desc or the package name of one the available packages
-in an archive in `package-archives'.  Interactively, prompt for its name."
+in an archive in `package-archives'.  Interactively, prompt for its name
+and add PKG to `package-selected-packages'.
+When called from lisp you will have to use ARG if you want to
+simulate an interactive call to add PKG to `package-selected-packages'."
   (interactive
    (progn
      ;; Initialize the package system to get the list of package
@@ -1206,7 +1220,11 @@ in an archive in `package-archives'.  Interactively, prompt for its name."
                                     (unless (package-installed-p (car elt))
                                       (symbol-name (car elt))))
                                   package-archive-contents))
-                    nil t)))))
+                    nil t))
+           "\p")))
+  (when (and arg (not (memq pkg package-selected-packages)))
+    (customize-save-variable 'package-selected-packages
+                            (cons pkg package-selected-packages)))
   (package-download-transaction
    (if (package-desc-p pkg)
        (package-compute-transaction (list pkg)
@@ -1214,6 +1232,16 @@ in an archive in `package-archives'.  Interactively, prompt for its name."
      (package-compute-transaction ()
                                   (list (list pkg))))))
 
+;;;###autoload
+(defun package-reinstall (pkg)
+  "Reinstall package PKG."
+  (interactive (list (intern (completing-read
+                              "Reinstall package: "
+                              (mapcar 'symbol-name
+                                      (mapcar 'car package-alist))))))
+  (package-delete (cadr (assq pkg package-alist)) t)
+  (package-install pkg))
+
 (defun package-strip-rcs-id (str)
   "Strip RCS version ID from the version string STR.
 If the result looks like a dotted numeric version, return it.
@@ -1354,24 +1382,29 @@ is derived from the main .el file in the directory.
 
 Downloads and installs required packages as needed."
   (interactive)
-  (let ((pkg-desc
-         (cond
-          ((derived-mode-p 'dired-mode)
-           ;; This is the only way a package-desc object with a `dir'
-           ;; desc-kind can be created.  Such packages can't be
-           ;; uploaded or installed from archives, they can only be
-           ;; installed from local buffers or directories.
-           (package-dir-info))
-          ((derived-mode-p 'tar-mode)
-           (package-tar-file-info))
-          (t
-           (package-buffer-info)))))
+  (let* ((pkg-desc
+          (cond
+            ((derived-mode-p 'dired-mode)
+             ;; This is the only way a package-desc object with a `dir'
+             ;; desc-kind can be created.  Such packages can't be
+             ;; uploaded or installed from archives, they can only be
+             ;; installed from local buffers or directories.
+             (package-dir-info))
+            ((derived-mode-p 'tar-mode)
+             (package-tar-file-info))
+            (t
+             (package-buffer-info))))
+         (name (package-desc-name pkg-desc)))
     ;; Download and install the dependencies.
     (let* ((requires (package-desc-reqs pkg-desc))
            (transaction (package-compute-transaction nil requires)))
       (package-download-transaction transaction))
     ;; Install the package itself.
     (package-unpack pkg-desc)
+    (unless (memq name package-selected-packages)
+      (push name package-selected-packages)
+      (customize-save-variable 'package-selected-packages
+                               package-selected-packages))
     pkg-desc))
 
 ;;;###autoload
@@ -1388,26 +1421,120 @@ The file can either be a tar file or an Emacs Lisp file."
       (when (string-match "\\.tar\\'" file) (tar-mode)))
     (package-install-from-buffer)))
 
-(defun package-delete (pkg-desc)
-  (let ((dir (package-desc-dir pkg-desc)))
-    (if (not (string-prefix-p (file-name-as-directory
-                               (expand-file-name package-user-dir))
-                              (expand-file-name dir)))
-        ;; Don't delete "system" packages.
-	(error "Package `%s' is a system package, not deleting"
-               (package-desc-full-name pkg-desc))
-      (delete-directory dir t t)
-      ;; Remove NAME-VERSION.signed file.
-      (let ((signed-file (concat dir ".signed")))
-	(if (file-exists-p signed-file)
-	    (delete-file signed-file)))
-      ;; Update package-alist.
-      (let* ((name (package-desc-name pkg-desc))
-             (pkgs (assq name package-alist)))
-        (delete pkg-desc pkgs)
-        (unless (cdr pkgs)
-          (setq package-alist (delq pkgs package-alist))))
-      (message "Package `%s' deleted." (package-desc-full-name pkg-desc)))))
+(defun package--get-deps (pkg &optional only)
+  (let* ((pkg-desc (cadr (assq pkg package-alist)))
+         (direct-deps (cl-loop for p in (package-desc-reqs pkg-desc)
+                               for name = (car p)
+                               when (assq name package-alist)
+                               collect name))
+         (indirect-deps (unless (eq only 'direct)
+                          (cl-loop for p in direct-deps
+                                   for dep = (cadr (assq p package-alist))
+                                   when (and dep (assq p package-alist))
+                                   append (mapcar 'car
+                                                  (package-desc-reqs
+                                                   dep))))))
+    (cl-case only
+      (direct   direct-deps)
+      (separate (list direct-deps indirect-deps))
+      (indirect indirect-deps)
+      (t        (append direct-deps indirect-deps)))))
+
+;;;###autoload
+(defun package-user-selected-packages-install ()
+  "Ensure packages in `package-selected-packages' are installed.
+If some packages are not installed propose to install them."
+  (interactive)
+  (cl-loop for p in package-selected-packages
+           unless (package-installed-p p)
+           collect p into lst
+           finally
+           (if lst
+               (when (y-or-n-p
+                      (format "%s packages will be installed:\n%s, proceed?"
+                              (length lst)
+                              (mapconcat 'symbol-name lst ", ")))
+                 (mapc 'package-install lst))
+               (message "All your packages are already installed"))))
+
+(defun package-used-elsewhere-p (pkg-desc &optional pkg-list)
+  "Check in PKG-LIST if PKG-DESC is used elsewhere as dependency.
+
+When not specified, PKG-LIST default to `package-alist'
+with PKG-DESC entry removed.
+Returns the first package found in PKG-LIST where PKG is used as dependency."
+  (unless (string= (package-desc-status pkg-desc) "obsolete")
+    (let ((pkg (package-desc-name pkg-desc)))
+      (cl-loop with alist = (or pkg-list
+                                (remove (assq pkg package-alist)
+                                        package-alist))
+               for p in alist thereis
+               (and (memq pkg (mapcar 'car (package-desc-reqs (cadr p))))
+                    (car p))))))
+
+(defun package-delete (pkg-desc &optional force)
+  "Delete package PKG-DESC.
+
+Argument PKG-DESC is a full description of package as vector.
+When package is used elsewhere as dependency of another package,
+refuse deleting it and return an error.
+If FORCE is non--nil package will be deleted even if it is used
+elsewhere."
+  (let ((dir (package-desc-dir pkg-desc))
+        (name (package-desc-name pkg-desc))
+        pkg-used-elsewhere-by)
+    (cond ((not (string-prefix-p (file-name-as-directory
+                                  (expand-file-name package-user-dir))
+                                 (expand-file-name dir)))
+           ;; Don't delete "system" packages.
+           (error "Package `%s' is a system package, not deleting"
+                  (package-desc-full-name pkg-desc)))
+          ((and (null force)
+                (setq pkg-used-elsewhere-by
+                      (package-used-elsewhere-p name)))
+           ;; Don't delete packages used as dependency elsewhere.
+           (error "Package `%s' is used by `%s' as dependency, not deleting"
+                  (package-desc-full-name pkg-desc)
+                  pkg-used-elsewhere-by))
+          (t 
+           (delete-directory dir t t)
+           ;; Remove NAME-VERSION.signed file.
+           (let ((signed-file (concat dir ".signed")))
+             (if (file-exists-p signed-file)
+                 (delete-file signed-file)))
+           ;; Update package-alist.
+           (let ((pkgs (assq name package-alist)))
+             (delete pkg-desc pkgs)
+             (unless (cdr pkgs)
+               (setq package-alist (delq pkgs package-alist))))
+           (message "Package `%s' deleted." (package-desc-full-name pkg-desc))))))
+
+;;;###autoload
+(defun package-autoremove ()
+  "Remove packages that are no more needed.
+
+Packages that are no more needed by other packages in
+`package-selected-packages' and their dependencies
+will be deleted."
+  (interactive)
+  (let* (old-direct
+         (needed (cl-loop for p in package-selected-packages
+                          if (assq p package-alist)
+                          append (package--get-deps p) into lst
+                          else do (push p old-direct)
+                          finally return lst)))
+    (cl-loop for p in (mapcar 'car package-alist)
+             unless (or (memq p needed)
+                        (memq p package-selected-packages))
+             collect p into lst
+             finally (if lst
+                         (when (y-or-n-p (format "%s packages will be deleted:\n%s, proceed? "
+                                                 (length lst)
+                                                 (mapconcat 'symbol-name lst ", ")))
+                           (mapc (lambda (p)
+                                   (package-delete (cadr (assq p package-alist)) t))
+                                 lst))
+                       (message "Nothing to autoremove")))))
 
 (defun package-archive-base (desc)
   "Return the archive containing the package NAME."
@@ -1721,7 +1848,7 @@ If optional arg NO-ACTIVATE is non-nil, don't activate packages."
   (let ((pkg-desc (button-get button 'package-desc)))
     (when (y-or-n-p (format "Install package `%s'? "
                             (package-desc-full-name pkg-desc)))
-      (package-install pkg-desc)
+      (package-install pkg-desc 1)
       (revert-buffer nil t)
       (goto-char (point-min)))))
 
@@ -2178,7 +2305,9 @@ Optional argument NOQUERY non-nil means do not ask the user to confirm."
                       (length install-list)
                       (mapconcat #'package-desc-full-name
                                  install-list ", ")))))
-	  (mapc 'package-install install-list)))
+	  (mapc (lambda (p)
+                  (package-install p (and (null (package-installed-p p)) 1)))
+                install-list)))
     ;; Delete packages, prompting if necessary.
     (when delete-list
       (if (or

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply related	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-31 23:26           ` Artur Malabarba
  2015-02-01  6:29             ` Thierry Volpiatto
  2015-02-01  7:02             ` Thierry Volpiatto
@ 2015-02-01 15:55             ` Thierry Volpiatto
  2015-02-01 23:47               ` Artur Malabarba
  2 siblings, 1 reply; 79+ messages in thread
From: Thierry Volpiatto @ 2015-02-01 15:55 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


Artur Malabarba <bruce.connor.am@gmail.com> writes:

>> While you're at it: if package-selected-packages (or whatever name you
>> end up using) is not yet set (e.g. still nil), you could guess an
>> initial value: take the list of packages that are installed and are not
>> required by other installed packages. I.e. choose as initial value of
>> package-selected-packages the smallest set of packages such that
>> package-autoremove won't have anything to remove.
>
> I'll work on this, and then I'll add it in once this patch is applied.

As a starting point:

(cl-loop for p in package-alist
         for name = (car p)
         unless
         (cl-loop for pkg in package-alist thereis
                  (memq name
                        (mapcar 'car
                                (package-desc-reqs (cadr pkg)))))
         collect name)

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-01 15:55             ` Thierry Volpiatto
@ 2015-02-01 23:47               ` Artur Malabarba
  0 siblings, 0 replies; 79+ messages in thread
From: Artur Malabarba @ 2015-02-01 23:47 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: emacs-devel

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

> As a starting point:
>
> (cl-loop for p in package-alist
>          for name = (car p)
>          unless
>          (cl-loop for pkg in package-alist thereis
>                   (memq name
>                         (mapcar 'car
>                                 (package-desc-reqs (cadr pkg)))))
>          collect name)

Thanks, I'll push it tomorrow, and it'll be quite similar to that.

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

^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-01-31 20:30         ` Stefan Monnier
  2015-01-31 22:10           ` Thierry Volpiatto
  2015-01-31 23:26           ` Artur Malabarba
@ 2015-02-02 12:00           ` Artur Malabarba
  2015-02-02 13:14             ` Thierry Volpiatto
  2 siblings, 1 reply; 79+ messages in thread
From: Artur Malabarba @ 2015-02-02 12:00 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel, Thierry Volpiatto

Done.

2015-01-31 20:30 GMT+00:00 Stefan Monnier <monnier@iro.umontreal.ca>:
>> Anyway, this is not working, and need to be fixed differently, will do
>> next week.
>
> While you're at it: if package-selected-packages (or whatever name you
> end up using) is not yet set (e.g. still nil), you could guess an
> initial value: take the list of packages that are installed and are not
> required by other installed packages.  I.e. choose as initial value of
> package-selected-packages the smallest set of packages such that
> package-autoremove won't have anything to remove.
>
>
>         Stefan



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-02 12:00           ` Artur Malabarba
@ 2015-02-02 13:14             ` Thierry Volpiatto
  2015-02-02 14:14               ` Thierry Volpiatto
  0 siblings, 1 reply; 79+ messages in thread
From: Thierry Volpiatto @ 2015-02-02 13:14 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


Artur Malabarba <bruce.connor.am@gmail.com> writes:

> Done.

We have now duplicates with package--get-deps:

(package--get-deps 'jedi)
=>(epc auto-complete python-environment epc auto-complete
python-environment 
concurrent ctable concurrent ctable deferred deferred 
popup popup deferred deferred)

As a workaround, you can use delete-dups, but this need to be
implemented differently IMO.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-02 13:14             ` Thierry Volpiatto
@ 2015-02-02 14:14               ` Thierry Volpiatto
  2015-02-02 14:56                 ` Artur Malabarba
  0 siblings, 1 reply; 79+ messages in thread
From: Thierry Volpiatto @ 2015-02-02 14:14 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:

> Artur Malabarba <bruce.connor.am@gmail.com> writes:
>
>> Done.
>
> We have now duplicates with package--get-deps:
>
> (package--get-deps 'jedi)
> =>(epc auto-complete python-environment epc auto-complete
> python-environment 
> concurrent ctable concurrent ctable deferred deferred 
> popup popup deferred deferred)
>
> As a workaround, you can use delete-dups, but this need to be
> implemented differently IMO.

Maybe like this: 

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 9a29d63..2157174 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1428,9 +1428,8 @@ The file can either be a tar file or an Emacs Lisp file."
                                when (assq name package-alist)
                                collect name))
          (indirect-deps (unless (eq only 'direct)
-                          (apply #'append
-                            direct-deps
-                            (mapcar #'package--get-deps direct-deps)))))
+                          (cl-loop for p in direct-deps
+                                append (package--get-deps p 'direct)))))
     (cl-case only
       (direct   direct-deps)
       (separate (list direct-deps indirect-deps))

(package--get-deps 'jedi)
=> (epc auto-complete python-environment concurrent ctable popup deferred)

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply related	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-02 14:14               ` Thierry Volpiatto
@ 2015-02-02 14:56                 ` Artur Malabarba
  2015-02-02 15:19                   ` Thierry Volpiatto
  0 siblings, 1 reply; 79+ messages in thread
From: Artur Malabarba @ 2015-02-02 14:56 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

>> We have now duplicates with package--get-deps:
>>
>> (package--get-deps 'jedi)
>> =>(epc auto-complete python-environment epc auto-complete
>> python-environment
>> concurrent ctable concurrent ctable deferred deferred
>> popup popup deferred deferred)
>>
>> As a workaround, you can use delete-dups, but this need to be
>> implemented differently IMO.
>
> Maybe like this:
>
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index 9a29d63..2157174 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -1428,9 +1428,8 @@ The file can either be a tar file or an Emacs Lisp file."
>                                 when (assq name package-alist)
>                                 collect name))
>           (indirect-deps (unless (eq only 'direct)
> -                          (apply #'append
> -                            direct-deps

Yes, I made a mistake by adding the `direct-deps' here. I'll remove it.

> -                            (mapcar #'package--get-deps direct-deps)))))
> +                          (cl-loop for p in direct-deps
> +                                append (package--get-deps p 'direct)))))
>      (cl-case only
>        (direct   direct-deps)
>        (separate (list direct-deps indirect-deps))
>
> (package--get-deps 'jedi)
> => (epc auto-complete python-environment concurrent ctable popup deferred)

It may have solved this particular example, but if two direct
dependencies share the same (indirect) dependency then this will still
lead to duplicates. We could try writting an efficient way of avoiding
this duplicates, but I'm fine with just using `delete-dups'.

And one more thing. Why did you use `(package--get-deps p 'direct)' in
this snippet? Passing the `direct' argument will cause it to only
return 2nd level dependencies at most (direct dependencies of the
direct dependencies). I think it should be just `(package--get-deps
p)', shouldn't it? This way it'll keep searching untill the bottom of
the dependency tree.



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-02 14:56                 ` Artur Malabarba
@ 2015-02-02 15:19                   ` Thierry Volpiatto
  2015-02-02 15:33                     ` Thierry Volpiatto
  0 siblings, 1 reply; 79+ messages in thread
From: Thierry Volpiatto @ 2015-02-02 15:19 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


Artur Malabarba <bruce.connor.am@gmail.com> writes:

>>> We have now duplicates with package--get-deps:
>>>
>>> (package--get-deps 'jedi)
>>> =>(epc auto-complete python-environment epc auto-complete
>>> python-environment
>>> concurrent ctable concurrent ctable deferred deferred
>>> popup popup deferred deferred)
>>>
>>> As a workaround, you can use delete-dups, but this need to be
>>> implemented differently IMO.
>>
>> Maybe like this:
>>
>> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
>> index 9a29d63..2157174 100644
>> --- a/lisp/emacs-lisp/package.el
>> +++ b/lisp/emacs-lisp/package.el
>> @@ -1428,9 +1428,8 @@ The file can either be a tar file or an Emacs Lisp file."
>>                                 when (assq name package-alist)
>>                                 collect name))
>>           (indirect-deps (unless (eq only 'direct)
>> -                          (apply #'append
>> -                            direct-deps
>
> Yes, I made a mistake by adding the `direct-deps' here. I'll remove it.
>
>> -                            (mapcar #'package--get-deps direct-deps)))))
>> +                          (cl-loop for p in direct-deps
>> +                                append (package--get-deps p 'direct)))))
>>      (cl-case only
>>        (direct   direct-deps)
>>        (separate (list direct-deps indirect-deps))
>>
>> (package--get-deps 'jedi)
>> => (epc auto-complete python-environment concurrent ctable popup deferred)
>
> It may have solved this particular example, but if two direct
> dependencies share the same (indirect) dependency then this will still
> lead to duplicates. We could try writting an efficient way of avoiding
> this duplicates, but I'm fine with just using `delete-dups'.

Yes I think using delete-dups is ok for the extras duplicates in this
case, but see below to avoid most of the duplicates.
So it would be:

(cl-loop for p in direct-deps
         append (package--get-deps p 'direct) into deps
         finally return (delete-dups deps))

> And one more thing. Why did you use `(package--get-deps p 'direct)' in
> this snippet? Passing the `direct' argument will cause it to only
> return 2nd level dependencies at most (direct dependencies of the
> direct dependencies).

Don't think so, see below.

> I think it should be just `(package--get-deps p)', shouldn't it?

No, this avoid duplicates and it will in turn recurse in the direct
dependency of the dependency and so on, if you don't specify 'direct you
will have immediately duplicates.

> This way it'll keep searching untill the bottom of the dependency
> tree.

It will with the recursion and the 'direct flag.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-02 15:19                   ` Thierry Volpiatto
@ 2015-02-02 15:33                     ` Thierry Volpiatto
  2015-02-02 15:50                       ` Artur Malabarba
  0 siblings, 1 reply; 79+ messages in thread
From: Thierry Volpiatto @ 2015-02-02 15:33 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:

>> And one more thing. Why did you use `(package--get-deps p 'direct)' in
>> this snippet? Passing the `direct' argument will cause it to only
>> return 2nd level dependencies at most (direct dependencies of the
>> direct dependencies).

Consider following example using this definition of package--get-deps:

--8<---------------cut here---------------start------------->8---
(defun package--get-deps (pkg &optional only)
  (let* ((pkg-desc (cadr (assq pkg package-alist)))
         (direct-deps (cl-loop for p in (package-desc-reqs pkg-desc)
                               for name = (car p)
                               when (assq name package-alist)
                               collect name))
         (indirect-deps (unless (eq only 'direct)
                          (cl-loop for p in direct-deps
                                append (package--get-deps p 'direct)))))
    (cl-case only
      (direct   direct-deps)
      (separate (list direct-deps indirect-deps))
      (indirect indirect-deps)
      (t        (append direct-deps indirect-deps)))))
--8<---------------cut here---------------end--------------->8---

Here the dependencies of the package "jedi":

(package--get-deps 'jedi 'direct)
=>(epc auto-complete python-environment)

(package--get-deps 'epc 'direct)
=>(concurrent ctable)

(package--get-deps 'concurrent 'direct)
=>(deferred)

(package--get-deps 'ctable 'direct)
=>nil

(package--get-deps 'auto-complete 'direct)
=>(popup)

(package--get-deps 'python-environment 'direct)
=>(deferred)

(package--get-deps 'jedi)
=>(epc auto-complete python-environment concurrent ctable popup deferred)

As you can see all the dependencies are here and there is no duplicates.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-02 15:33                     ` Thierry Volpiatto
@ 2015-02-02 15:50                       ` Artur Malabarba
  2015-02-02 16:07                         ` Thierry Volpiatto
  2015-02-02 21:23                         ` Thierry Volpiatto
  0 siblings, 2 replies; 79+ messages in thread
From: Artur Malabarba @ 2015-02-02 15:50 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

2015-02-02 15:33 GMT+00:00 Thierry Volpiatto <thierry.volpiatto@gmail.com>:
>
> Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:
>
>>> And one more thing. Why did you use `(package--get-deps p 'direct)' in
>>> this snippet? Passing the `direct' argument will cause it to only
>>> return 2nd level dependencies at most (direct dependencies of the
>>> direct dependencies).
>
> Consider following example using this definition of package--get-deps:
>
> --8<---------------cut here---------------start------------->8---
> (defun package--get-deps (pkg &optional only)
>   (let* ((pkg-desc (cadr (assq pkg package-alist)))
>          (direct-deps (cl-loop for p in (package-desc-reqs pkg-desc)
>                                for name = (car p)
>                                when (assq name package-alist)
>                                collect name))
>          (indirect-deps (unless (eq only 'direct)
>                           (cl-loop for p in direct-deps
>                                 append (package--get-deps p 'direct)))))
>     (cl-case only
>       (direct   direct-deps)
>       (separate (list direct-deps indirect-deps))
>       (indirect indirect-deps)
>       (t        (append direct-deps indirect-deps)))))
> --8<---------------cut here---------------end--------------->8---

I think the snippet you suggest lists 1st and 2nd order dependencies.
It does not, in general, list all dependencies without duplicates.

> Here the dependencies of the package "jedi":
> [...]
> As you can see all the dependencies are here and there is no duplicates.

In this particular case, yes, but that's just a coincidence. What the
function is actually doing, is to return 1st and 2nd order
dependencies.
I explain below:

1st order dependencies
> (package--get-deps 'jedi 'direct)
> =>(epc auto-complete python-environment)

2nd order dependencies
> (package--get-deps 'epc 'direct)
> =>(concurrent ctable)
> (package--get-deps 'auto-complete 'direct)
> =>(popup)
> (package--get-deps 'python-environment 'direct)
> =>(deferred)

3rd order dependencies:
> (package--get-deps 'concurrent 'direct)
> =>(deferred)
> (package--get-deps 'ctable 'direct)
> =>nil

Union of the 1st and 2nd order:
> (package--get-deps 'jedi)
> =>(epc auto-complete python-environment concurrent ctable popup deferred)

1. The only reason you don't get duplicates there, is because the
dependencies of the concurrent and ctable packages are not even being
looked at.

2. Similarly, the only reason you don't have omissions in the
resulting list, is because these two packages don't depend on anything
different from the other packages. If 'ctable or 'concurrent happened
to depend on X, then X would not show up on your returned list.



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-02 15:50                       ` Artur Malabarba
@ 2015-02-02 16:07                         ` Thierry Volpiatto
  2015-02-02 21:23                         ` Thierry Volpiatto
  1 sibling, 0 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-02-02 16:07 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


Artur Malabarba <bruce.connor.am@gmail.com> writes:

> I think the snippet you suggest lists 1st and 2nd order dependencies.
> It does not, in general, list all dependencies without duplicates.
>
>> Here the dependencies of the package "jedi":
>> [...]
>> As you can see all the dependencies are here and there is no duplicates.
>
> In this particular case, yes, but that's just a coincidence. What the
> function is actually doing, is to return 1st and 2nd order
> dependencies.
> I explain below:
>
> 1st order dependencies
>> (package--get-deps 'jedi 'direct)
>> =>(epc auto-complete python-environment)
>
> 2nd order dependencies
>> (package--get-deps 'epc 'direct)
>> =>(concurrent ctable)
>> (package--get-deps 'auto-complete 'direct)
>> =>(popup)
>> (package--get-deps 'python-environment 'direct)
>> =>(deferred)
>
> 3rd order dependencies:
>> (package--get-deps 'concurrent 'direct)
>> =>(deferred)
>> (package--get-deps 'ctable 'direct)
>> =>nil
>
> Union of the 1st and 2nd order:
>> (package--get-deps 'jedi)
>> =>(epc auto-complete python-environment concurrent ctable popup deferred)
>
> 1. The only reason you don't get duplicates there, is because the
> dependencies of the concurrent and ctable packages are not even being
> looked at.
>
> 2. Similarly, the only reason you don't have omissions in the
> resulting list, is because these two packages don't depend on anything
> different from the other packages. If 'ctable or 'concurrent happened
> to depend on X, then X would not show up on your returned list.

Hmm, I will have a look at this more seriously, another thing:
We have a bug about the initialization of package-selected-packages:
bug#19745
On my side I have no error, but package-selected-packages is not feeded
after package-initialize run.  Using my own version of
package-initialize where package-selected-packages is just setq'ed with
the snippet I sent you, it is working as expected.
It seems that for some reason customize-set-variable is not working at
this time of loading, perhaps we should just use setq here ?

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
       [not found]                         ` <jwvy4oggva5.fsf-monnier+emacs@gnu.org>
@ 2015-02-02 20:35                           ` Thierry Volpiatto
  2015-02-02 21:37                             ` Artur Malabarba
  2015-02-03 11:39                             ` Artur Malabarba
  2015-02-02 21:19                           ` Thierry Volpiatto
  1 sibling, 2 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-02-02 20:35 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Artur Malabarba, emacs-devel


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

> We have a problem: since package-reinstall does a `package-delete'
> and since this `package-delete' always marks the package as unselected,
> it follows that package-reinstall will always mark the package as
> unselected :-(

This patch fix this issue:

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 9a29d63..296ee46 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1239,7 +1239,8 @@ to `package-selected-packages'."
                               "Reinstall package: "
                               (mapcar #'symbol-name
                                       (mapcar #'car package-alist))))))
-  (package-delete (cadr (assq pkg package-alist)) t)
+  (package-delete (cadr (assq pkg package-alist)) 'force
+                  (memq pkg package-selected-packages))
   (package-install pkg))
 
 (defun package-strip-rcs-id (str)
@@ -1470,7 +1471,7 @@ with PKG-DESC entry removed."
                (and (memq pkg (mapcar #'car (package-desc-reqs (cadr p))))
                     (car p))))))
 
-(defun package-delete (pkg-desc &optional force)
+(defun package-delete (pkg-desc &optional force nosave)
   "Delete package PKG-DESC.
 
 Argument PKG-DESC is a full description of package as vector.
@@ -1506,7 +1507,8 @@ elsewhere."
              (unless (cdr pkgs)
                (setq package-alist (delq pkgs package-alist))))
            ;; Update package-selected-packages.
-           (when (memq name package-selected-packages)
+           (when (and (memq name package-selected-packages)
+                      (null nosave))
              (customize-save-variable
               'package-selected-packages (remove name package-selected-packages)))
            (message "Package `%s' deleted." (package-desc-full-name pkg-desc))))))

> One more thing: now that we have package-selected-packages,
> package-delete should be fixed to be a real "inverse" of
> package-install, i.e. it should delete its unused dependencies.

Hmm, not sure it is a good idea.

> One more thing: it'd be great to extend the list-package display so as
> to indicate which packages are selected and which ones aren't.

I already did this in helm, with "I" in first column, but in helm the
first 2 columns are not needed like in list-package ("I" and "D").
Will see what I can do even if I don't use this UI.

Will look in next issues tomorrow.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply related	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
       [not found]                         ` <jwvy4oggva5.fsf-monnier+emacs@gnu.org>
  2015-02-02 20:35                           ` Thierry Volpiatto
@ 2015-02-02 21:19                           ` Thierry Volpiatto
  2015-02-02 21:22                             ` Dmitry Gutov
  1 sibling, 1 reply; 79+ messages in thread
From: Thierry Volpiatto @ 2015-02-02 21:19 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Artur Malabarba, emacs-devel


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

> Looks good, feel free to install.  See some further comments below.

The version of package--get-deps Arthur pushed (he modify original
version) returns many duplicates and I am not sure it recurse correctly,
I suggest this version (See discussion on emacs-devel), can you review it ? Thanks.

--8<---------------cut here---------------start------------->8---
(defun package--get-deps (pkg &optional only)
  (let* ((pkg-desc (cadr (assq pkg package-alist)))
         (direct-deps (cl-loop for p in (package-desc-reqs pkg-desc)
                               for name = (car p)
                               when (assq name package-alist)
                               collect name))
         (indirect-deps (unless (eq only 'direct)
                          (cl-loop for p in direct-deps
                                append (package--get-deps p 'direct) into lst
                                finally return (delete-dups lst)))))
    (cl-case only
      (direct   direct-deps)
      (separate (list direct-deps indirect-deps))
      (indirect indirect-deps)
      (t        (append direct-deps indirect-deps)))))
--8<---------------cut here---------------end--------------->8---


-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-02 21:19                           ` Thierry Volpiatto
@ 2015-02-02 21:22                             ` Dmitry Gutov
  2015-02-03 11:39                               ` Artur Malabarba
  0 siblings, 1 reply; 79+ messages in thread
From: Dmitry Gutov @ 2015-02-02 21:22 UTC (permalink / raw)
  To: Thierry Volpiatto, Stefan Monnier; +Cc: Artur Malabarba, emacs-devel

On 02/02/2015 11:19 PM, Thierry Volpiatto wrote:
>
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> Looks good, feel free to install.  See some further comments below.
>
> The version of package--get-deps Arthur pushed (he modify original
> version) returns many duplicates and I am not sure it recurse correctly,

Could either of your write a test or two for this feature?

Then it'll be much easier to be sure the related functions work and 
"recurse correctly".



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-02 15:50                       ` Artur Malabarba
  2015-02-02 16:07                         ` Thierry Volpiatto
@ 2015-02-02 21:23                         ` Thierry Volpiatto
  1 sibling, 0 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-02-02 21:23 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


Artur Malabarba <bruce.connor.am@gmail.com> writes:

> 1. The only reason you don't get duplicates there, is because the
> dependencies of the concurrent and ctable packages are not even being
> looked at.

Agree on this one, just need to remove dups so.

> 2. Similarly, the only reason you don't have omissions in the
> resulting list, is because these two packages don't depend on anything
> different from the other packages. If 'ctable or 'concurrent happened
> to depend on X, then X would not show up on your returned list.

I am not sure of this I think the version I sended recurse in all
packages. I will double check tomorrow though.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-02 20:35                           ` Thierry Volpiatto
@ 2015-02-02 21:37                             ` Artur Malabarba
  2015-02-03  4:53                               ` Thierry Volpiatto
  2015-02-03  5:45                               ` Thierry Volpiatto
  2015-02-03 11:39                             ` Artur Malabarba
  1 sibling, 2 replies; 79+ messages in thread
From: Artur Malabarba @ 2015-02-02 21:37 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

2015-02-02 18:35 GMT-02:00 Thierry Volpiatto <thierry.volpiatto@gmail.com>:
>
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> We have a problem: since package-reinstall does a `package-delete'
>> and since this `package-delete' always marks the package as unselected,
>> it follows that package-reinstall will always mark the package as
>> unselected :-(
>
> This patch fix this issue:
>
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index 9a29d63..296ee46 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -1239,7 +1239,8 @@ to `package-selected-packages'."
>                                "Reinstall package: "
>                                (mapcar #'symbol-name
>                                        (mapcar #'car package-alist))))))
> -  (package-delete (cadr (assq pkg package-alist)) t)
> +  (package-delete (cadr (assq pkg package-alist)) 'force
> +                  (memq pkg package-selected-packages))
>    (package-install pkg))
>
>  (defun package-strip-rcs-id (str)
> @@ -1470,7 +1471,7 @@ with PKG-DESC entry removed."
>                 (and (memq pkg (mapcar #'car (package-desc-reqs (cadr p))))
>                      (car p))))))
>
> -(defun package-delete (pkg-desc &optional force)
> +(defun package-delete (pkg-desc &optional force nosave)
>    "Delete package PKG-DESC.
>
>  Argument PKG-DESC is a full description of package as vector.
> @@ -1506,7 +1507,8 @@ elsewhere."
>               (unless (cdr pkgs)
>                 (setq package-alist (delq pkgs package-alist))))
>             ;; Update package-selected-packages.
> -           (when (memq name package-selected-packages)
> +           (when (and (memq name package-selected-packages)
> +                      (null nosave))
>               (customize-save-variable
>                'package-selected-packages (remove name package-selected-packages)))
>             (message "Package `%s' deleted." (package-desc-full-name pkg-desc))))))

I was about to suggest the following.

@@ -1238,9 +1238,10 @@ to `package-selected-packages'."
   (interactive (list (intern (completing-read
                               "Reinstall package: "
                               (mapcar #'symbol-name
-                                      (mapcar #'car package-alist))))))
-  (package-delete (cadr (assq pkg package-alist)) t)
-  (package-install pkg))
+                                (mapcar #'car package-alist))))))
+  (let ((selected (memq pkg package-selected-packages)))
+    (package-delete (cadr (assq pkg package-alist)) 'force)
+    (package-install pkg selected)))

 (defun package-strip-rcs-id (str)
   "Strip RCS version ID from the version string STR.

>> One more thing: now that we have package-selected-packages,
>> package-delete should be fixed to be a real "inverse" of
>> package-install, i.e. it should delete its unused dependencies.
>
> Hmm, not sure it is a good idea.

I'm on the fence on this. Personally, I would automate this (as Stefan
suggests).
But, of all linux package managers I've used, I don't remember any of
them removing dependencies without explicit request.
Other options, for the sake of completeness:

1. Just like we say "N packages can be upgraded, hit U...", we could
have a message saying "N packages are dependencies which are no longer
needed, hit R to remove them".
2. After `package-execute' is done, we could check if there are any
unneeded deps and ask the user a 3rd question "The following
dependencies are no longer necessary, remove them? ...". If the user
answers `n' here, we can also ask "mark these as explicitily selected
to prevent future autoremoval?".

>> One more thing: it'd be great to extend the list-package display so as
>> to indicate which packages are selected and which ones aren't.
>
> I already did this in helm, with "I" in first column, but in helm the
> first 2 columns are not needed like in list-package ("I" and "D").
> Will see what I can do even if I don't use this UI.

My suggestion would be to change the "installed" string (displayed on
the status column) to "dependency" for packages that are not in
`package-selected-packages'.



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-02 21:37                             ` Artur Malabarba
@ 2015-02-03  4:53                               ` Thierry Volpiatto
  2015-02-03  5:13                                 ` Stefan Monnier
                                                   ` (2 more replies)
  2015-02-03  5:45                               ` Thierry Volpiatto
  1 sibling, 3 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-02-03  4:53 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


Artur Malabarba <bruce.connor.am@gmail.com> writes:

>
> I was about to suggest the following.
>
> +    (package-delete (cadr (assq pkg package-alist)) 'force)
> +    (package-install pkg selected)))

At first I had the same idea, but after I thought it was calling two time
custom-save-varaible for nothing (i.e one time to remove the package and
on time to add it again), so I think it is better to just delete the
package without removing it from the list.

>>> One more thing: now that we have package-selected-packages,
>>> package-delete should be fixed to be a real "inverse" of
>>> package-install, i.e. it should delete its unused dependencies.
>>
>> Hmm, not sure it is a good idea.
>
> I'm on the fence on this. Personally, I would automate this (as Stefan
> suggests).
> But, of all linux package managers I've used, I don't remember any of
> them removing dependencies without explicit request.
> Other options, for the sake of completeness:
>
> 1. Just like we say "N packages can be upgraded, hit U...", we could
> have a message saying "N packages are dependencies which are no longer
> needed, hit R to remove them".
> 2. After `package-execute' is done, we could check if there are any
> unneeded deps and ask the user a 3rd question "The following
> dependencies are no longer necessary, remove them? ...". If the user
> answers `n' here, we can also ask "mark these as explicitily selected
> to prevent future autoremoval?".

apt-get is sending a message when running install saying "x packages are
no more needed, remove them with apt-get autoremove", I think this would
be enough.

> My suggestion would be to change the "installed" string (displayed on
> the status column) to "dependency" for packages that are not in
> `package-selected-packages'.

Agree.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-03  4:53                               ` Thierry Volpiatto
@ 2015-02-03  5:13                                 ` Stefan Monnier
  2015-02-03 10:04                                 ` Artur Malabarba
  2015-02-03 14:06                                 ` Artur Malabarba
  2 siblings, 0 replies; 79+ messages in thread
From: Stefan Monnier @ 2015-02-03  5:13 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: bruce.connor.am, emacs-devel

> At first I had the same idea, but after I thought it was calling two time
> custom-save-varaible for nothing (i.e one time to remove the package and
> on time to add it again), so I think it is better to just delete the
> package without removing it from the list.

200% agreement, especially in case power goes out in the middle.

> apt-get is sending a message when running install saying "x packages are
> no more needed, remove them with apt-get autoremove", I think this would
> be enough.

I tend to completely overlook those messages, so I prefer aptitude which
removes them for me.  But yes, a prompt is OK, and in the end, either
behavior is acceptable.


        Stefan



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-02 21:37                             ` Artur Malabarba
  2015-02-03  4:53                               ` Thierry Volpiatto
@ 2015-02-03  5:45                               ` Thierry Volpiatto
  2015-02-03 10:05                                 ` Artur Malabarba
  1 sibling, 1 reply; 79+ messages in thread
From: Thierry Volpiatto @ 2015-02-03  5:45 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


The first thing we have to fix now is package--get-deps, Arthur, yes you
are right we should recurse without the 'direct flag, so we should use
this version:

(defun package--get-deps (pkg &optional only)
  (let* ((pkg-desc (cadr (assq pkg package-alist)))
         (direct-deps (cl-loop for p in (package-desc-reqs pkg-desc)
                               for name = (car p)
                               when (assq name package-alist)
                               collect name))
         (indirect-deps (unless (eq only 'direct)
                          (cl-loop for p in direct-deps
                                append (package--get-deps p) into lst
                                finally return (delete-dups lst)))))
    (cl-case only
      (direct   direct-deps)
      (separate (list direct-deps indirect-deps))
      (indirect indirect-deps)
      (t        (append direct-deps indirect-deps)))))


Can you push it ?

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-03  4:53                               ` Thierry Volpiatto
  2015-02-03  5:13                                 ` Stefan Monnier
@ 2015-02-03 10:04                                 ` Artur Malabarba
  2015-02-03 14:06                                 ` Artur Malabarba
  2 siblings, 0 replies; 79+ messages in thread
From: Artur Malabarba @ 2015-02-03 10:04 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

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

> > I was about to suggest the following.
> >
> > +    (package-delete (cadr (assq pkg package-alist)) 'force)
> > +    (package-install pkg selected)))
>
> At first I had the same idea, but after I thought it was calling two time
> custom-save-varaible for nothing (i.e one time to remove the package and
> on time to add it again), so I think it is better to just delete the
> package without removing it from the list.

Good point.

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

^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-03  5:45                               ` Thierry Volpiatto
@ 2015-02-03 10:05                                 ` Artur Malabarba
  2015-02-03 10:18                                   ` Thierry Volpiatto
  0 siblings, 1 reply; 79+ messages in thread
From: Artur Malabarba @ 2015-02-03 10:05 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

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

On 3 Feb 2015 05:45, "Thierry Volpiatto" <thierry.volpiatto@gmail.com>
wrote:
>
>
> The first thing we have to fix now is package--get-deps, Arthur, yes you
> are right we should recurse without the 'direct flag, so we should use
> this version:
>
> (defun package--get-deps (pkg &optional only)
>   (let* ((pkg-desc (cadr (assq pkg package-alist)))
>          (direct-deps (cl-loop for p in (package-desc-reqs pkg-desc)
>                                for name = (car p)
>                                when (assq name package-alist)
>                                collect name))
>          (indirect-deps (unless (eq only 'direct)
>                           (cl-loop for p in direct-deps
>                                 append (package--get-deps p) into lst
>                                 finally return (delete-dups lst)))))
>     (cl-case only
>       (direct   direct-deps)
>       (separate (list direct-deps indirect-deps))
>       (indirect indirect-deps)
>       (t        (append direct-deps indirect-deps)))))
>
>
> Can you push it ?

Yes, but possibly not today.

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

^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-03 10:05                                 ` Artur Malabarba
@ 2015-02-03 10:18                                   ` Thierry Volpiatto
  0 siblings, 0 replies; 79+ messages in thread
From: Thierry Volpiatto @ 2015-02-03 10:18 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: Stefan Monnier, emacs-devel


Artur Malabarba <bruce.connor.am@gmail.com> writes:

>> Can you push it ?
>
> Yes, but possibly not today.

Ignore, what you have installed does the same, just a difference of
style, not important.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-02 20:35                           ` Thierry Volpiatto
  2015-02-02 21:37                             ` Artur Malabarba
@ 2015-02-03 11:39                             ` Artur Malabarba
  1 sibling, 0 replies; 79+ messages in thread
From: Artur Malabarba @ 2015-02-03 11:39 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

2015-02-02 20:35 GMT+00:00 Thierry Volpiatto <thierry.volpiatto@gmail.com>:
>
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> We have a problem: since package-reinstall does a package-delete' &gt;&gt; and since thispackage-delete' always marks the package as unselected,
>> it follows that package-reinstall will always mark the package as
>> unselected :-(
>
> This patch fix this issue:

Pushed.



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-02 21:22                             ` Dmitry Gutov
@ 2015-02-03 11:39                               ` Artur Malabarba
  2015-02-03 11:44                                 ` Dmitry Gutov
  0 siblings, 1 reply; 79+ messages in thread
From: Artur Malabarba @ 2015-02-03 11:39 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel, Stefan Monnier, Thierry Volpiatto

Good point. Done.

2015-02-02 21:22 GMT+00:00 Dmitry Gutov <dgutov@yandex.ru>:
> On 02/02/2015 11:19 PM, Thierry Volpiatto wrote:
>>
>>
>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>
>>> Looks good, feel free to install.  See some further comments below.
>>
>>
>> The version of package--get-deps Arthur pushed (he modify original
>> version) returns many duplicates and I am not sure it recurse correctly,
>
>
> Could either of your write a test or two for this feature?
>
> Then it'll be much easier to be sure the related functions work and "recurse
> correctly".



^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-03 11:39                               ` Artur Malabarba
@ 2015-02-03 11:44                                 ` Dmitry Gutov
  0 siblings, 0 replies; 79+ messages in thread
From: Dmitry Gutov @ 2015-02-03 11:44 UTC (permalink / raw)
  To: bruce.connor.am; +Cc: emacs-devel, Stefan Monnier, Thierry Volpiatto

On 02/03/2015 01:39 PM, Artur Malabarba wrote:
> Good point. Done.

Thanks!

> 2015-02-02 21:22 GMT+00:00 Dmitry Gutov <dgutov@yandex.ru>:
>> On 02/02/2015 11:19 PM, Thierry Volpiatto wrote:
>>>
>>>
>>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>>
>>>> Looks good, feel free to install.  See some further comments below.
>>>
>>>
>>> The version of package--get-deps Arthur pushed (he modify original
>>> version) returns many duplicates and I am not sure it recurse correctly,
>>
>>
>> Could either of your write a test or two for this feature?
>>
>> Then it'll be much easier to be sure the related functions work and "recurse
>> correctly".




^ permalink raw reply	[flat|nested] 79+ messages in thread

* Re: package.el dependencies
  2015-02-03  4:53                               ` Thierry Volpiatto
  2015-02-03  5:13                                 ` Stefan Monnier
  2015-02-03 10:04                                 ` Artur Malabarba
@ 2015-02-03 14:06                                 ` Artur Malabarba
  2 siblings, 0 replies; 79+ messages in thread
From: Artur Malabarba @ 2015-02-03 14:06 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: Stefan Monnier, emacs-devel

>> I was about to suggest the following.
>>
>> +    (package-delete (cadr (assq pkg package-alist)) 'force)
>> +    (package-install pkg selected)))
>
> At first I had the same idea, but after I thought it was calling two time
> custom-save-varaible for nothing (i.e one time to remove the package and
> on time to add it again), so I think it is better to just delete the
> package without removing it from the list.

Good point. I've applied your patch.

>> My suggestion would be to change the "installed" string (displayed on
>> the status column) to "dependency" for packages that are not in
>> `package-selected-packages'.
>
> Agree.

Done.



^ permalink raw reply	[flat|nested] 79+ messages in thread

end of thread, other threads:[~2015-02-03 14:06 UTC | newest]

Thread overview: 79+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-23 13:37 package.el dependencies Thierry Volpiatto
2015-01-23 13:46 ` Dmitry Gutov
2015-01-23 14:12   ` Ivan Shmakov
2015-01-23 20:40 ` Stefan Monnier
2015-01-23 21:02   ` Thierry Volpiatto
2015-01-24  0:50     ` Artur Malabarba
2015-01-24  4:55       ` Stefan Monnier
2015-01-25  6:51         ` Thierry Volpiatto
2015-01-26  7:17         ` Thierry Volpiatto
2015-01-26  9:19           ` Artur Malabarba
2015-01-26  9:54             ` Thierry Volpiatto
2015-01-26 12:46               ` Artur Malabarba
2015-01-26 14:52           ` Stefan Monnier
2015-01-27  6:10             ` Thierry Volpiatto
2015-01-27 11:52               ` Artur Malabarba
2015-01-25  9:18   ` Thierry Volpiatto
2015-01-25 14:54     ` Stefan Monnier
2015-01-25 15:48       ` Thierry Volpiatto
2015-01-25 17:10         ` Dmitry Gutov
2015-01-25 18:32           ` Stephen Leake
2015-01-25 18:21         ` Artur Malabarba
2015-01-26  4:48           ` Thierry Volpiatto
2015-01-26 12:35             ` Artur Malabarba
2015-01-26 12:53         ` Artur Malabarba
2015-01-26 15:22   ` Thierry Volpiatto
2015-01-26 15:44     ` Stefan Monnier
2015-01-27  6:08       ` Thierry Volpiatto
2015-01-26 16:34     ` Artur Malabarba
2015-01-28  7:30   ` Thierry Volpiatto
2015-01-28  8:55     ` Thierry Volpiatto
2015-01-28 12:42       ` Thierry Volpiatto
2015-01-28 13:17         ` Artur Malabarba
2015-01-28 14:32           ` Thierry Volpiatto
2015-01-28 13:40         ` Dmitry Gutov
2015-01-28 10:47     ` Artur Malabarba
2015-01-28 11:58       ` Thierry Volpiatto
2015-01-28 19:33       ` Stefan Monnier
2015-01-28 19:50         ` Ivan Shmakov
2015-01-28 20:12           ` Artur Malabarba
2015-01-28 22:20           ` Stefan Monnier
2015-01-29  5:31         ` Thierry Volpiatto
2015-01-29  7:22           ` Thierry Volpiatto
2015-01-30  5:38   ` Thierry Volpiatto
2015-01-30 16:43     ` Artur Malabarba
2015-01-30 17:13       ` Thierry Volpiatto
2015-01-31  6:01       ` Thierry Volpiatto
2015-01-31 10:58         ` Artur Malabarba
2015-01-31 20:26         ` Stefan Monnier
     [not found]           ` <874mr67gjb.fsf@gmail.com>
     [not found]             ` <jwvvbjmnun4.fsf-monnier+emacs@gnu.org>
     [not found]               ` <87oapervqv.fsf@gmail.com>
     [not found]                 ` <jwvk302nnmd.fsf-monnier+emacs@gnu.org>
     [not found]                   ` <877fw2kp1y.fsf@gmail.com>
     [not found]                     ` <jwvioflbrlg.fsf-monnier+emacs@gnu.org>
     [not found]                       ` <87d25tps2q.fsf@gmail.com>
     [not found]                         ` <jwvy4oggva5.fsf-monnier+emacs@gnu.org>
2015-02-02 20:35                           ` Thierry Volpiatto
2015-02-02 21:37                             ` Artur Malabarba
2015-02-03  4:53                               ` Thierry Volpiatto
2015-02-03  5:13                                 ` Stefan Monnier
2015-02-03 10:04                                 ` Artur Malabarba
2015-02-03 14:06                                 ` Artur Malabarba
2015-02-03  5:45                               ` Thierry Volpiatto
2015-02-03 10:05                                 ` Artur Malabarba
2015-02-03 10:18                                   ` Thierry Volpiatto
2015-02-03 11:39                             ` Artur Malabarba
2015-02-02 21:19                           ` Thierry Volpiatto
2015-02-02 21:22                             ` Dmitry Gutov
2015-02-03 11:39                               ` Artur Malabarba
2015-02-03 11:44                                 ` Dmitry Gutov
2015-01-31  6:51       ` Thierry Volpiatto
2015-01-31 20:30         ` Stefan Monnier
2015-01-31 22:10           ` Thierry Volpiatto
2015-01-31 23:26           ` Artur Malabarba
2015-02-01  6:29             ` Thierry Volpiatto
2015-02-01  7:02             ` Thierry Volpiatto
2015-02-01 15:55             ` Thierry Volpiatto
2015-02-01 23:47               ` Artur Malabarba
2015-02-02 12:00           ` Artur Malabarba
2015-02-02 13:14             ` Thierry Volpiatto
2015-02-02 14:14               ` Thierry Volpiatto
2015-02-02 14:56                 ` Artur Malabarba
2015-02-02 15:19                   ` Thierry Volpiatto
2015-02-02 15:33                     ` Thierry Volpiatto
2015-02-02 15:50                       ` Artur Malabarba
2015-02-02 16:07                         ` Thierry Volpiatto
2015-02-02 21:23                         ` Thierry Volpiatto

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).