bug#66542: Fix: locate-dominating-file predicate should receive dir not file

[dalanicolai <dalanicolai@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2198 bytes --]

You are right (of course :)
So I have attached another patch which only strips the file name (applies
file-name-directory) if the file is not a directory.

If the file is not a directory, then that stripped name should also be
returned by the function, i.e. in the 'cond`, root should be set to the
stripped file name. Therefore, the patched version simply 'checks and sets'
the file name first inside the setq.

Also, now we can remove the check in the first clause of the 'if' (of 'setq
try'), because 'file' is guaranteed to be a directory name, and the
existence of the directory name is already checked by the 'file-exists-p'.

This change does not affect other parts of the function (except that it
speeds it up a little, because it excludes the cycles, that only strip the
file names in case file is a 'real' file.
Indeed, the function should only check for 'parent directories' (not files).

I hope I did not miss anything.


On Sat, 14 Oct 2023 at 17:46, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: dalanicolai <dalanicolai@gmail.com>
> > Date: Sat, 14 Oct 2023 17:11:18 +0200
> >
> > The docstring of 'locate-dominating-file' mentions that its NAME argument
> > should be a dir, but currently it simply receives the FILE
> > argument. Therefore, using the function e.g. with the following
> predicate for NAME
> >
> > (lambda (dir)
> > (seq-filter (apply-partially #'string-match-p "paint")
> > (directory-files
> > dir)))
> >
> > to check if a directory contains a file regexp-matching 'paint', throws
> > an error:
> >
> > (file-error "Opening directory" "Not a directory" "/home/...")
> >
> > This patch simply wraps the FILE argument in the (funcall NAME FILE) with
> > a 'file-name-directory' thereby fixing the function.
>
> Sorry, I don't understand: the return value of file-name-directory is
> not identical to its argument when the argument is a directory, so
> this patch might change the behavior.  Or what am I missing?
>
> In addition, I don't think I understand the problem you are trying to
> solve.  Could you please describe the problem more completely, by
> telling when and how was locate-dominating-file called with the name
> of a file that is not a directory?
>

[-- Attachment #1.2: Type: text/html, Size: 2966 bytes --]

[-- Attachment #2: locate-dominating-file-patch --]
[-- Type: application/octet-stream, Size: 819 bytes --]

diff --git a/lisp/files.el b/lisp/files.el
index e1421b403bf..4e3c9699852 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -1135,9 +1135,11 @@ locate-dominating-file
     (while (not (or root
                     (null file)
                     (string-match locate-dominating-stop-dir-regexp file)))
-      (setq try (if (stringp name)
-                    (and (file-directory-p file)
-                         (file-exists-p (expand-file-name name file)))
+      (setq file (if (file-directory-p file)
+                     file
+                   (file-name-directory file))
+            try (if (stringp name)
+                    (file-exists-p (expand-file-name name file))
                   (funcall name file)))
       (cond (try (setq root file))
             ((equal file (setq file (file-name-directory