Rework treatment of built-in packages (#16938)

* Move transient to bootstrap layer

Transient is a built-in package, and also a dependency of many other packages
users might install. It makes more sense to always configure it, also when the
git layer is not used.

* [core] Correctly initialize location of built-in packages

Previously, if no location was specified, it was set to the default `elpa`.
This did not mean, however, that the packages where installed from ELPA; the
built-in packages were used regardless (they might not even be published on an
ELPA, of course).

Now we correctly set the location to `built-in`, in preparation to making
`:location elpa` take effect for built-in packages, too.

* [core] Ensure built-in packages with different locations are updated

We need to account for built-in packages when installing, checking availability
of new versions, updating, and rolling back packages.

Also refactor the last call site of configuration-layer//delete-package that
used package names instead of pkg-desc objects (namely
configuration-layer/rollback), such that we can simplify the function.

* [org] Remove `:min-version` for org package

The package will now get updated by `configuration-layer/update-packages`.
We do not need to manually bump the version anymore.

* [core] Prevent outdated built-in packages from being loaded

Author: fnussbaum

core/core-configuration-layer.el

@@ -426,8 +426,11 @@ installation of initialization.")
 directory with a name starting with `+'.")
 
 (defvar update-packages-alist '()
-  "Used to collect information about rollback packages in the
-cache folder.")
+  "List used to collect information about rollback packages in the
+cache folder.
+
+Each element is a cons cell of the form (PACKAGE-NAME . DIRECTORY),
+where DIRECTORY may be nil for built-in packages.")
 
 (defun configuration-layer/load-lock-file ()
   "Load the .lock file"
@@ -825,7 +828,7 @@ a new object."
     (oset obj excluded
           (and (configuration-layer/layer-used-p layer-name)
                (or excluded (oref obj excluded))))
-    (when location
+    (if location
       (if (and (listp location)
                (eq (car location) 'recipe)
                (eq (plist-get (cdr location) :fetcher) 'local))
@@ -838,7 +841,9 @@ a new object."
                     (oset
                      obj location `(recipe :fetcher file :path ,path))))
            ((eq 'dotfile layer-name) nil))
-        (oset obj location location)))
+        (oset obj location location))
+      (when (and ownerp (package-built-in-p pkg-name))
+        (oset obj location 'built-in)))
     ;; cannot override protected packages
     (unless copyp
       ;; a bootstrap package is protected
@@ -1699,7 +1704,9 @@ RNAME is the name symbol of another existing layer."
              not-inst-count)
      t)
     (spacemacs//redisplay)
-    (unless (package-installed-p pkg-name min-version)
+    (unless (and (package-installed-p pkg-name min-version)
+                 (not (and (package-built-in-p pkg-name)
+                           (not (eq location 'built-in)))))
       (condition-case-unless-debug err
           (cond
            ((or (null pkg) (eq 'elpa location))
@@ -1774,25 +1781,53 @@ RNAME is the name symbol of another existing layer."
       ;; installation
       (when upkg-names
         (spacemacs-buffer/set-mode-line "Installing packages..." t)
+        ;; Prevent built-in org from being loaded when updating consult, for example.
+        (dolist (pkg-name configuration-layer--used-packages)
+          (when (and (package-built-in-p pkg-name)
+                     (not (memq pkg-name upkg-names)))
+            (package-activate pkg-name)))
         (let ((delayed-warnings-backup delayed-warnings-list))
           (spacemacs-buffer/append
            (format "Found %s new package(s) to install...\n"
                    not-inst-count))
           (configuration-layer/retrieve-package-archives)
           (setq installed-count 0)
           (spacemacs//redisplay)
-          ;; bootstrap and pre step packages first
-          (dolist (pkg-name upkg-names)
-            (let ((pkg (configuration-layer/get-package pkg-name)))
-              (when (and pkg (memq (oref pkg step) '(bootstrap pre)))
-                (setq installed-count (1+ installed-count))
-                (configuration-layer//install-package pkg pkg-name installed-count not-inst-count))))
-          ;; then all other packages
-          (dolist (pkg-name upkg-names)
-            (let ((pkg (configuration-layer/get-package pkg-name)))
-              (unless (and pkg (memq (oref pkg step) '(bootstrap pre)))
-                (setq installed-count (1+ installed-count))
+
+          ;; We sort the packages to be installed as follows:
+          ;; 1. built-in packages
+          ;; 2. bootstrap and pre packages
+          ;; 3. all other packages
+          ;; In particular, we install new versions of built-in packages first,
+          ;; to avoid having the built-in package loaded instead of the new one
+          ;; (for example when another package only depends on an older version;
+          ;; or does not explicitly depend on it, but requires some of its
+          ;; features somewhere).
+          ;; FIXME Dependencies between built-in packages that should get updated
+          ;; could still lead to errors due to built-in versions being loaded. For
+          ;; example, if hypothetically, org (optionally) requires transient in
+          ;; the future, we should take care to update transient before org.
+          (let* (built-in bootstrap-pre remaining
+                 sorted-upkg-names)
+            (dolist (pkg-name upkg-names)
+              (let ((pkg (configuration-layer/get-package pkg-name)))
+                (push pkg-name
+                      (cond ((package-built-in-p pkg-name)
+                             built-in)
+                            ((and pkg (or (eq (oref pkg step) 'bootstrap)
+                                          (eq (oref pkg step) 'pre)))
+                             bootstrap-pre)
+                            (t
+                             remaining)))))
+            (setq sorted-upkg-names
+                  (append (nreverse built-in)
+                          (nreverse bootstrap-pre)
+                          (nreverse remaining)))
+            (dolist (pkg-name sorted-upkg-names)
+              (cl-incf installed-count)
+              (let ((pkg (configuration-layer/get-package pkg-name)))
                 (configuration-layer//install-package pkg pkg-name installed-count not-inst-count))))
+
           (spacemacs-buffer/append "\n")
           (unless init-file-debug
             ;; get rid of all delayed warnings when byte-compiling packages
@@ -1857,7 +1892,11 @@ RNAME is the name symbol of another existing layer."
    pkg-names (lambda (x)
                (let* ((pkg (configuration-layer/get-package x))
                       (min-version (when pkg (oref pkg min-version))))
-                 (not (package-installed-p x min-version))))))
+                 (or (and pkg
+                          (package-built-in-p x)
+                          (not (eq 'built-in (oref pkg location)))
+                          (not (assq x package-alist)))
+                     (not (package-installed-p x min-version)))))))
 
 (defun configuration-layer//get-package-recipe (pkg-name)
   "Return the recipe for PKG-NAME if it has one."
@@ -1874,7 +1913,11 @@ RNAME is the name symbol of another existing layer."
         (cur-version (configuration-layer//get-package-version-string pkg-name))
         (quelpa-upgrade-p t)
         new-version)
-    (when cur-version
+    (when (and cur-version
+               ;; Consider built-in packages, but only when
+               ;; they are installed from a different location.
+               (or (not (package-built-in-p pkg-name))
+                   (and pkg (not (eq 'built-in (oref pkg :location))))))
       (setq new-version
             (if recipe
                 (or (quelpa-checkout (configuration-layer//make-quelpa-recipe pkg)
@@ -2187,12 +2230,14 @@ in the back-up directory."
     (dolist (pkg update-packages)
       (unless (memq pkg dotspacemacs-frozen-packages)
         (let* ((src-dir (configuration-layer//get-package-directory pkg))
-               (dest-dir (expand-file-name
-                          (concat rollback-dir
-                                  (file-name-as-directory
-                                   (file-name-nondirectory src-dir))))))
-          (copy-directory src-dir dest-dir 'keeptime 'create 'copy-content)
-          (push (cons pkg (file-name-nondirectory src-dir))
+               (dest-dir (and src-dir
+                              (expand-file-name
+                               (concat rollback-dir
+                                       (file-name-as-directory
+                                        (file-name-nondirectory src-dir)))))))
+          (when src-dir
+            (copy-directory src-dir dest-dir 'keeptime 'create 'copy-content))
+          (push (cons pkg (and src-dir (file-name-nondirectory src-dir)))
                 update-packages-alist))))
     (spacemacs/dump-vars-to-file
      '(update-packages-alist)
@@ -2283,13 +2328,14 @@ Rollback slots are stored in
       (spacemacs//redisplay)
       (dolist (apkg update-packages-alist)
         (let* ((pkg (car apkg))
-               (pkg-dir-name (cdr apkg))
+               (pkg-dir-name (cdr apkg)) ; nil for built-in packages
                (installed-ver
                 (configuration-layer//get-package-version-string pkg))
                (elpa-dir (file-name-as-directory package-user-dir))
-               (src-dir (expand-file-name
-                         (concat rollback-dir (file-name-as-directory
-                                               pkg-dir-name))))
+               (src-dir (and pkg-dir-name
+                             (expand-file-name
+                              (concat rollback-dir (file-name-as-directory
+                                                    pkg-dir-name)))))
                (dest-dir (expand-file-name
                           (concat elpa-dir (file-name-as-directory
                                             pkg-dir-name)))))
@@ -2303,9 +2349,15 @@ Rollback slots are stored in
               (spacemacs-buffer/replace-last-line
                (format "--> rolling back package %s... [%s/%s]"
                        pkg rollbacked-count rollback-count) t)
-              (configuration-layer//package-delete pkg)
-              (copy-directory src-dir dest-dir
-                              'keeptime 'create 'copy-content)))
+              (let ((pkg-desc (cadr (assq pkg package-alist))))
+                (cond
+                 (pkg-desc
+                  (configuration-layer//package-delete pkg-desc))
+                 ((package-built-in-p pkg)
+                  (message "Skipping deletion of package %s since it is built-in." pkg))))
+              (when src-dir
+                (copy-directory src-dir dest-dir
+                                'keeptime 'create 'copy-content))))
           (spacemacs//redisplay)))
       (spacemacs-buffer/append
        (format "\n--> %s packages rolled back.\n" rollbacked-count))
@@ -2357,8 +2409,10 @@ depends on it."
         (gethash pkg-name dependencies))))
 
 (defun configuration-layer//get-package-directory (pkg-name)
-  "Return the directory path for package with name PKG-NAME."
-  (let ((pkg-desc (cadr (assq pkg-name package-alist))))
+  "Return the directory path for package with name PKG-NAME.
+
+Return nil when the package is built-in, and no other version is installed."
+  (and-let* ((pkg-desc (cadr (assq pkg-name package-alist))))
     (package-desc-dir pkg-desc)))
 
 (defun configuration-layer//get-package-deps-from-alist (pkg-name)
@@ -2380,8 +2434,11 @@ depends on it."
 
 (defun configuration-layer//get-package-version-string (pkg-name)
   "Return the version string for package with name PKG-NAME."
-  (and-let* ((pkg-desc (cadr (assq pkg-name package-alist))))
-    (package-version-join (package-desc-version pkg-desc))))
+  (and-let* ((pkg-version
+              (or (and-let* ((pkg-desc (cadr (assq pkg-name package-alist))))
+                    (package-desc-version pkg-desc))
+                  (alist-get pkg-name package--builtin-versions))))
+    (package-version-join pkg-version)))
 
 (defun configuration-layer//get-latest-package-version-string (pkg-name)
   "Return the version string for package with name PKG-NAME."
@@ -2402,9 +2459,7 @@ depends on it."
   (if (configuration-layer//system-package-p pkg-desc)
       (message "Would have removed package %s but this is a system package so it has not been changed."
                (package-desc-name pkg-desc))
-    (if (package-desc-p pkg-desc)
-        (package-delete pkg-desc t t)
-      (package-delete (car (alist-get pkg-desc package-alist)) t t))))
+    (package-delete pkg-desc t t)))
 
 (defun configuration-layer/delete-orphan-packages (packages &optional include-system)
   "Delete PACKAGES if they are orphan.

layers/+distributions/spacemacs-bootstrap/packages.el

@@ -43,7 +43,8 @@
     (spacemacs-theme :location built-in)
     (which-key-posframe :step pre :toggle (and (consp dotspacemacs-which-key-position)
                                                (eq (car dotspacemacs-which-key-position) 'posframe)))
-    dash))
+    dash
+    (transient :location elpa)))
 
 ;; bootstrap packages
 
@@ -689,3 +690,15 @@ Press \\[which-key-toggle-persistent] to hide."
           (intern (format "posframe-poshandler-frame-%s"
                           (cdr dotspacemacs-which-key-position))))
     (which-key-posframe-mode)))
+
+(defun spacemacs-bootstrap/init-transient ()
+  (use-package transient
+    :defer t
+    :init
+    (setq-default transient-history-file (expand-file-name "transient/history.el"
+                                                           spacemacs-cache-directory))
+    (setq-default transient-levels-file (expand-file-name "transient/levels.el"
+                                                          spacemacs-cache-directory))
+    ;; Values are the users saved preferences so they should persist.
+    (setq-default transient-values-file (expand-file-name "transient/values.el"
+                                                          dotspacemacs-directory))))

layers/+emacs/org/packages.el

@@ -33,7 +33,7 @@
     htmlize
     ;; ob, org, org-agenda and org-contacts are installed by `org-contrib'
     (ob :location built-in)
-    (org :location elpa :min-version "9.7.8")
+    (org :location elpa)
     (org-agenda :location built-in)
     (org-alert  :toggle org-enable-notifications)
     (org-contacts :toggle org-enable-org-contacts-support)

layers/+source-control/git/packages.el

@@ -46,8 +46,7 @@
     org
     (orgit :requires org)
     (orgit-forge :requires (org forge))
-    smeargle
-    transient))
+    smeargle))
 
 
 (defun git/pre-init-golden-ratio ()
@@ -323,19 +322,6 @@
       "gHh" 'smeargle-commits
       "gHt" 'smeargle)))
 
-(defun git/pre-init-transient ()
-  (setq-default transient-history-file (expand-file-name "transient/history.el"
-                                                         spacemacs-cache-directory))
-  (setq-default transient-levels-file (expand-file-name "transient/levels.el"
-                                                        spacemacs-cache-directory))
-  ;; Values are the users saved preferences so they should persist.
-  (setq-default transient-values-file (expand-file-name "transient/values.el"
-                                                        dotspacemacs-directory)))
-
-(defun git/init-transient ()
-  (use-package transient
-    :defer t))
-
 (defun git/init-forge ()
   (use-package forge
     :after magit