bug#74040: `require-with-check` signals error for `project`

bug#74040: `require-with-check` signals error for `project`

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Package: Emacs
Version: 30.0.50

1. Customize `load-prefer-newer` to t.
2. Edit project.el in-tree, save it, don't recompile Emacs.
3. Restart Emacs.
4. Visit some code file and try to launch `M-x eglot` there.

You should get an error along the lines of:

    require-with-check: Feature ‘project’ loaded from
    ".../lisp/progmodes/project.el" is now provided by
    ".../lisp/progmodes/project.elc"

This error is spurious.  AFAICT, it is due to the fact that
`require-with-check` uses `locate-file` to "guess" which file `load`
would use, but `locate-file` doesn't obey `load-prefer-newer`, so it
guesses wrong.


        Stefan

bug#74040: `require-with-check` signals error for `project`

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 794 bytes --]

> 1. Customize `load-prefer-newer` to t.
> 2. Edit project.el in-tree, save it, don't recompile Emacs.
> 3. Restart Emacs.
> 4. Visit some code file and try to launch `M-x eglot` there.
>
> You should get an error along the lines of:
>
>     require-with-check: Feature ‘project’ loaded from
>     ".../lisp/progmodes/project.el" is now provided by
>     ".../lisp/progmodes/project.elc"
>
> This error is spurious.  AFAICT, it is due to the fact that
> `require-with-check` uses `locate-file` to "guess" which file `load`
> would use, but `locate-file` doesn't obey `load-prefer-newer`, so it
> guesses wrong.

One approach is to expose the `prefer` option of `openp` to
`locate-file` so we can use it in `require-with-check`, as in the
patch below.


        Stefan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: require-with-check.patch --]
[-- Type: text/x-diff, Size: 4031 bytes --]

diff --git a/lisp/files.el b/lisp/files.el
index a81f742bbb4..a67d34f81b2 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -1039,7 +1039,7 @@ load-file
 
 (defvar comp-eln-to-el-h)
 
-(defun locate-file (filename path &optional suffixes predicate)
+(defun locate-file (filename path &optional suffixes predicate prefer-newer)
   "Search for FILENAME through PATH.
 If found, return the absolute file name of FILENAME; otherwise
 return nil.
@@ -1057,7 +1057,10 @@ locate-file
 in which case file name handlers are ignored.  This usage is deprecated.
 For compatibility, PREDICATE can also be one of the symbols
 `executable', `readable', `writable', or `exists', or a list of
-one or more of those symbols."
+one or more of those symbols.
+
+PREFER-NEWER, if non-nil, means to disregard the order of SUFFIXES and
+return the most recently modified file instead."
   (if (and predicate (symbolp predicate) (not (functionp predicate)))
       (setq predicate (list predicate)))
   (when (and (consp predicate) (not (functionp predicate)))
@@ -1065,7 +1068,7 @@ locate-file
 	  (logior (if (memq 'executable predicate) 1 0)
 		  (if (memq 'writable predicate) 2 0)
 		  (if (memq 'readable predicate) 4 0))))
-  (locate-file-internal filename path suffixes predicate))
+  (locate-file-internal filename path suffixes predicate prefer-newer))
 
 (defun locate-file-completion-table (dirs suffixes string pred action)
   "Do completion for file names passed to `locate-file'."
@@ -1278,7 +1281,8 @@ require-with-check
     (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))))
+                             load-path (get-load-suffixes) nil
+                             load-prefer-newer)))
         (cond
          ((assoc fn load-history) nil)  ;We loaded the right file.
          ((eq noerror 'reload) (load fn nil 'nomessage))
diff --git a/src/lread.c b/src/lread.c
index 854aaa784ad..6cc2e8b6349 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -1793,7 +1793,7 @@ complete_filename_p (Lisp_Object pathname)
 	      && IS_DEVICE_SEP (s[1]) && IS_DIRECTORY_SEP (s[2])));
 }
 
