GNU bug report logs - #27822
26.0.50; package-autoremove and 'package-directory-list

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 27822 in the body.
You can then email your comments to 27822 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Yuri D'Elia <wavexx <at> thregr.org>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.0.50; package-autoremove and 'package-directory-list
Date: Tue, 25 Jul 2017 16:46:10 +0200

package-autoremove shouldn't attempt to remove external packages
(additional packages detected via in package-directory-list).

Debian now includes several copies of elpa packages in the repository.
The directory including those are added to 'package-directory-list.

I also manage some personal packages locally, and I also add the
directory into 'package'directory-list.

These packages are shown as "external" by "list-packages", which is
correct. They are obviously not directly installed via package-install,
and cannot be autoremoved either.

package-autoremove however will still attempt to remove them. My feeling
is that any package which is not directly in 'package-user-dir shouldn't
be autoremoved.

Message #8 received at 27822 <at> debbugs.gnu.org (full text, mbox):

From: Yuri D'Elia <wavexx <at> thregr.org>
To: emacs-devel <at> gnu.org, 27822 <at> debbugs.gnu.org,
Subject: [PATCH] do not auto-remove external packages (fixes #27822)
Date: Tue, 17 Jul 2018 13:16:04 +0200

[Message part 1 (text/plain, inline)]

User-agent: mu4e 1.1.0; emacs 27.0.50
The package-autoremove machinery assumes all packages which are not
user-selected are removable (see https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27822)

However this is blatantly untrue. For example, in debian unstable, emacs
packages can be installed system-wide via apt as well (see all the elpa-*
packages).

Such packages _cannot_ and shouldn't be auto-removed.

The following patch checks whether a package has been _directly_ installed
into `package-user-dir' before marking it eligible for auto-removal.

[copyright assignment already done]

[0001-Do-not-consider-external-packages-to-be-removable.patch (text/x-diff, inline)]

From 5a4234b3c1289960fd760256aa6399efbd04bfc3 Mon Sep 17 00:00:00 2001
From: Yuri D'Elia <wavexx <at> thregr.org>
Date: Tue, 17 Jul 2018 12:59:35 +0200
Subject: [PATCH] Do not consider external packages to be removable

Packages which are not directly user-installed shouldn't be autoremoved,
since they can be setup through a different path (via
`package-directory-list') where we have no authority over.
---
 lisp/emacs-lisp/package.el | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 36233522..534f155c 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1754,6 +1754,15 @@ if it is still empty."
       (indirect indirect-deps)
       (t        (delete-dups (append direct-deps indirect-deps))))))
 
