Package: emacs;
Reported by: Daniel Mendler <mail <at> daniel-mendler.de>
Date: Fri, 29 Nov 2024 15:40:02 UTC
Severity: wishlist
Found in version 30.0.92
View this message in rfc822 format
From: Nobuyuki Kamimoto <kamimoto527 <at> gmail.com> To: 74604 <at> debbugs.gnu.org Subject: bug#74604: [PATCH v1] package.el: Add diff display and confirmation for package upgrades (Bug#74604) Date: Sun, 31 Aug 2025 12:20:39 +0900
Hello,
This patch implements a new feature for package upgrades, as discussed
in Bug#74604.
Summary:
- Shows diffs between the installed and the new version of a package
before upgrading.
- Lets the user review changes and confirm whether to proceed.
- Supports both regular tarball packages and VC packages.
- New user options:
- package-show-upgrade-diffs
- package-upgrade-diff-exclude-packages
- package-upgrade-diff-exclude-archives
- package-vc-show-upgrade-diffs
- package-vc-upgrade-diff-exclude-packages
- Updated documentation (doc/emacs/package.texi).
- Added entry to etc/NEWS.
Testing:
- Built Emacs from the patched branch.
- Verified interactive upgrade of both tarball and VC packages.
- Ran `make check` and confirmed tests pass.
See Bug#74604 for context.
Best regards,
Nobuyuki Kamimoto
---8<---cut here---8<---
From f1793d5c62b194c37a61d8d4d02b655dc8b714c3 Mon Sep 17 00:00:00 2001
From: Nobuyuki Kamimoto <kamimoto527 <at> gmail.com>
Date: Sun, 31 Aug 2025 12:08:50 +0900
Subject: [PATCH] Add diff display and confirmation for package upgrades
Show differences between current and new versions before upgrading
packages, allowing users to review changes and choose whether to
proceed. This applies to both regular packages and VC packages.
* lisp/emacs-lisp/package.el (package-show-upgrade-diffs)
(package-upgrade-diff-exclude-packages)
(package-upgrade-diff-exclude-archives): New user options to control
diff display for package upgrades.
(package--extract-tarball-to-temp, package--get-installed-package-dir)
(package--show-package-diff, package--confirm-tarball-upgrade): New
functions to handle diff display and confirmation for tarball upgrades.
(package-upgrade): Check if tarball package upgrade needs confirmation.
(package-menu--perform-transaction): Handle diff confirmation in menu.
* lisp/emacs-lisp/package-vc.el (package-vc-show-upgrade-diffs)
(package-vc-upgrade-diff-exclude-packages): New user options to control
diff display for VC package upgrades.
(package-vc--show-diff, package-vc--confirm-upgrade): New functions to
handle diff display and confirmation for VC package upgrades.
(package-vc-upgrade): Integrate diff confirmation into upgrade process.
* doc/emacs/package.texi (Package Installation): Document the new diff
display functionality for both regular and VC package upgrades,
including the new user options for controlling the behavior.
* etc/NEWS: Add announcement for the new package upgrade diff feature.
---
doc/emacs/package.texi | 37 ++++++++
etc/NEWS | 16 ++++
lisp/emacs-lisp/package-vc.el | 140 ++++++++++++++++++++++-------
lisp/emacs-lisp/package.el | 165 ++++++++++++++++++++++++++++++----
4 files changed, 308 insertions(+), 50 deletions(-)
diff --git a/doc/emacs/package.texi b/doc/emacs/package.texi
index c29beea3b08..1f57910a207 100644
--- a/doc/emacs/package.texi
+++ b/doc/emacs/package.texi
@@ -389,6 +389,25 @@ Package Installation
use these bulk commands if you want to update only a small number of
built-in packages.
+@vindex package-show-upgrade-diffs
+@vindex package-upgrade-diff-exclude-packages
+@vindex package-upgrade-diff-exclude-archives
+ By default, Emacs shows a diff of the changes when upgrading
+packages, allowing you to review what has changed between the current
+and new version before proceeding. This is controlled by the
+@code{package-show-upgrade-diffs} variable. When non-@code{nil} (the
+default), package upgrades will display the differences and ask for
+confirmation before proceeding. You can disable this behavior by
+setting @code{package-show-upgrade-diffs} to @code{nil}.
+
+ You can exclude specific packages or package archives from diff
+checking using @code{package-upgrade-diff-exclude-packages} and
+@code{package-upgrade-diff-exclude-archives}. The first variable
+takes a list of package names (as symbols), while the second takes a
+list of archive names (as strings). Packages or archives in these
+lists will upgrade without showing diffs, even when
+@code{package-show-upgrade-diffs} is non-@code{nil}.
+
@cindex package requirements
A package may @dfn{require} certain other packages to be installed,
because it relies on functionality provided by them. When Emacs
@@ -645,6 +664,24 @@ Fetching Package Sources
Note that currently, built-in packages cannot be upgraded using
@code{package-vc-install}.
+@vindex package-vc-show-upgrade-diffs
+@vindex package-vc-upgrade-diff-exclude-packages
+ Like regular package upgrades, VC package upgrades can also show
+diffs before proceeding. This is controlled by the
+@code{package-vc-show-upgrade-diffs} variable, which operates
+independently of @code{package-show-upgrade-diffs}. When
+non-@code{nil} (the default), upgrading VC packages will display the
+differences between the current and updated versions, allowing you to
+review the changes before confirming the upgrade.
+
+ You can exclude specific VC packages from diff checking by adding
+their names (as symbols) to
@code{package-vc-upgrade-diff-exclude-packages}.
+Packages in this list will upgrade without showing diffs, even when
+@code{package-vc-show-upgrade-diffs} is non-@code{nil}. This
+exclusion list is separate from
@code{package-upgrade-diff-exclude-packages}
+to allow independent control over diff behavior for VC packages versus
+regular packages.
+
@findex package-report-bug
@findex package-vc-prepare-patch
With the source checkout, you might want to reproduce a bug against
diff --git a/etc/NEWS b/etc/NEWS
index 8a139cb03ca..2fdbd1c291f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -74,6 +74,22 @@ done from early-init.el, such as adding to
'package-directory-list'.
** 'prettify-symbols-mode' attempts to ignore undisplayable characters.
Previously, such characters would be rendered as, e.g., white boxes.
++++
+** Package management now shows diffs before upgrades.
+Package upgrades can now display differences between the current and
+new version before proceeding. This applies to both regular packages
+and VC packages installed from version control repositories.
+
+The behavior is controlled by these new user options:
+- 'package-show-upgrade-diffs': Enable/disable diff display for regular
packages
+- 'package-vc-show-upgrade-diffs': Enable/disable diff display for VC
packages
+- 'package-upgrade-diff-exclude-packages': Exclude specific packages
from diff checking
+- 'package-upgrade-diff-exclude-archives': Exclude specific archives
from diff checking
+- 'package-vc-upgrade-diff-exclude-packages': Exclude specific VC
packages from diff checking
+
+When enabled (the default), users will see a diff buffer showing changes
+and can choose whether to proceed with or cancel the upgrade.
+
+++
** 'standard-display-table' now has more extra slots.
'standard-display-table' has been extended to allow specifying glyphs
diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
index 7433fce2d89..da9977b429f 100644
--- a/lisp/emacs-lisp/package-vc.el
+++ b/lisp/emacs-lisp/package-vc.el
@@ -83,6 +83,27 @@ package-vc-register-as-project
:type 'boolean
:version "30.1")
+(defcustom package-vc-show-upgrade-diffs t
+ "Non-nil means to show diffs before upgrading VC packages.
+This controls whether VC package upgrades show a diff buffer
+comparing the current and updated versions of the package before
+proceeding with the upgrade. This is independent of the
+`package-show-upgrade-diffs' variable which controls diff
+display for regular package upgrades."
+ :type 'boolean
+ :group 'package-vc
+ :version "31.1")
+
+(defcustom package-vc-upgrade-diff-exclude-packages nil
+ "List of VC package names to exclude from upgrade diff checking.
+This list contains package names (as symbols) for which diff checking
+should be skipped when upgrading VC packages. This is separate from
+`package-upgrade-diff-exclude-packages' to allow independent control
+of diff exclusions for VC packages versus regular package upgrades."
+ :type '(repeat symbol)
+ :group 'package-vc
+ :version "31.1")
+
(defvar package-vc-selected-packages) ; pacify byte-compiler
;;;###autoload
@@ -729,6 +750,55 @@ package-vc-upgrade-all
(declare-function vc-dir-prepare-status-buffer "vc-dir"
(bname dir backend &optional create-new))
+(defun package-vc--show-diff (old-dir new-dir)
+ "Show diff between OLD-DIR and NEW-DIR package directories.
+Return t if user confirms to continue, nil to cancel."
+ (let ((diff-buffer "*Package VC Diff*"))
+ (with-current-buffer (get-buffer-create diff-buffer)
+ (erase-buffer)
+ (let ((inhibit-read-only t)
+ (exit-status (call-process "diff" nil t nil "-ru" old-dir
new-dir)))
+ (cond
+ ((eq exit-status 0)
+ (insert "No differences found between current and updated
package versions.\n"))
+ ((eq exit-status 1)
+ ;; Normal case: differences found, they're already in buffer
+ nil)
+ (t
+ ;; Error case
+ (insert (format "Error running diff (exit status %d).\n"
exit-status)))))
+ (diff-mode)
+ (read-only-mode 1)
+ (goto-char (point-min)))
+ (display-buffer diff-buffer)
+ (let ((response (yes-or-no-p "Show differences. Continue with
upgrade? ")))
+ (kill-buffer diff-buffer)
+ response)))
+
+(defun package-vc--confirm-upgrade (pkg-desc)
+ "Show diff for VC package upgrade and ask for confirmation.
+Return t if user wants to proceed, nil otherwise."
+ ;; Check exclusion list first
+ (let ((package-name (package-desc-name pkg-desc)))
+ ;; If package-vc-show-upgrade-diffs is nil, or package is excluded,
proceed without diffs
+ (if (or (not package-vc-show-upgrade-diffs)
+ (memq package-name package-vc-upgrade-diff-exclude-packages))
+ t ; Skip diff checking, proceed with upgrade
+ (let* ((pkg-dir (package-desc-dir pkg-desc))
+ (temp-dir (make-temp-file "package-vc-diff-" t))
+ (pkg-spec (package-vc--desc->spec pkg-desc)))
+ ;; Delete temp directory so package-vc--clone can create and
populate it
+ (delete-directory temp-dir)
+ (unwind-protect
+ (progn
+ ;; Clone the updated version to temp directory
+ (package-vc--clone pkg-desc pkg-spec temp-dir nil)
+ ;; Show diff and get user confirmation
+ (package-vc--show-diff pkg-dir temp-dir))
+ ;; Clean up temp directory
+ (when (file-exists-p temp-dir)
+ (delete-directory temp-dir t)))))))
+
;;;###autoload
(defun package-vc-upgrade (pkg-desc)
"Upgrade the package described by PKG-DESC from package's VC repository.
@@ -736,40 +806,42 @@ package-vc-upgrade
This may fail if the local VCS state of the package conflicts
with the remote repository state."
(interactive (list (package-vc--read-package-desc "Upgrade VC
package: " t)))
- ;; HACK: To run `package-vc--unpack-1' after checking out the new
- ;; revision, we insert a hook into `vc-post-command-functions', and
- ;; remove it right after it ran. To avoid running the hook multiple
- ;; times or even for the wrong repository (as `vc-pull' is often
- ;; asynchronous), we extract the relevant arguments using a pseudo
- ;; filter for `vc-filter-command-function', executed only for the
- ;; side effect, and store them in the lexical scope. When the hook
- ;; is run, we check if the arguments are the same (`eq') as the ones
- ;; previously extracted, and only in that case will be call
- ;; `package-vc--unpack-1'. Ugh...
- ;;
- ;; If there is a better way to do this, it should be done.
- (cl-assert (package-vc-p pkg-desc))
- (letrec ((pkg-dir (package-desc-dir pkg-desc))
- (vc-flags)
- (vc-filter-command-function
- (lambda (command file-or-list flags)
- (setq vc-flags flags)
- (list command file-or-list flags)))
- (post-upgrade
- (lambda (_command _file-or-list flags)
- (when (and (file-equal-p pkg-dir default-directory)
- (eq flags vc-flags))
- (unwind-protect
- (with-demoted-errors "Failed to activate: %S"
- (package-vc--unpack-1 pkg-desc pkg-dir))
- (remove-hook 'vc-post-command-functions
post-upgrade))))))
- (add-hook 'vc-post-command-functions post-upgrade)
- (with-demoted-errors "Failed to fetch: %S"
- (require 'vc-dir)
- (with-current-buffer (vc-dir-prepare-status-buffer
- (format " *package-vc-dir: %s*" pkg-dir)
- pkg-dir (vc-responsible-backend pkg-dir))
- (vc-pull)))))
+ ;; Check if user wants to see diff and confirm upgrade
+ (when (package-vc--confirm-upgrade pkg-desc)
+ ;; HACK: To run `package-vc--unpack-1' after checking out the new
+ ;; revision, we insert a hook into `vc-post-command-functions', and
+ ;; remove it right after it ran. To avoid running the hook multiple
+ ;; times or even for the wrong repository (as `vc-pull' is often
+ ;; asynchronous), we extract the relevant arguments using a pseudo
+ ;; filter for `vc-filter-command-function', executed only for the
+ ;; side effect, and store them in the lexical scope. When the hook
+ ;; is run, we check if the arguments are the same (`eq') as the ones
+ ;; previously extracted, and only in that case will be call
+ ;; `package-vc--unpack-1'. Ugh...
+ ;;
+ ;; If there is a better way to do this, it should be done.
+ (cl-assert (package-vc-p pkg-desc))
+ (letrec ((pkg-dir (package-desc-dir pkg-desc))
+ (vc-flags)
+ (vc-filter-command-function
+ (lambda (command file-or-list flags)
+ (setq vc-flags flags)
+ (list command file-or-list flags)))
+ (post-upgrade
+ (lambda (_command _file-or-list flags)
+ (when (and (file-equal-p pkg-dir default-directory)
+ (eq flags vc-flags))
+ (unwind-protect
+ (with-demoted-errors "Failed to activate: %S"
+ (package-vc--unpack-1 pkg-desc pkg-dir))
+ (remove-hook 'vc-post-command-functions
post-upgrade))))))
+ (add-hook 'vc-post-command-functions post-upgrade)
+ (with-demoted-errors "Failed to fetch: %S"
+ (require 'vc-dir)
+ (with-current-buffer (vc-dir-prepare-status-buffer
+ (format " *package-vc-dir: %s*" pkg-dir)
+ pkg-dir (vc-responsible-backend pkg-dir))
+ (vc-pull))))))
(defun package-vc--archives-initialize ()
"Initialize package.el and fetch package specifications."
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ba9999c20e6..1d1266b7ede 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -450,6 +450,44 @@ package-archive-column-width
:type 'natnum
:version "28.1")
+(defcustom package-show-upgrade-diffs t
+ "Whether to show diffs before upgrading packages.
+When non-nil, package upgrades will display the differences between
+the current and new version before proceeding. Users can review the
+changes and choose whether to continue with the upgrade.
+When nil, packages upgrade without showing diffs."
+ :type 'boolean
+ :group 'package
+ :version "31.1")
+
+(defcustom package-upgrade-diff-exclude-packages nil
+ "List of package names to exclude from diff checking during upgrades.
+When a package name is in this list, the upgrade will proceed
+without showing diffs, even if `package-show-upgrade-diffs' is non-nil.
+Package names should be specified as symbols.
+
+For example: \\='(emacs magit org-mode)
+
+This allows for fine-grained control over which packages show
+diffs during upgrades."
+ :type '(repeat symbol)
+ :group 'package
+ :version "31.1")
+
+(defcustom package-upgrade-diff-exclude-archives nil
+ "List of package archives to exclude from diff checking during upgrades.
+When a package's archive is in this list, the upgrade will proceed
+without showing diffs, even if `package-show-upgrade-diffs' is non-nil.
+Archive names should be specified as strings.
+
+For example: \\='(\"melpa-stable\" \"nongnu\")
+
+This allows for fine-grained control over which archives show
+diffs during package upgrades."
+ :type '(repeat string)
+ :group 'package
+ :version "31.1")
+
;;; `package-desc' object definition
;; This is the struct used internally to represent packages.
@@ -1019,6 +1057,77 @@ package--alist-to-plist-args
(declare-function dired-get-marked-files "dired")
+(defun package--extract-tarball-to-temp (buffer pkg-desc temp-dir)
+ "Extract tarball from BUFFER to TEMP-DIR for PKG-DESC.
+Returns the directory where package was extracted."
+ (let ((pkg-dir (expand-file-name (package-desc-full-name pkg-desc)
temp-dir)))
+ (make-directory temp-dir t)
+ (with-current-buffer buffer
+ (let ((default-directory temp-dir))
+ (package-untar-buffer (package-desc-full-name pkg-desc))))
+ pkg-dir))
+
+(defun package--get-installed-package-dir (pkg-desc)
+ "Get directory of installed PKG-DESC package."
+ (expand-file-name (package-desc-full-name pkg-desc) package-user-dir))
+
+(defun package--show-package-diff (old-dir new-dir)
+ "Show diff between OLD-DIR and NEW-DIR package directories.
+Return t if user confirms to continue, nil to cancel."
+ (let ((diff-buffer "*Package Diff*"))
+ (with-current-buffer (get-buffer-create diff-buffer)
+ (erase-buffer)
+ (let ((inhibit-read-only t)
+ (exit-status (call-process "diff" nil t nil "-ru" old-dir
new-dir)))
+ (cond
+ ((eq exit-status 0)
+ (insert "No differences found between old and new package
versions.\n"))
+ ((eq exit-status 1)
+ ;; Normal case: differences found, they're already in buffer
+ nil)
+ (t
+ ;; Error case
+ (insert (format "Error running diff (exit status %d).\n"
exit-status)))))
+ (diff-mode)
+ (read-only-mode 1)
+ (goto-char (point-min)))
+ (display-buffer diff-buffer)
+ (let ((response (yes-or-no-p "Show differences. Continue with
upgrade? ")))
+ (kill-buffer diff-buffer)
+ response)))
+
+(defun package--confirm-tarball-upgrade (pkg-desc new-pkg-desc)
+ "Show diff for tarball package upgrade and ask for confirmation.
+Return t if user wants to proceed, nil otherwise."
+ ;; Check exclusion lists first
+ (let ((package-name (package-desc-name new-pkg-desc))
+ (package-archive (package-desc-archive new-pkg-desc)))
+ ;; If package-show-upgrade-diffs is nil, or package/archive is
excluded, proceed without diffs
+ (if (or (not package-show-upgrade-diffs)
+ (memq package-name package-upgrade-diff-exclude-packages)
+ (member package-archive package-upgrade-diff-exclude-archives))
+ t ; Skip diff checking, proceed with upgrade
+ (let* ((temp-dir (make-temp-file "package-diff-" t))
+ (old-dir (package--get-installed-package-dir pkg-desc))
+ new-dir)
+ (unwind-protect
+ (progn
+ ;; Download and extract new version to temp directory
+ (let* ((location (package-archive-base new-pkg-desc))
+ (file (concat (package-desc-full-name new-pkg-desc)
+ (package-desc-suffix new-pkg-desc))))
+ (package--with-response-buffer location :file file
+ (setq new-dir (package--extract-tarball-to-temp
+ (current-buffer) new-pkg-desc temp-dir))))
+ ;; Show diff and get user confirmation
+ (if (file-exists-p old-dir)
+ (package--show-package-diff old-dir new-dir)
+ ;; If no old version exists, just confirm
+ (yes-or-no-p "New package installation. Continue? ")))
+ ;; Clean up temp directory
+ (when (file-exists-p temp-dir)
+ (delete-directory temp-dir t)))))))
+
(defun package-unpack (pkg-desc)
"Install the contents of the current buffer as a package."
(let* ((name (package-desc-name pkg-desc))
@@ -2290,16 +2399,31 @@ package-upgrade
(package--upgradeable-packages t) nil t))))
(cl-check-type name symbol)
(let* ((pkg-desc (cadr (assq name package-alist)))
- (package-install-upgrade-built-in (not pkg-desc)))
+ (package-install-upgrade-built-in (not pkg-desc))
+ (new-pkg-desc (cadr (assq name package-archive-contents))))
;; `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)
- (when pkg-desc
- (package-delete pkg-desc 'force 'dont-unselect))
- (package-install name
- ;; An active built-in has never been "selected"
- ;; before. Mark it as installed explicitly.
- (and pkg-desc 'dont-select)))))
+ ;; Check if this is a tarball package upgrade that needs diff
confirmation
+ (if (and pkg-desc new-pkg-desc
+ (eq (package-desc-kind new-pkg-desc) 'tar))
+ ;; For tarball packages, show diff and ask for confirmation
+ (if (package--confirm-tarball-upgrade pkg-desc new-pkg-desc)
+ (progn
+ (package-delete pkg-desc 'force 'dont-unselect)
+ (package-install name
+ ;; An active built-in has never been
"selected"
+ ;; before. Mark it as installed
explicitly.
+ (and pkg-desc 'dont-select)))
+ (message "Package upgrade cancelled"))
+ ;; For non-tarball packages, proceed with normal upgrade
+ (progn
+ (when pkg-desc
+ (package-delete pkg-desc 'force 'dont-unselect))
+ (package-install name
+ ;; 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
@@ -2688,16 +2812,16 @@ package-isolate
in a clean environment."
(interactive
(cl-loop for p in (cl-loop for p in (package--alist) append (cdr p))
- unless (package-built-in-p p)
- collect (cons (package-desc-full-name p) p) into table
- finally return
- (list
+ unless (package-built-in-p p)
+ collect (cons (package-desc-full-name p) p) into table
+ finally return
+ (list
(cl-loop for c in
(completing-read-multiple
"Packages to isolate: " table
nil t)
- collect (alist-get c table nil nil #'string=))
- current-prefix-arg)))
+ collect (alist-get c table nil nil #'string=))
+ current-prefix-arg)))
(let* ((name (concat "package-isolate-"
(mapconcat #'package-desc-full-name packages ",")))
(all-packages (delete-consecutive-dups
@@ -4172,9 +4296,18 @@ package-menu--perform-transaction
(format status-format (incf i)))
(force-mode-line-update)
(redisplay 'force)
- ;; Don't mark as selected, `package-menu-execute' already
- ;; does that.
- (package-install pkg 'dont-select))))
+ ;; Check if this is a tarball package upgrade and needs
confirmation
+ (let* ((pkg-name (package-desc-name pkg))
+ (old-pkg (cl-find-if (lambda (del-pkg)
+ (eq (package-desc-name
del-pkg) pkg-name))
+ delete-list)))
+ (if (and old-pkg (eq (package-desc-kind pkg) 'tar))
+ ;; This is a tarball upgrade - ask for confirmation
+ (if (package--confirm-tarball-upgrade old-pkg pkg)
+ (package-install pkg 'dont-select)
+ (push (package-desc-full-name pkg) errors))
+ ;; Normal installation
+ (package-install pkg 'dont-select))))))
(let ((package-menu--transaction-status ":Deleting"))
(force-mode-line-update)
(redisplay 'force)
--
2.43.0
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.