diff --git a/core/core-configuration-layer.el b/core/core-configuration-layer.el index 1025b2459..dd35c6d5f 100644 --- a/core/core-configuration-layer.el +++ b/core/core-configuration-layer.el @@ -1823,43 +1823,57 @@ RNAME is the name symbol of another existing layer." (defun configuration-layer//configure-packages-2 (packages) "Configure all passed PACKAGES." - (dolist (pkg-name packages) - (spacemacs-buffer/loading-animation) - (let ((pkg (configuration-layer/get-package pkg-name))) - (cond - ((oref pkg :lazy-install) - (spacemacs-buffer/message - (format "%S ignored since it can be lazily installed." pkg-name))) - ((and (oref pkg :excluded) - (not (oref pkg :protected))) - (spacemacs-buffer/message - (format "%S ignored since it has been excluded." pkg-name))) - ((null (oref pkg :owners)) - (spacemacs-buffer/message - (format "%S ignored since it has no owner layer." pkg-name))) - ((not (configuration-layer//package-reqs-used-p pkg)) - (spacemacs-buffer/message - (format (concat "%S is ignored since it has dependencies " - "that are not used.") pkg-name))) - ((not (cfgl-package-enabled-p pkg)) - (spacemacs-buffer/message (format "%S is disabled." pkg-name))) - (t - ;; load-path - (let ((dir (configuration-layer/get-location-directory - pkg-name - (oref pkg :location) - (car (oref pkg :owners))))) - (when dir - (add-to-list 'load-path dir))) - ;; configuration - (unless (memq (oref pkg :location) '(local site built-in)) - (configuration-layer//activate-package pkg-name)) + (let (packages-to-configure) + (dolist (pkg-name packages) + (spacemacs-buffer/loading-animation) + (let ((pkg (configuration-layer/get-package pkg-name))) (cond - ((eq 'dotfile (car (oref pkg :owners))) + ((oref pkg :lazy-install) (spacemacs-buffer/message - (format "%S is configured in the dotfile." pkg-name))) + (format "%S ignored since it can be lazily installed." pkg-name))) + ((and (oref pkg :excluded) + (not (oref pkg :protected))) + (spacemacs-buffer/message + (format "%S ignored since it has been excluded." pkg-name))) + ((null (oref pkg :owners)) + (spacemacs-buffer/message + (format "%S ignored since it has no owner layer." pkg-name))) + ((not (configuration-layer//package-reqs-used-p pkg)) + (spacemacs-buffer/message + (format (concat "%S is ignored since it has dependencies " + "that are not used.") pkg-name))) + ((not (cfgl-package-enabled-p pkg)) + (spacemacs-buffer/message (format "%S is disabled." pkg-name))) (t - (configuration-layer//configure-package pkg)))))))) + ;; load-path + (let ((dir (configuration-layer/get-location-directory + pkg-name + (oref pkg :location) + (car (oref pkg :owners))))) + (when dir + (add-to-list 'load-path dir))) + ;; configuration + (unless (memq (oref pkg :location) '(local site built-in)) + (configuration-layer//activate-package pkg-name)) + (cond + ((eq 'dotfile (car (oref pkg :owners))) + (spacemacs-buffer/message + (format "%S is configured in the dotfile." pkg-name))) + (t + ;; first loop executes pre-init functions, this allows to setup + ;; use-package hooks without sorting issues. + ;; For instance a package B adds a use-package hook on package A, + ;; since we configure packages in alphabetical order, the package B + ;; is configured after package A. But we need B to setup the + ;; use-package hook for A before A is being actually configured. + ;; The solution is to always put use-package hook declarations in + ;; pre-init functions and first call all pre-init functions so we + ;; effectively setup all the use-package hooks. Then we configure + ;; the packages in alphabetical order as usual. + (push pkg packages-to-configure) + (configuration-layer//pre-configure-package pkg))))))) + ;; actually configure packages in alphabetical order + (mapc 'configuration-layer//configure-package packages-to-configure))) (defun configuration-layer/get-location-directory (pkg-name location owner) "Return the location on disk for PKG." @@ -1899,20 +1913,18 @@ LAYER must not be the owner of PKG." (memq layer enabled) (not (memq layer disabled)))))) -(defun configuration-layer//configure-package (pkg) - "Configure PKG object." +(defun configuration-layer//pre-configure-package (pkg) + "Pre-configure PKG object, i.e. call its pre-init functions." (let* ((pkg-name (oref pkg :name)) (owner (car (oref pkg :owners)))) - (spacemacs-buffer/message (format "Configuring %S..." pkg-name)) - ;; pre-init (mapc (lambda (layer) (when (configuration-layer/layer-used-p layer) (if (not (configuration-layer//package-enabled-p pkg layer)) (spacemacs-buffer/message - (format " -> ignored pre-init (%S)..." layer)) + (format "%S -> ignored pre-init (%S)..." pkg-name layer)) (spacemacs-buffer/message - (format " -> pre-init (%S)..." layer)) + (format "%S -> pre-init (%S)..." pkg-name layer)) (condition-case-unless-debug err (funcall (intern (format "%S/pre-init-%S" layer pkg-name))) ('error @@ -1920,9 +1932,14 @@ LAYER must not be the owner of PKG." (concat "\nAn error occurred while pre-configuring %S " "in layer %S (error: %s)\n") pkg-name layer err)))))) - (oref pkg :pre-layers)) + (oref pkg :pre-layers)))) + +(defun configuration-layer//configure-package (pkg) + "Configure PKG object (call their init function and post-init functions)." + (let* ((pkg-name (oref pkg :name)) + (owner (car (oref pkg :owners)))) ;; init - (spacemacs-buffer/message (format " -> init (%S)..." owner)) + (spacemacs-buffer/message (format "%S -> init (%S)..." pkg-name owner)) (funcall (intern (format "%S/init-%S" owner pkg-name))) ;; post-init (mapc @@ -1930,9 +1947,9 @@ LAYER must not be the owner of PKG." (when (configuration-layer/layer-used-p layer) (if (not (configuration-layer//package-enabled-p pkg layer)) (spacemacs-buffer/message - (format " -> ignored post-init (%S)..." layer)) + (format "%S -> ignored post-init (%S)..." pkg-name layer)) (spacemacs-buffer/message - (format " -> post-init (%S)..." layer)) + (format "%S -> post-init (%S)..." pkg-name layer)) (condition-case-unless-debug err (funcall (intern (format "%S/post-init-%S" layer pkg-name))) ('error diff --git a/doc/LAYERS.org b/doc/LAYERS.org index f613820e1..8e5199790 100644 --- a/doc/LAYERS.org +++ b/doc/LAYERS.org @@ -577,9 +577,8 @@ Since a call to =use-package= may evaluate the =:init= block immediately, any function that wants to inject code into this block must run =before= the call to =use-package=. Further, since this call to =use-package= typically takes place in the =init-= function, calls to =spacemacs|use-package-add-hook= -typically happen in the =pre-init-= functions, and not in -=post-init-=. It is quite safe to do this in =pre-init=, so that should -be the default choice. +*always* happen in the =pre-init-= functions, and not in +=post-init-=. ** Best practices If you break any of these rules, you should know what you are doing and have a diff --git a/tests/core/core-configuration-layer-utest.el b/tests/core/core-configuration-layer-utest.el index 922f36df1..4100997f9 100644 --- a/tests/core/core-configuration-layer-utest.el +++ b/tests/core/core-configuration-layer-utest.el @@ -2066,6 +2066,24 @@ pkg5) (ht-keys configuration-layer--indexed-packages)))))) +;; --------------------------------------------------------------------------- +;; configuration-layer//pre-configure-package +;; --------------------------------------------------------------------------- + +(ert-deftest test-pre-configure-package--pre-init-is-evaluated () + (let ((pkg (cfgl-package "pkg" :name 'pkg :owners '(layer1) :pre-layers '(layer2))) + configuration-layer--used-layers + (configuration-layer--indexed-layers (make-hash-table :size 1024)) + (mocker-mock-default-record-cls 'mocker-stub-record)) + (helper--add-layers `(,(cfgl-layer "layer1" :name 'layer1) + ,(cfgl-layer "layer2" :name 'layer2)) t) + (defun layer1/init-pkg nil) + (defun layer2/pre-init-pkg nil) + (mocker-let + ((spacemacs-buffer/message (m) ((:output nil))) + (layer2/pre-init-pkg nil ((:output nil :occur 1)))) + (configuration-layer//pre-configure-package pkg)))) + ;; --------------------------------------------------------------------------- ;; configuration-layer//configure-package ;; --------------------------------------------------------------------------- @@ -2082,20 +2100,6 @@ (layer1/init-pkg nil ((:output nil :occur 1)))) (configuration-layer//configure-package pkg)))) -(ert-deftest test-configure-package--pre-init-is-evaluated () - (let ((pkg (cfgl-package "pkg" :name 'pkg :owners '(layer1) :pre-layers '(layer2))) - configuration-layer--used-layers - (configuration-layer--indexed-layers (make-hash-table :size 1024)) - (mocker-mock-default-record-cls 'mocker-stub-record)) - (helper--add-layers `(,(cfgl-layer "layer1" :name 'layer1) - ,(cfgl-layer "layer2" :name 'layer2)) t) - (defun layer1/init-pkg nil) - (defun layer2/pre-init-pkg nil) - (mocker-let - ((spacemacs-buffer/message (m) ((:output nil))) - (layer2/pre-init-pkg nil ((:output nil :occur 1)))) - (configuration-layer//configure-package pkg)))) - (ert-deftest test-configure-package--post-init-is-evaluated () (let ((pkg (cfgl-package "pkg" :name 'pkg :owners '(layer1) :post-layers '(layer2))) configuration-layer--used-layers @@ -2110,21 +2114,6 @@ (layer2/post-init-pkg nil ((:output nil :occur 1)))) (configuration-layer//configure-package pkg)))) -(ert-deftest test-configure-package--pre-init-is-evaluated-before-init () - (let ((pkg (cfgl-package "pkg" :name 'pkg :owners '(layer1) :pre-layers '(layer2))) - configuration-layer--used-layers - (configuration-layer--indexed-layers (make-hash-table :size 1024)) - (witness nil) - (mocker-mock-default-record-cls 'mocker-stub-record)) - (helper--add-layers `(,(cfgl-layer "layer1" :name 'layer1) - ,(cfgl-layer "layer2" :name 'layer2)) t) - (defun layer1/init-pkg () (push 'init witness)) - (defun layer2/pre-init-pkg () (push 'pre-init witness)) - (mocker-let - ((spacemacs-buffer/message (m) ((:output nil)))) - (configuration-layer//configure-package pkg) - (should (equal '(init pre-init) witness))))) - (ert-deftest test-configure-package--post-init-is-evaluated-after-init () (let ((pkg (cfgl-package "pkg" :name 'pkg :owners '(layer1) :post-layers '(layer2))) configuration-layer--used-layers @@ -2177,6 +2166,7 @@ (defun layer3/post-init-pkg () (push 'post-init witness)) (mocker-let ((spacemacs-buffer/message (m) ((:output nil)))) + (configuration-layer//pre-configure-package pkg) (configuration-layer//configure-package pkg) (should (equal '(post-init init pre-init) witness))))) @@ -2196,9 +2186,9 @@ (defun layer2/pre-init-pkg () (push 'pre-init witness)) (defun layer3/post-init-pkg () (push 'post-init witness)) (mocker-let - ((spacemacs-buffer/message (m) ((:output nil)))) - (configuration-layer//configure-package pkg) - (should (equal '(init) witness))))) + ((spacemacs-buffer/message (m) ((:output nil)))) + (configuration-layer//configure-package pkg) + (should (equal '(init) witness))))) (ert-deftest test-configure-package--enabled-for-partial () (let ((pkg (cfgl-package "pkg" :name 'pkg :owners '(layer1) @@ -2216,14 +2206,33 @@ (defun layer2/pre-init-pkg () (push 'pre-init witness)) (defun layer3/post-init-pkg () (push 'post-init witness)) (mocker-let - ((spacemacs-buffer/message (m) ((:output nil)))) - (configuration-layer//configure-package pkg) - (should (equal '(init pre-init) witness))))) + ((spacemacs-buffer/message (m) ((:output nil)))) + (configuration-layer//pre-configure-package pkg) + (configuration-layer//configure-package pkg) + (should (equal '(init pre-init) witness))))) ;; --------------------------------------------------------------------------- ;; configuration-layer//configure-packages-2 ;; --------------------------------------------------------------------------- +(ert-deftest test-configure-packages-2--pre-init-is-evaluated-before-init () + (let ((pkg (cfgl-package "pkg" :name 'pkg :owners '(layer1) :pre-layers '(layer2))) + configuration-layer--used-layers + (configuration-layer--indexed-layers (make-hash-table :size 1024)) + configuration-layer--used-packages + (configuration-layer--indexed-packages (make-hash-table :size 2048)) + (witness nil) + (mocker-mock-default-record-cls 'mocker-stub-record)) + (helper--add-layers `(,(cfgl-layer "layer1" :name 'layer1) + ,(cfgl-layer "layer2" :name 'layer2)) t) + (helper--add-packages (list pkg) t) + (defun layer1/init-pkg () (push 'init witness)) + (defun layer2/pre-init-pkg () (push 'pre-init witness)) + (mocker-let + ((spacemacs-buffer/loading-animation nil ((:output nil)))) + (configuration-layer//configure-packages-2 `(,(oref pkg :name))) + (should (equal '(init pre-init) witness))))) + (ert-deftest test-configure-packages-2--package-w/-layer-owner-is-configured() (let ((pkg (cfgl-package "pkg" :name 'pkg :owners '(layer1))) configuration-layer--used-packages