Importing package-keyring.gpg...done
Waiting for refresh to finish...
user-error: No operations specified
Package refresh done
25 packages marked for upgrading.
Which seems to suggest that package-menu-mark-upgrades is running after package-menu-execute.
Not necessarily. This only suggests that when package-menu-execute is run, there are no operations to perform; in other words, that no packages have been marked by pakage-menu-mark-upgrades or package-menu-mark-obsolete-for-deletion.
This, in turn, is probably because package-menu-async defaults to non-nil, which causes package-list-packages to run asynchronously, which means the function returns before package.el has fetched the latest package data and determined which packages are upgrade/obsoletion candidates. This is confirmed by the message "Waiting for refresh to finish...", which is emitted by package-menu-mark-upgrades when a download is still in progress.
Setting/binding package-menu-async to nil before calling package-list-packages should result in the upgrades and deletions happening as expected, but the retrieval and subsequent actions will be synchronous and thus obstruct you from otherwise interacting with Emacs for their duration.
Either way, you should wrap the call to package-menu-execute in something like condition-case to handle the error it emits when there are no packages to be upgraded or deleted. Note also that save-window-excursion, as its docstring describes, is an unreliable protection against buffer/window/frame configuration changes. Even if it were reliable, package-menu-execute would and will still pop up the *Packages* buffer for the duration of any synchronous package upgrades. Hence the complexity of the answer you link to.
Here is how you could modify your function to work synchronously, as described above:
(defun my-package-upgrade ()
"Refresh, upgrade and delete obsolete packages synchronously."
(interactive)
(save-window-excursion
(let (package-menu-async)
(package-list-packages)))
(with-current-buffer "*Packages*"
(package-menu-mark-upgrades)
(package-menu-mark-obsolete-for-deletion)
(condition-case err
(package-menu-execute t)
;; Don't barf if there is nothing to do
(user-error (message "Nothing to do"))
;; But allow other errors through
(error (signal (car err) (cdr err))))))
Here is a bad example of how you could write your function to keep package refreshes asynchronous whilst performing any marking/upgrades at the right time. As such, this is a compromise between setting package-menu-async to nil and using the code from the linked answer.
(defun my-package-upgrade ()
"Refresh, upgrade and delete obsolete packages."
(interactive)
(letrec ((hook
(lambda ()
(unwind-protect
(with-current-buffer "*Packages*"
(package-menu-mark-upgrades)
(package-menu-mark-obsolete-for-deletion)
(condition-case err
(package-menu-execute t)
(user-error (message "Nothing to do"))
(error (signal (car err) (cdr err)))))
(remove-hook 'package--post-download-archives-hook hook)))))
(save-window-excursion
(package-list-packages)
(add-hook 'package--post-download-archives-hook hook t))))
What makes this a kludge is:
- The (unlikely) race condition between the calls to
package-list-packages and add-hook in an attempt to run hook after the other functions on this hook variable.
- The fact that
package.el isn't designed to be used/scripted like this. Even replacing hooks with well-placed function advice would be a bit of a kludge.
YMMV, of course.
