bug#67696: 30.0.50; Help deal with multiple versions in load-path

[Stefan Monnier]

Package: Emacs
Version: 30.0.50


With packages being available both as bundled with Emacs and as ELPA
packages, it has become a lot more common place to have two versions of
a package in the `load-path` and to have to deal with situations
where the "incorrect" version has been loaded before `load-path`
was changed.

These kinds of problems manifest in various ways and we try to
circumvent them in `package.el` in some cases but that can't cover
all cases.

I suggest we introduce a new function to help packages susceptible to
those problems.  The patch below introduces a new function which
I tentatively called `require-with-check` and shows how it could be used
in the case of `eglot.el` (which relies on several core packages also
distributed via GNU ELPA and currently uses a hack which slows it down
unnecessarily in the normal case).

As mentioned in a FIXME in there, maybe we should also consider
adding to `seq` (and `eldoc`) something like

    ;;;###autoload
    (if (featurep 'seq) (require-with-check 'seq 'reload))


-- Stefan

diff --git a/lisp/files.el b/lisp/files.el
index 1cdcec23b11..e1e885462ce 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -1246,6 +1246,29 @@ load-library
   (interactive (list (read-library-name)))
   (load library))
 
+(defun require-with-check (feature &optional filename noerror)
+  "If FEATURE is not already loaded, load it from FILENAME.
+This is like `require' except if FEATURE is already a member of the list
+`features’, then we check if this was provided by a different file than the
+one that we would load now (presumably because `load-path' has been
+changed since the file was loaded).
+If it's the case, we either signal an error (the default), or forcibly reload
+the new file (if NOERROR is equal to `reload'), or otherwise emit a warning."
+  (let ((lh load-history)
+        (res (require feature filename (if (eq noerror 'reload) nil noerror))))
+    ;; If the `feature' was not yet provided, `require' just loaded the right
+    ;; file, so we're done.
+    (when (eq lh load-history)
+      ;; If `require' did nothing, we need to make sure that was warranted.
+      (let ((fn (locate-file (or filename (symbol-name feature))
+                             load-path (get-load-suffixes))))
+        (cond
+         ((assoc fn load-history) nil)  ;We loaded the right file.
+         ((eq noerror 'reload) (load fn nil 'nomessage))
+         (t (funcall (if noerror #'warn #'error)
+                     "Feature provided by other file: %S" feature)))))
+    res))
+
 (defun file-remote-p (file &optional identification connected)
   "Test whether FILE specifies a location on a remote system.
 A file is considered remote if accessing it is likely to
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index d410367f902..8124a3f52e0 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -116,13 +116,16 @@
 ;; having installed them, didn't correctly re-load them over the
 ;; built-in versions.
 (eval-and-compile
-  (load "project")
-  (load "eldoc")
-  (load "seq")
-  (load "flymake")
-  (load "xref")
-  (load "jsonrpc")
-  (load "external-completion"))
+  ;; For those packages that are preloaded, reload them if needed,
+  ;; since that's the best we can do anyway.
+  ;; FIXME: Maybe the ELPA packages for those preloaded packages should
+  ;; force-reload themselves eagerly when the package is activated!
+  (require-with-check 'eldoc nil 'reload)
+  (require-with-check 'seq nil 'reload)
+  ;; For those packages which are not preloaded OTOH, signal an error if
+  ;; the loaded file is not the one that should have been loaded.
+  (mapc #'require-with-check
+        '(project flymake xref jsonrpc external-completion)))
 
 ;; forward-declare, but don't require (Emacs 28 doesn't seem to care)
 (defvar markdown-fontify-code-blocks-natively)
@@ -138,11 +141,12 @@ tramp-use-ssh-controlmaster-options
                         'eglot-managed-mode-hook "1.6")
 (define-obsolete-variable-alias 'eglot-confirm-server-initiated-edits
   'eglot-confirm-server-edits "1.16")
-(define-obsolete-function-alias 'eglot--uri-to-path 'eglot-uri-to-path "1.16")
-(define-obsolete-function-alias 'eglot--path-to-uri 'eglot-path-to-uri "1.16")
-(define-obsolete-function-alias 'eglot--range-region 'eglot-range-region 
"1.16")
-(define-obsolete-function-alias 'eglot--server-capable 'eglot-server-capable 
"1.16")
-(define-obsolete-function-alias 'eglot--server-capable-or-lose 
'eglot-server-capable-or-lose "1.16")
+(define-obsolete-function-alias 'eglot--uri-to-path #'eglot-uri-to-path "1.16")
+(define-obsolete-function-alias 'eglot--path-to-uri #'eglot-path-to-uri "1.16")
+(define-obsolete-function-alias 'eglot--range-region #'eglot-range-region 
"1.16")
+(define-obsolete-function-alias 'eglot--server-capable #'eglot-server-capable 
"1.16")
+(define-obsolete-function-alias 'eglot--server-capable-or-lose
+  #'eglot-server-capable-or-lose "1.16")
 (define-obsolete-function-alias
   'eglot-lsp-abiding-column 'eglot-utf-16-linepos "1.12")
 (define-obsolete-function-alias