From ee262e7737b08a1c7c0037af009c125d23a8464c Mon Sep 17 00:00:00 2001 From: Henrik Lissner Date: Thu, 7 Jun 2018 17:59:23 +0200 Subject: [PATCH] Refactor error handling in package management API I am hoping this will improve the ambiguous errors that originate from package.el or quelpa.el. --- core/autoload/packages.el | 50 ++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/core/autoload/packages.el b/core/autoload/packages.el index a35641f3d..c890a9cec 100644 --- a/core/autoload/packages.el +++ b/core/autoload/packages.el @@ -16,18 +16,14 @@ (defmacro doom--condition-case! (&rest body) `(condition-case-unless-debug ex - (condition-case ex2 - (progn ,@body) - ('file-error - (print! (bold (red " FILE ERROR: %s" (error-message-string ex2)))) - (print! " Trying again...") - (quiet! (doom-refresh-packages-maybe t)) - ,@body)) + (progn ,@body) ('user-error - (print! (bold (red " ERROR: %s" ex)))) - ('error - (doom--refresh-pkg-cache) - (print! (bold (red " FATAL ERROR: %s" ex)))))) + (print! (bold (red " NOTICE: %s" ex)))) + ('file-error + (print! (bold (red " FILE ERROR: %s" (error-message-string ex2)))) + (print! " Trying again...") + (quiet! (doom-refresh-packages-maybe t)) + ,@body))) (defun doom--refresh-pkg-cache () "Clear the cache for `doom-refresh-packages-maybe'." @@ -60,7 +56,7 @@ (defun doom-package-backend (name &optional noerror) "Get which backend the package NAME was installed with. Can either be elpa or quelpa. Throws an error if NOERROR is nil and the package isn't installed." - (cl-assert (symbolp name) t) + (cl-check-type name symbol) (cond ((assq name quelpa-cache) 'quelpa) ((assq name package-alist) @@ -75,7 +71,7 @@ quelpa. Throws an error if NOERROR is nil and the package isn't installed." "Determine whether NAME (a symbol) is outdated or not. If outdated, returns a list, whose car is NAME, and cdr the current version list and latest version list of the package." - (cl-assert (symbolp name) t) + (cl-check-type name symbol) (when-let* ((desc (cadr (assq name package-alist)))) (let* ((old-version (package-desc-version desc)) (new-version @@ -100,15 +96,16 @@ list of the package." ;;;###autoload (defun doom-package-prop (name prop) "Return PROPerty in NAME's plist." - (cl-assert (symbolp name) t) - (cl-assert (keywordp prop) t) + (cl-check-type name symbol) + (cl-check-type prop keyword) (plist-get (cdr (assq name doom-packages)) prop)) ;;;###autoload (defun doom-package-different-backend-p (name) "Return t if a package named NAME (a symbol) has a new backend than what it was installed with. Returns nil otherwise, or if package isn't installed." - (cl-assert (symbolp name) t) + (cl-check-type name symbol) + (doom-initialize-packages) (and (package-installed-p name) (let* ((plist (cdr (assq name doom-packages))) (old-backend (doom-package-backend name 'noerror)) @@ -119,7 +116,7 @@ was installed with. Returns nil otherwise, or if package isn't installed." (defun doom-package-different-recipe-p (name) "Return t if a package named NAME (a symbol) has a different recipe than it was installed with." - (cl-assert (symbolp name) t) + (cl-check-type name symbol) (when (package-installed-p name) (let ((quelpa-recipe (assq name quelpa-cache)) (doom-recipe (assq name doom-packages))) @@ -153,14 +150,17 @@ If INSTALLED-ONLY-P, only return packages that are installed." ;;;###autoload (defun doom-get-depending-on (name) "Return a list of packages that depend on the package named NAME." + (cl-check-type name symbol) (when (package-built-in-p name) (error "Can't get the dependency tree for built-in packages")) - (when-let* ((desc (cadr (assq name package-alist)))) - (mapcar #'package-desc-name (package--used-elsewhere-p desc nil t)))) + (if-let* ((desc (cadr (assq name package-alist)))) + (mapcar #'package-desc-name (package--used-elsewhere-p desc nil t)) + (error "Couldn't find %s, is it installed?" name))) ;;;###autoload (defun doom-get-dependencies-for (name &optional only) "Return a list of dependencies for a package." + (cl-check-type name symbol) (when (package-built-in-p name) (error "Can't get the dependency tree for built-in packages")) (package--get-deps name only)) @@ -262,6 +262,7 @@ Used by `doom//packages-install'." (defun doom-install-package (name &optional plist) "Installs package NAME with optional quelpa RECIPE (see `quelpa-recipe' for an example; the package name can be omitted)." + (cl-check-type name symbol) (doom-initialize-packages) (when (and (package-installed-p name) (not (package-built-in-p name))) @@ -286,6 +287,8 @@ example; the package name can be omitted)." (defun doom-update-package (name &optional force-p) "Updates package NAME (a symbol) if it is out of date, using quelpa or package.el as appropriate." + (cl-check-type name symbol) + (doom-initialize-packages) (unless (package-installed-p name) (user-error "%s isn't installed" name)) (when (doom-package-different-backend-p name) @@ -294,10 +297,10 @@ package.el as appropriate." (let ((inhibit-message (not doom-debug-mode)) (desc (cadr (assq name package-alist)))) (pcase (doom-package-backend name) - ('quelpa + (`quelpa (let ((quelpa-upgrade-p t)) (quelpa (assq name quelpa-cache)))) - ('elpa + (`elpa (let* ((archive (cadr (assq name package-archive-contents))) (packages (if (package-desc-p archive) @@ -313,6 +316,8 @@ package.el as appropriate." ;;;###autoload (defun doom-delete-package (name &optional force-p) "Uninstalls package NAME if it exists, and clears it from `quelpa-cache'." + (cl-check-type name symbol) + (doom-initialize-packages) (unless (package-installed-p name) (user-error "%s isn't installed" name)) (let ((inhibit-message (not doom-debug-mode)) @@ -322,7 +327,8 @@ package.el as appropriate." (quelpa-save-cache) (setq quelpa-p t)) (package-delete (cadr (assq name package-alist)) force-p) - (unless (package-installed-p name) + (when (or (not (package-installed-p name)) + (package-built-in-p name)) (let ((pkg-build-dir (expand-file-name (symbol-name name) quelpa-build-dir))) (when (and quelpa-p (file-directory-p pkg-build-dir)) (delete-directory pkg-build-dir t)))