+(defun package--user-installed-p (pkg)
+  "Check if the package is a user-installed package.
+PKG is a package name.
+Checks whether the package was installed into `package-user-dir' where
+we assume to have control over."
+  (let* ((pkg-desc (cadr (assq pkg package-alist)))
+         (dir (package-desc-dir pkg-desc)))
+    (file-in-directory-p dir package-user-dir)))
+
 (defun package--removable-packages ()
   "Return a list of names of packages no longer needed.
 These are packages which are neither contained in
@@ -1763,7 +1772,9 @@ These are packages which are neither contained in
                          ;; `p' and its dependencies are needed.
                          append (cons p (package--get-deps p)))))
     (cl-loop for p in (mapcar #'car package-alist)
-             unless (memq p needed)
+             unless (or (memq p needed)
+                        ;; do not auto-remove external packages
+                        (not (package--user-installed-p p)))
              collect p)))
 
 (defun package--used-elsewhere-p (pkg-desc &optional pkg-list all)
-- 
2.18.0

Message #13 received at 27822 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: "Yuri D'Elia" <wavexx <at> thregr.org>
Cc: 27822 <at> debbugs.gnu.org
Subject: Re: bug#27822: [PATCH] do not auto-remove external packages (fixes
 #27822)
Date: Tue, 17 Jul 2018 08:40:52 -0400

[ Please don't crosspost between bugs and emacs-devel.  ]

> package-autoremove however will still attempt to remove them. My feeling
> is that any package which is not directly in 'package-user-dir shouldn't
> be autoremoved.

AFAIK packages installed via dpkg can't be removed (lack of access
rights), so attempting to remove them is not that big of a deal, is it?
Or does it signal an error, prevent local packages from being
auto-removed, or something (I mean, beside the philosophical argument,
does it result in an actual harmful behavior in practice)?

> The following patch checks whether a package has been _directly_ installed
> into `package-user-dir' before marking it eligible for auto-removal.

Looks good to me,


        Stefan

Message #16 received at 27822 <at> debbugs.gnu.org (full text, mbox):

From: Yuri D'Elia <wavexx <at> thregr.org>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 27822 <at> debbugs.gnu.org
Subject: Re: bug#27822: [PATCH] do not auto-remove external packages (fixes
 #27822)
Date: Tue, 17 Jul 2018 14:50:00 +0200

On Tue, Jul 17 2018, Stefan Monnier wrote:
> AFAIK packages installed via dpkg can't be removed (lack of access
> rights), so attempting to remove them is not that big of a deal, is it?

As described in the bug report, if I add a directory of with custom
packages, autoremove will happily zap all of them because they're not
marked as manually installed.

Additional directories shouldn't be managed by package.el.

> Or does it signal an error, prevent local packages from being
> auto-removed, or something (I mean, beside the philosophical argument,
> does it result in an actual harmful behavior in practice)?

Both. Harmful behavior (deletion) in case there's an additional
directory of user-handled packages. Removal error due to missing
permissions also prevents dependent packages to be auto-removed.

Message #19 received at 27822 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Yuri D'Elia <wavexx <at> thregr.org>
Cc: 27822 <at> debbugs.gnu.org
Subject: Re: bug#27822: [PATCH] do not auto-remove external packages (fixes
 #27822)
Date: Wed, 08 Aug 2018 21:19:02 -0400

Yuri D'Elia <wavexx <at> thregr.org> writes:
>
>>From 5a4234b3c1289960fd760256aa6399efbd04bfc3 Mon Sep 17 00:00:00 2001
> From: Yuri D'Elia <wavexx <at> thregr.org>
> Date: Tue, 17 Jul 2018 12:59:35 +0200
> Subject: [PATCH] Do not consider external packages to be removable
>
> Packages which are not directly user-installed shouldn't be autoremoved,
> since they can be setup through a different path (via
> `package-directory-list') where we have no authority over.

The patch looks okay for emacs-26, I'll push in a few days assuming no
objections.

A couple of nitpicks below:

> +(defun package--user-installed-p (pkg)
> +  "Check if the package is a user-installed package.
> +PKG is a package name.

I think this is better shortened to

    Return non-nil if the package named PKG is user-installed.

> +Checks whether the package was installed into `package-user-dir' where
> +we assume to have control over."
> +  (let* ((pkg-desc (cadr (assq pkg package-alist)))
> +         (dir (package-desc-dir pkg-desc)))
> +    (file-in-directory-p dir package-user-dir)))
> +
>  (defun package--removable-packages ()
>    "Return a list of names of packages no longer needed.
>  These are packages which are neither contained in
> @@ -1763,7 +1772,9 @@ These are packages which are neither contained in

By the way, if you run './autogen --git' you should get nicer headers
for lisp diffs.

>                           ;; `p' and its dependencies are needed.
>                           append (cons p (package--get-deps p)))))
>      (cl-loop for p in (mapcar #'car package-alist)
> -             unless (memq p needed)
> +             unless (or (memq p needed)
> +                        ;; do not auto-remove external packages

Comments should be capitalized and end with a period.

> +                        (not (package--user-installed-p p)))
>               collect p)))

Message #22 received at 27822 <at> debbugs.gnu.org (full text, mbox):

From: Yuri D'Elia <wavexx <at> thregr.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 27822 <at> debbugs.gnu.org
Subject: Re: bug#27822: [PATCH] do not auto-remove external packages (fixes
 #27822)
Date: Thu, 09 Aug 2018 13:16:06 +0200

On Thu, Aug 09 2018, Noam Postavsky wrote:
>> @@ -1763,7 +1772,9 @@ These are packages which are neither contained in
>
> By the way, if you run './autogen --git' you should get nicer headers
> for lisp diffs.

Thanks for pointing this out!

As for the rest, I have no objections.
I assume I don't have to tweak the patch myself.

Message #25 received at 27822 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: wavexx <at> thregr.org, 27822 <at> debbugs.gnu.org
Subject: Re: bug#27822: [PATCH] do not auto-remove external packages (fixes
 #27822)
Date: Thu, 09 Aug 2018 18:24:18 +0300

> From: Noam Postavsky <npostavs <at> gmail.com>
> Date: Wed, 08 Aug 2018 21:19:02 -0400
> Cc: 27822 <at> debbugs.gnu.org
> 
> > +(defun package--user-installed-p (pkg)
> > +  "Check if the package is a user-installed package.
> > +PKG is a package name.
> 
> I think this is better shortened to
> 
>     Return non-nil if the package named PKG is user-installed.

Or maybe

  (defun package--user-installed-p (package)
    "Return non-nil if PACKAGE is a user-installed package.
  PACKAGE is the package name, a string."

Thanks.

Message #28 received at 27822 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: wavexx <at> thregr.org, 27822 <at> debbugs.gnu.org
Subject: Re: bug#27822: [PATCH] do not auto-remove external packages (fixes
 #27822)
Date: Sat, 11 Aug 2018 21:08:33 -0400

tags 27822 fixed
close 27822 26.2
quit

Eli Zaretskii <eliz <at> gnu.org> writes:

> Or maybe
>
>   (defun package--user-installed-p (package)
>     "Return non-nil if PACKAGE is a user-installed package.
>   PACKAGE is the package name, a string."

I went with this phrasing, except PACKAGE is a symbol, not a string.

[1: d2ad4ba4f3]: 2018-08-11 21:06:26 -0400
  Do not consider external packages to be removable (Bug#27822)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=d2ad4ba4f3c5db6f6be7d73c17332e9bc4570e29

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.