GNU bug report logs - #74556
30.0.92; Package upgrade can fail and results in deleted package

Previous Next

Full log

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

From: Philip Kaludercic <philipk <at> posteo.net>
To: Daniel Mendler <mail <at> daniel-mendler.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 74556 <at> debbugs.gnu.org
Subject: Re: bug#74556: 30.0.92; Package upgrade can fail and results in
 deleted package
Date: Tue, 10 Dec 2024 20:12:15 +0000

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

Daniel Mendler <mail <at> daniel-mendler.de> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>>>> Cc: 74556 <at> debbugs.gnu.org
>>>> From: Philip Kaludercic <philipk <at> posteo.net>
>>>> Date: Thu, 28 Nov 2024 11:34:51 +0000
>>>> 
>>>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>>> 
>>>> [...]
>>>> 
>>>> > It might make sense to try and "deactivate" a package before installing
>>>> > the new package.  Looking into some second-try fallback for
>>>> > package-install to refresh the package index if a package was not found
>>>> > would also be a good idea ^^
>>>> 
>>>> This might do it?
>>>
>>> Philip, please install this on the emacs-30 branch, unless you see any
>>> problems with the change.  We'd like to make another pretest soon.
>>
>> Done.  But we should keep the report open as there might be better
>> approaches to discuss in the future.
>
> Hello Philip,
>
> I just tried the modified `package-upgrade' function and it doesn't seem
> to work. It seems to break the upgrade procedure in an even worse way,
> at least in my setup. Now `package-install' is tried first with the
> package symbol, which will be a no-op, since the package is already
> installed. Afterwards the package is deleted and we always end up with
> no package. Probably `package-install' should also be called with a
> package descriptor of the new package version?

Right, my sincere apologies for that oversight.  That being said, I
don't feel comfortable fixing this right now as I am short on time to
fix and test something like this on the "emacs-30" branch.  My vote
would be to revert the commit and try to tackle the issue on the
"master" branch.  An alternative I can propose that would be closer to
the original code might be

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

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index fad0762e126..93b647edcea 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -2268,16 +2285,14 @@ package-upgrade
     ;; `pkg-desc' will be nil when the package is an "active built-in".
     (if (and pkg-desc (package-vc-p pkg-desc))
         (package-vc-upgrade pkg-desc)
-      (package-install package
-                       ;; An active built-in has never been "selected"
-                       ;; before.  Mark it as installed explicitly.
-                       (and pkg-desc 'dont-select))
-      ;; We delete the old package via the descriptor after installing
-      ;; the new package to avoid losing the package if there issues
-      ;; during installation (Bug#74556).
-      (when pkg-desc
-        (cl-assert (package-desc-dir pkg-desc))
-        (package-delete pkg-desc 'force 'dont-unselect)))))
+      (unwind-protect
+          (when pkg-desc
+            (cl-assert (package-desc-dir pkg-desc))
+            (package-delete pkg-desc 'force 'dont-unselect))
+        (package-install package
+                         ;; An active built-in has never been "selected"
+                         ;; before.  Mark it as installed explicitly.
+                         (and pkg-desc 'dont-select))))))
 
 (defun package--upgradeable-packages (&optional include-builtins)
   ;; Initialize the package system to get the list of package

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

> Thanks!
>
> Daniel

Previous Next

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