From 8f73fc00906c5c179a7ea92c5d71483af4f8cb86 Mon Sep 17 00:00:00 2001 From: "Ryan C. Thompson" Date: Fri, 9 Mar 2012 18:48:50 -0800 Subject: [PATCH 1/2] Prevent package names from being passed internally as strings. Assertions have been added to many functions that take packages or lists of packages to enforce this. by itself, this breaks el-get horribly, because other functions must also be modified not to pass strings when they call these functions. --- el-get-dependencies.el | 1 + el-get-list-packages.el | 1 + el-get-recipes.el | 8 ++++---- el-get-status.el | 7 +++++++ el-get.el | 5 +++++ methods/el-get-elpa.el | 2 ++ 6 files changed, 20 insertions(+), 4 deletions(-) diff --git a/el-get-dependencies.el b/el-get-dependencies.el index 50a5a22df..e2c6ab719 100644 --- a/el-get-dependencies.el +++ b/el-get-dependencies.el @@ -26,6 +26,7 @@ (defun el-get-dependencies-graph (package) "Return the graph of packages on which PACKAGE depends" + (assert (symbolp package)) (let* ((source (el-get-package-def (symbol-name package))) (method (el-get-package-method source)) (pdeps (el-get-as-list (plist-get source :depends))) diff --git a/el-get-list-packages.el b/el-get-list-packages.el index c9a0d2bc2..4d0e45c13 100644 --- a/el-get-list-packages.el +++ b/el-get-list-packages.el @@ -76,6 +76,7 @@ matching REGEX with TYPE and ARGS as parameter." (defun el-get-describe-1 (package) (let* ((psym (el-get-as-symbol package)) (pname (symbol-name psym)) + (assert (symbolp package)) (status (el-get-read-package-status package)) (def (el-get-package-def pname)) (name (plist-get def :name)) diff --git a/el-get-recipes.el b/el-get-recipes.el index 3c024e378..95151788f 100644 --- a/el-get-recipes.el +++ b/el-get-recipes.el @@ -172,13 +172,13 @@ which defaults to installed, required and removed. Example: (el-get-package-types-alist \"installed\" 'http 'cvs) " - (loop for src in (apply 'el-get-list-package-names-with-status + (loop for pkg in (apply 'el-get-list-package-names-with-status (cond ((stringp statuses) (list statuses)) ((null statuses) '("installed" "required" "removed")) (t statuses))) - for name = (el-get-as-symbol src) - for type = (el-get-package-type name) + do (assert (symbolp pkg)) + for type = (el-get-package-type pkg) when (or (null types) (memq 'all types) (memq type types)) - collect (cons name type))) + collect (cons pkg type))) (provide 'el-get-recipes) diff --git a/el-get-status.el b/el-get-status.el index 6a1fcac89..5aea521dd 100644 --- a/el-get-status.el +++ b/el-get-status.el @@ -96,6 +96,7 @@ (defun el-get-read-package-status (package &optional package-status-alist) "return current status for PACKAGE" + (assert (symbolp package)) (let ((p-alist (or package-status-alist (el-get-read-status-file)))) (plist-get (cdr (assq (el-get-as-symbol package) p-alist)) 'status))) @@ -103,12 +104,14 @@ (defun el-get-read-package-status-recipe (package &optional package-status-alist) "return current status for PACKAGE" + (assert (symbolp package)) (let ((p-alist (or package-status-alist (el-get-read-status-file)))) (plist-get (cdr (assq (el-get-as-symbol package) p-alist)) 'recipe))) (defun el-get-filter-package-alist-with-status (package-status-alist &rest statuses) "Return package names that are currently in given status" (loop for (p . prop) in package-status-alist + do (assert (symbolp p)) for s = (plist-get prop 'status) when (member s statuses) collect (el-get-as-string p))) @@ -130,12 +133,16 @@ (defun el-get-count-packages-with-status (packages &rest statuses) "Return how many packages are currently in given status in PACKAGES" + (assert (null (remove-if 'symbolp packages))) (length (intersection (mapcar #'el-get-as-symbol (apply #'el-get-list-package-names-with-status statuses)) (mapcar #'el-get-as-symbol packages)))) (defun el-get-extra-packages (&rest packages) "Return installed or required packages that are not in given package list" + (loop for p in packages + when (listp p) do (assert (null (remove-if 'symbolp p))) + else do (assert (symbolp p))) (let ((packages ;; &rest could contain both symbols and lists (loop for p in packages diff --git a/el-get.el b/el-get.el index bc316537d..5c50d0c59 100644 --- a/el-get.el +++ b/el-get.el @@ -363,6 +363,7 @@ which defaults to the first element in `el-get-recipe-path'." "Like `eval-after-load', but first arg is an el-get package name." (let* ((package (el-get-as-symbol package)) (source (el-get-package-def package)) + (assert (symbolp package)) (pkgname (plist-get source :pkgname)) (feats (el-get-as-list (plist-get source :features))) (library (or (plist-get source :library) @@ -602,6 +603,7 @@ PACKAGE may be either a string or the corresponding symbol." (defun el-get-do-install (package) "Install any PACKAGE for which you have a recipe." + (assert (symbolp package)) (el-get-error-unless-package-p package) (if (el-get-package-is-installed package) (el-get-init package) @@ -699,6 +701,7 @@ different install methods." (defun el-get-do-update (package) "Update " + (assert (symbolp package)) (el-get-error-unless-package-p package) (assert (el-get-package-is-installed package) nil "Package %s cannot be updated because it is not installed.") @@ -726,6 +729,7 @@ itself.") "Update PACKAGE." (interactive (list (el-get-read-package-with-status "Update" "required" "installed"))) + (assert (symbolp package)) (el-get-error-unless-package-p package) (if (el-get-update-requires-reinstall package) (el-get-reinstall package) @@ -884,6 +888,7 @@ Also put the checksum in the kill-ring." When PACKAGES is non-nil, only process entries from this list. Those packages from the list we don't know the status of are considered \"required\"." + (assert (null (remove-if 'symbolp packages))) (let* ((p-s-alist (el-get-read-status-file)) (required (el-get-filter-package-alist-with-status p-s-alist "required")) (installed (el-get-filter-package-alist-with-status p-s-alist "installed")) diff --git a/methods/el-get-elpa.el b/methods/el-get-elpa.el index 768ce750a..4108ae738 100644 --- a/methods/el-get-elpa.el +++ b/methods/el-get-elpa.el @@ -89,6 +89,7 @@ the recipe, then return nil." (defun el-get-elpa-install (package url post-install-fun) "Ask elpa to install given PACKAGE." + (assert (symbolp package)) (let* ((elpa-dir (el-get-elpa-package-directory package)) (elpa-repo (el-get-elpa-package-repo package)) ;; Set `package-archive-base' to elpa-repo for old package.el @@ -116,6 +117,7 @@ the recipe, then return nil." (defun el-get-elpa-update (package url post-update-fun) "Ask elpa to update given PACKAGE." + (assert (symbolp package)) (el-get-elpa-remove package url nil) (package-refresh-contents) (package-install (el-get-as-symbol package)) From 7276974bb85683dce06c3625f79d7075c67b693d Mon Sep 17 00:00:00 2001 From: "Ryan C. Thompson" Date: Fri, 9 Mar 2012 18:55:00 -0800 Subject: [PATCH 2/2] Pass pacakge names internally as symbols The function "el-get-as-symbol" is only used in interactive functions that accept user input. All other functions assume packages are passed and returned as symbols. --- el-get-list-packages.el | 3 +-- el-get-status.el | 29 ++++++++++++++--------------- el-get.el | 20 +++++++++----------- methods/el-get-elpa.el | 4 ++-- 4 files changed, 26 insertions(+), 30 deletions(-) diff --git a/el-get-list-packages.el b/el-get-list-packages.el index 4d0e45c13..b691ebd55 100644 --- a/el-get-list-packages.el +++ b/el-get-list-packages.el @@ -74,9 +74,8 @@ matching REGEX with TYPE and ARGS as parameter." (funcall guesser package)))) (defun el-get-describe-1 (package) - (let* ((psym (el-get-as-symbol package)) - (pname (symbol-name psym)) (assert (symbolp package)) + (let* ((pname (el-get-as-string package)) (status (el-get-read-package-status package)) (def (el-get-package-def pname)) (name (plist-get def :name)) diff --git a/el-get-status.el b/el-get-status.el index 5aea521dd..8a6159140 100644 --- a/el-get-status.el +++ b/el-get-status.el @@ -49,8 +49,8 @@ (defun el-get-save-package-status (package status) "Save given package status" - (let* ((package (el-get-as-symbol package)) - (recipe (el-get-package-def package)) + (assert (symbolp package)) + (let* ((recipe (el-get-package-def package)) (package-status-alist (assq-delete-all package (el-get-read-status-file))) (new-package-status-alist @@ -98,7 +98,7 @@ "return current status for PACKAGE" (assert (symbolp package)) (let ((p-alist (or package-status-alist (el-get-read-status-file)))) - (plist-get (cdr (assq (el-get-as-symbol package) p-alist)) 'status))) + (plist-get (cdr (assq package p-alist)) 'status))) (define-obsolete-function-alias 'el-get-package-status 'el-get-read-package-status) @@ -106,7 +106,7 @@ "return current status for PACKAGE" (assert (symbolp package)) (let ((p-alist (or package-status-alist (el-get-read-status-file)))) - (plist-get (cdr (assq (el-get-as-symbol package) p-alist)) 'recipe))) + (plist-get (cdr (assq package p-alist)) 'recipe))) (defun el-get-filter-package-alist-with-status (package-status-alist &rest statuses) "Return package names that are currently in given status" @@ -114,7 +114,7 @@ do (assert (symbolp p)) for s = (plist-get prop 'status) when (member s statuses) - collect (el-get-as-string p))) + collect p)) (defun el-get-list-package-names-with-status (&rest statuses) "Return package names that are currently in given status" @@ -127,16 +127,16 @@ (completing-read (format "%s package: " action) (apply 'el-get-list-package-names-with-status statuses))) -(defun el-get-count-package-with-status (&rest statuses) - "Return how many packages are currently in given status" - (length (apply #'el-get-list-package-names-with-status statuses))) +;; (defun el-get-count-package-with-status (&rest statuses) +;; "Return how many packages are currently in given status" +;; (length (apply #'el-get-list-package-names-with-status statuses))) (defun el-get-count-packages-with-status (packages &rest statuses) "Return how many packages are currently in given status in PACKAGES" (assert (null (remove-if 'symbolp packages))) (length (intersection - (mapcar #'el-get-as-symbol (apply #'el-get-list-package-names-with-status statuses)) - (mapcar #'el-get-as-symbol packages)))) + (apply #'el-get-list-package-names-with-status statuses) + packages))) (defun el-get-extra-packages (&rest packages) "Return installed or required packages that are not in given package list" @@ -146,15 +146,14 @@ (let ((packages ;; &rest could contain both symbols and lists (loop for p in packages - when (listp p) append (mapcar 'el-get-as-symbol p) - else collect (el-get-as-symbol p)))) + when (listp p) append p + else collect p))) (when packages (loop for (p . prop) in (el-get-read-status-file) for s = (plist-get prop 'status) - for x = (el-get-package-symbol p) - unless (member x packages) + unless (member p packages) unless (equal s "removed") - collect (list x s))))) + collect (list p s))))) (defmacro el-get-with-status-sources (&rest body) "Evaluate BODY with `el-get-sources' bound to recipes from status file." diff --git a/el-get.el b/el-get.el index 5c50d0c59..5e0e33ced 100644 --- a/el-get.el +++ b/el-get.el @@ -361,9 +361,8 @@ which defaults to the first element in `el-get-recipe-path'." (defun el-get-eval-after-load (package form) "Like `eval-after-load', but first arg is an el-get package name." - (let* ((package (el-get-as-symbol package)) - (source (el-get-package-def package)) (assert (symbolp package)) + (let* ((source (el-get-package-def package)) (pkgname (plist-get source :pkgname)) (feats (el-get-as-list (plist-get source :features))) (library (or (plist-get source :library) @@ -558,7 +557,7 @@ PACKAGE may be either a string or the corresponding symbol." (if package ;; el-get-do-install will either init the package, installing it ;; first only when necessary to do so - (el-get-do-install (el-get-as-string package)) + (el-get-do-install package) ;; no more packages to install in the dependency walk, clean up (remove-hook 'el-get-post-init-hooks 'el-get-install-next-packages)))) @@ -705,8 +704,7 @@ different install methods." (el-get-error-unless-package-p package) (assert (el-get-package-is-installed package) nil "Package %s cannot be updated because it is not installed.") - (let* ((package (el-get-as-symbol package)) - (source (el-get-package-def package)) + (let* ((source (el-get-package-def package)) (method (el-get-package-method source)) (update (el-get-method method :update)) (url (plist-get source :url))) @@ -733,8 +731,7 @@ itself.") (el-get-error-unless-package-p package) (if (el-get-update-requires-reinstall package) (el-get-reinstall package) - (let* ((package (el-get-as-symbol package)) - (new-dependencies (remove-if 'el-get-package-is-installed + (let* ((new-dependencies (remove-if 'el-get-package-is-installed (el-get-dependencies package))) (source (el-get-package-def package))) (if (plist-get source :checksum) @@ -894,15 +891,15 @@ considered \"required\"." (installed (el-get-filter-package-alist-with-status p-s-alist "installed")) (to-init (if packages (loop for p in packages - when (member (el-get-as-string p) installed) + when (member p installed) collect p) - (mapcar 'el-get-as-symbol installed))) + installed)) (init-deps (el-get-dependencies to-init)) (to-install (if packages (loop for p in packages unless (member p init-deps) collect p) - (mapcar 'el-get-as-symbol required))) + required)) (install-deps (el-get-dependencies to-install)) done) (el-get-verbose-message "el-get-init-and-install: install %S" install-deps) @@ -956,6 +953,7 @@ already installed packages is considered." (let* ((packages ;; (el-get 'sync 'a 'b my-package-list) (loop for p in packages when (listp p) append p else collect p)) + (packages (mapcar 'el-get-as-symbol packages)) (total (length packages)) (installed (el-get-count-packages-with-status packages "installed")) (progress (and (eq sync 'wait) @@ -967,7 +965,7 @@ already installed packages is considered." ;; keep the result of `el-get-init-and-install' to return it even in the ;; 'wait case (prog1 - (el-get-init-and-install (mapcar 'el-get-as-symbol packages)) + (el-get-init-and-install packages) ;; el-get-install is async, that's now ongoing. (when progress diff --git a/methods/el-get-elpa.el b/methods/el-get-elpa.el index 4108ae738..d27414cb9 100644 --- a/methods/el-get-elpa.el +++ b/methods/el-get-elpa.el @@ -108,7 +108,7 @@ the recipe, then return nil." emacs-lisp-mode-hook fundamental-mode-hook prog-mode-hook) (unless p (package-refresh-contents))) - (package-install (el-get-as-symbol package))) + (package-install package)) ;; we symlink even when the package already is installed because it's ;; not an error to have installed ELPA packages before using el-get, and ;; that will register them @@ -120,7 +120,7 @@ the recipe, then return nil." (assert (symbolp package)) (el-get-elpa-remove package url nil) (package-refresh-contents) - (package-install (el-get-as-symbol package)) + (package-install package) (funcall post-update-fun package)) (defun el-get-elpa-remove (package url post-remove-fun)