* bug#27822: 26.0.50; package-autoremove and 'package-directory-list
@ 2017-07-25 14:46 Yuri D'Elia
2018-07-17 11:16 ` bug#27822: [PATCH] do not auto-remove external packages (fixes #27822) Yuri D'Elia
2018-07-17 11:16 ` Yuri D'Elia
0 siblings, 2 replies; 10+ messages in thread
From: Yuri D'Elia @ 2017-07-25 14:46 UTC (permalink / raw)
To: 27822
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#27822: [PATCH] do not auto-remove external packages (fixes #27822)
2017-07-25 14:46 bug#27822: 26.0.50; package-autoremove and 'package-directory-list Yuri D'Elia
@ 2018-07-17 11:16 ` Yuri D'Elia
2018-07-17 11:16 ` Yuri D'Elia
1 sibling, 0 replies; 10+ messages in thread
From: Yuri D'Elia @ 2018-07-17 11:16 UTC (permalink / raw)
To: emacs-devel, 27822
[-- Attachment #1: Type: text/plain, Size: 589 bytes --]
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]
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Do-not-consider-external-packages-to-be-removable.patch --]
[-- Type: text/x-diff, Size: 1911 bytes --]
From 5a4234b3c1289960fd760256aa6399efbd04bfc3 Mon Sep 17 00:00:00 2001
From: Yuri D'Elia <wavexx@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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] do not auto-remove external packages (fixes #27822)
@ 2018-07-17 11:16 ` Yuri D'Elia
2018-07-17 12:40 ` bug#27822: " Stefan Monnier
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Yuri D'Elia @ 2018-07-17 11:16 UTC (permalink / raw)
To: emacs-devel, 27822
[-- Attachment #1: Type: text/plain, Size: 589 bytes --]
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]
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Do-not-consider-external-packages-to-be-removable.patch --]
[-- Type: text/x-diff, Size: 1911 bytes --]
From 5a4234b3c1289960fd760256aa6399efbd04bfc3 Mon Sep 17 00:00:00 2001
From: Yuri D'Elia <wavexx@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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#27822: [PATCH] do not auto-remove external packages (fixes #27822)
2018-07-17 11:16 ` Yuri D'Elia
@ 2018-07-17 12:40 ` Stefan Monnier
2018-07-17 12:50 ` Yuri D'Elia
2018-08-07 11:12 ` package.el fix (was Re: [PATCH] do not auto-remove external packages (fixes #27822)) Yuri D'Elia
2018-08-09 1:19 ` bug#27822: [PATCH] do not auto-remove external packages (fixes #27822) Noam Postavsky
2 siblings, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2018-07-17 12:40 UTC (permalink / raw)
To: Yuri D'Elia; +Cc: 27822
[ 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#27822: [PATCH] do not auto-remove external packages (fixes #27822)
2018-07-17 12:40 ` bug#27822: " Stefan Monnier
@ 2018-07-17 12:50 ` Yuri D'Elia
0 siblings, 0 replies; 10+ messages in thread
From: Yuri D'Elia @ 2018-07-17 12:50 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 27822
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* package.el fix (was Re: [PATCH] do not auto-remove external packages (fixes #27822))
2018-07-17 11:16 ` Yuri D'Elia
2018-07-17 12:40 ` bug#27822: " Stefan Monnier
@ 2018-08-07 11:12 ` Yuri D'Elia
2018-08-09 1:19 ` bug#27822: [PATCH] do not auto-remove external packages (fixes #27822) Noam Postavsky
2 siblings, 0 replies; 10+ messages in thread
From: Yuri D'Elia @ 2018-08-07 11:12 UTC (permalink / raw)
To: emacs-devel
Can anybody have a look at the proposed fix in bug #27822?
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27822
That's a trivial fix in package.el to improve autoremoval. I've been
using it ever since and I'd like to get it mainlined.
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#27822: [PATCH] do not auto-remove external packages (fixes #27822)
2018-07-17 11:16 ` Yuri D'Elia
2018-07-17 12:40 ` bug#27822: " Stefan Monnier
2018-08-07 11:12 ` package.el fix (was Re: [PATCH] do not auto-remove external packages (fixes #27822)) Yuri D'Elia
@ 2018-08-09 1:19 ` Noam Postavsky
2018-08-09 11:16 ` Yuri D'Elia
2018-08-09 15:24 ` Eli Zaretskii
2 siblings, 2 replies; 10+ messages in thread
From: Noam Postavsky @ 2018-08-09 1:19 UTC (permalink / raw)
To: Yuri D'Elia; +Cc: 27822
Yuri D'Elia <wavexx@thregr.org> writes:
>
>>From 5a4234b3c1289960fd760256aa6399efbd04bfc3 Mon Sep 17 00:00:00 2001
> From: Yuri D'Elia <wavexx@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)))
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#27822: [PATCH] do not auto-remove external packages (fixes #27822)
2018-08-09 1:19 ` bug#27822: [PATCH] do not auto-remove external packages (fixes #27822) Noam Postavsky
@ 2018-08-09 11:16 ` Yuri D'Elia
2018-08-09 15:24 ` Eli Zaretskii
1 sibling, 0 replies; 10+ messages in thread
From: Yuri D'Elia @ 2018-08-09 11:16 UTC (permalink / raw)
To: Noam Postavsky; +Cc: 27822
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#27822: [PATCH] do not auto-remove external packages (fixes #27822)
2018-08-09 1:19 ` bug#27822: [PATCH] do not auto-remove external packages (fixes #27822) Noam Postavsky
2018-08-09 11:16 ` Yuri D'Elia
@ 2018-08-09 15:24 ` Eli Zaretskii
2018-08-12 1:08 ` Noam Postavsky
1 sibling, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2018-08-09 15:24 UTC (permalink / raw)
To: Noam Postavsky; +Cc: wavexx, 27822
> From: Noam Postavsky <npostavs@gmail.com>
> Date: Wed, 08 Aug 2018 21:19:02 -0400
> Cc: 27822@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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#27822: [PATCH] do not auto-remove external packages (fixes #27822)
2018-08-09 15:24 ` Eli Zaretskii
@ 2018-08-12 1:08 ` Noam Postavsky
0 siblings, 0 replies; 10+ messages in thread
From: Noam Postavsky @ 2018-08-12 1:08 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: wavexx, 27822
tags 27822 fixed
close 27822 26.2
quit
Eli Zaretskii <eliz@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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-08-12 1:08 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-25 14:46 bug#27822: 26.0.50; package-autoremove and 'package-directory-list Yuri D'Elia
2018-07-17 11:16 ` bug#27822: [PATCH] do not auto-remove external packages (fixes #27822) Yuri D'Elia
2018-07-17 11:16 ` Yuri D'Elia
2018-07-17 12:40 ` bug#27822: " Stefan Monnier
2018-07-17 12:50 ` Yuri D'Elia
2018-08-07 11:12 ` package.el fix (was Re: [PATCH] do not auto-remove external packages (fixes #27822)) Yuri D'Elia
2018-08-09 1:19 ` bug#27822: [PATCH] do not auto-remove external packages (fixes #27822) Noam Postavsky
2018-08-09 11:16 ` Yuri D'Elia
2018-08-09 15:24 ` Eli Zaretskii
2018-08-12 1:08 ` Noam Postavsky
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.