-DEFUN ("locate-file-internal", Flocate_file_internal, Slocate_file_internal, 2, 4, 0,
+DEFUN ("locate-file-internal", Flocate_file_internal, Slocate_file_internal, 2, 5, 0,
        doc: /* Search for FILENAME through PATH.
 Returns the file's name in absolute form, or nil if not found.
 If SUFFIXES is non-nil, it should be a list of suffixes to append to
@@ -1802,12 +1802,15 @@ DEFUN ("locate-file-internal", Flocate_file_internal, Slocate_file_internal, 2,
 PREDICATE can also be an integer to pass to the faccessat(2) function,
 in which case file-name-handlers are ignored.
 This function will normally skip directories, so if you want it to find
-directories, make sure the PREDICATE function returns `dir-ok' for them.  */)
-  (Lisp_Object filename, Lisp_Object path, Lisp_Object suffixes, Lisp_Object predicate)
+directories, make sure the PREDICATE function returns `dir-ok' for them.
+PREFER-NEWER, if non-nil, means to disregard the order of SUFFIXES and
+return the most recently modified file instead.  */)
+  (Lisp_Object filename, Lisp_Object path, Lisp_Object suffixes,
+   Lisp_Object predicate, Lisp_Object prefer_newer)
 {
   Lisp_Object file;
-  int fd = openp (path, filename, suffixes, &file, predicate, false, true,
-		  NULL);
+  int fd = openp (path, filename, suffixes, &file, predicate,
+		  !NILP (prefer_newer), true, NULL);
   if (NILP (predicate) && fd >= 0)
     emacs_close (fd);
   return file;
@@ -1886,7 +1889,7 @@ maybe_swap_for_eln (bool no_native, Lisp_Object *filename, int *fd,
 		 can't find even central .el files.  */
 	      if (NILP (Flocate_file_internal (build_string ("simple.el"),
 					       Vload_path,
-					       Qnil, Qnil)))
+					       Qnil, Qnil, Qnil)))
 		return;
 	      Vdelayed_warnings_list
 		= Fcons (list2

bug#74040: `require-with-check` signals error for `project`

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 409 bytes --]

> One approach is to expose the `prefer` option of `openp` to
> `locate-file` so we can use it in `require-with-check`, as in the
> patch below.

Another is to replace the `assoc` check with one that is more lenient,
ignoring difference in file extensions.  E.g. the patch below.
This makes the change more localized, at the cost of making
`require-with-check` slower and less strict.

WDYT?


        Stefan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: require-with-check.patch --]
[-- Type: text/x-diff, Size: 1293 bytes --]

diff --git a/lisp/files.el b/lisp/files.el
index a81f742bbb4..a5177250f45 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -1277,10 +1280,19 @@ require-with-check
     ;; 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))))
+      (let* ((fn (locate-file (or filename (symbol-name feature))
+                              load-path (get-load-suffixes) nil
+                              )) ;; load-prefer-newer
+             (fn (if (string-match
+                      (concat (regexp-opt (get-load-suffixes)) "\\'") fn)
+                     (concat (substring fn 0 (match-end 0)) ".")
+                   fn))
+             (lh load-history))
+        (while (and lh (let ((file (car-safe (car lh))))
+                         (not (and file (string-prefix-p fn file)))))
+          (setq lh (cdr lh)))
         (cond
-         ((assoc fn load-history) nil)  ;We loaded the right file.
+         (lh nil)  ;We loaded the right file.
          ((eq noerror 'reload) (load fn nil 'nomessage))
          ((and fn (memq feature features))
           (funcall (if noerror #'warn #'error)

bug#74040: `require-with-check` signals error for `project`

[Stefan Kangas]

Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

>> One approach is to expose the `prefer` option of `openp` to
>> `locate-file` so we can use it in `require-with-check`, as in the
>> patch below.
>
> Another is to replace the `assoc` check with one that is more lenient,
> ignoring difference in file extensions.  E.g. the patch below.
> This makes the change more localized, at the cost of making
> `require-with-check` slower and less strict.

Either patch is fine by me, but having the change more localized is
certainly nice.

Do you have any other use case in mind for the new PREFER-NEWER argument
of `locate-file` than `require-with-check`?  If not, perhaps we should
avoid making it more complex?

bug#74040: `require-with-check` signals error for `project`

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Either patch is fine by me, but having the change more localized is
> certainly nice.

Agreed.

> Do you have any other use case in mind for the new PREFER-NEWER argument
> of `locate-file` than `require-with-check`?  If not, perhaps we should
> avoid making it more complex?

In principle, it's nice to be able to reproduce *exactly* the search
performed by `load` and `require`, but in practice, I don't have any
real use case, no.

I pushed the more localized patch (after fixing it so it actually
works).


        Stefan