Re: master 888ff3755d4 1/3: New function internal--c-header-file-path

Re: master 888ff3755d4 1/3: New function internal--c-header-file-path

[Eli Zaretskii]

Stefan,

Thanks for adding this function.  However, the method it uses to find
the include directories is incorrect and unportable.  On most systems
it will produce the default "/usr/include", which is most probably
wrong.

The way to ask GCC to show the list of directories where it looks for
header files is like this:

   (call-process "gcc" nil BUFFER nil "-v" "-E" "-")

and then look in BUFFER for text that begins with "#include <...>
search starts here:" and ends with "End of search list."  What's
in-between is the list of include directories, one directory per line,
which GCC searches for header file, in the order it searches them.

Re: master 888ff3755d4 1/3: New function internal--c-header-file-path

[Andreas Schwab]

On Jan 06 2025, Eli Zaretskii wrote:

> and then look in BUFFER for text that begins with "#include <...>
> search starts here:" and ends with "End of search list."

Modulo l10n.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: master 888ff3755d4 1/3: New function internal--c-header-file-path

[Stefan Kangas]

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

Eli Zaretskii <eliz@gnu.org> writes:

> Stefan,
>
> Thanks for adding this function.  However, the method it uses to find
> the include directories is incorrect and unportable.  On most systems
> it will produce the default "/usr/include", which is most probably
> wrong.
>
> The way to ask GCC to show the list of directories where it looks for
> header files is like this:
>
>    (call-process "gcc" nil BUFFER nil "-v" "-E" "-")
>
> and then look in BUFFER for text that begins with "#include <...>
> search starts here:" and ends with "End of search list."  What's
> in-between is the list of include directories, one directory per line,
> which GCC searches for header file, in the order it searches them.

Thanks for helping improve this.  How about the attached patch?

[-- Attachment #2: 0001-Make-internal-c-header-file-path-more-portable.patch --]
[-- Type: text/x-patch, Size: 8018 bytes --]

From c8aac6531fbf228d004b15498fd0758e080c784e Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Mon, 6 Jan 2025 22:37:51 +0100
Subject: [PATCH] Make internal--c-header-file-path more portable

* lisp/subr.el (internal--c-header-file-path): Make more portable.
Problem reported by Eli Zaretskii <eliz@gnu.org>.
* test/lisp/subr-tests.el
(subr-tests-internal--c-header-file-path)
(subr-tests-internal--c-header-file-path/gcc-mocked)
(subr-tests-internal--c-header-file-path/clang-mocked): Adjust tests.
---
 lisp/subr.el            | 57 +++++++++++++++------------------------
 test/lisp/subr-tests.el | 60 ++++++++++++++++++++++++++---------------
 2 files changed, 59 insertions(+), 58 deletions(-)

diff --git a/lisp/subr.el b/lisp/subr.el
index 5be8d8f52d4..e0f5542a011 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -7574,41 +7574,26 @@ internal--c-header-file-path
   ;; See also (Bug#10702):
   ;; cc-search-directories, semantic-c-dependency-system-include-path,
   ;; semantic-gcc-setup
-  (delete-dups
-   (let ((base '("/usr/include" "/usr/local/include")))
-     (cond ((or (internal--gcc-is-clang-p)
-                (and (executable-find "clang")
-                     (not (executable-find "gcc"))))
-            ;; This is either macOS, or a system with clang only.
-            (with-temp-buffer
-              (ignore-errors
-                (call-process (if (internal--gcc-is-clang-p) "gcc" "clang")
-                              nil t nil
-                              "-v" "-E" "-"))
-              (goto-char (point-min))
-              (narrow-to-region
-               (save-excursion
-                 (re-search-forward
-                  "^#include <\\.\\.\\.> search starts here:\n" nil t)
-                 (point))
-               (save-excursion
-                 (re-search-forward "^End of search list.$" nil t)
-                 (pos-bol)))
-              (while (search-forward "(framework directory)" nil t)
-                (delete-line))
-              (append base
-                      (reverse
-                       (split-string (buffer-substring-no-properties
-                                      (point-min) (point-max)))))))
-           ;; Prefer GCC.
-           ((let ((arch (with-temp-buffer
-                          (when (eq 0 (ignore-errors
-                                        (call-process "gcc" nil '(t nil) nil
-                                                      "-print-multiarch")))
-                            (goto-char (point-min))
-                            (buffer-substring (point) (line-end-position))))))
-              (if (zerop (length arch))
-                  base
-                (append base (list (expand-file-name arch "/usr/include"))))))))))
+  (if (or (executable-find "gcc")
+          (executable-find "clang"))
+      (with-temp-buffer
+        (ignore-errors
+          (call-process (if (executable-find "gcc") "gcc" "clang")
+                        nil t nil
+                        "-v" "-E" "-"))
+        (goto-char (point-min))
+        (narrow-to-region
+         (save-excursion
+           (re-search-forward
+            "^#include <\\.\\.\\.> [[:word:] ]+:\n" nil t)
+           (point))
+         (save-excursion
+           (re-search-forward "^[[:word:] ]+\\.$" nil t)
+           (pos-bol)))
+        (while (search-forward "(framework directory)" nil t)
+          (delete-line))
+        (split-string (buffer-substring-no-properties
+                       (point-min) (point-max))))
+    '("/usr/include" "/usr/include/local")))
 
 ;;; subr.el ends here
diff --git a/test/lisp/subr-tests.el b/test/lisp/subr-tests.el
index 5b17c598efa..4f566698a08 100644
--- a/test/lisp/subr-tests.el
+++ b/test/lisp/subr-tests.el
@@ -1385,34 +1385,49 @@ subr--subst-char-in-string
 
 (ert-deftest subr-tests-internal--c-header-file-path ()
   (should (seq-every-p #'stringp (internal--c-header-file-path)))
-  (should (member "/usr/include" (internal--c-header-file-path)))
-  (should (equal (internal--c-header-file-path)
-                 (delete-dups (internal--c-header-file-path))))
   ;; Return a meaningful result even if calling some compiler fails.
   (cl-letf (((symbol-function 'call-process)
              (lambda (_program &optional _infile _destination _display &rest _args) 1)))
-    (should (seq-every-p #'stringp (internal--c-header-file-path)))
-    (should (member "/usr/include" (internal--c-header-file-path)))
-    (should (equal (internal--c-header-file-path)
-                   (delete-dups (internal--c-header-file-path))))))
+    (should (seq-every-p #'stringp (internal--c-header-file-path)))))
 
 (ert-deftest subr-tests-internal--c-header-file-path/gcc-mocked ()
-  ;; Handle empty values of "gcc -print-multiarch".
-  (cl-letf (((symbol-function 'call-process)
-             (lambda (_program &optional _infile _destination _display &rest args)
-               (when (equal (car args) "-print-multiarch")
-                 (insert "\n") 0))))
-    (should (member "/usr/include" (internal--c-header-file-path))))
-  ;; Handle single values of "gcc -print-multiarch".
-  (cl-letf (((symbol-function 'call-process)
-             (lambda (_program &optional _infile _destination _display &rest args)
-               (when (equal (car args) "-print-multiarch")
-                 (insert "x86_64-linux-gnu\n") 0))))
-    (should (member "/usr/include/x86_64-linux-gnu" (internal--c-header-file-path)))))
+  (cl-letf (((symbol-function 'executable-find)
+             (lambda (cmd &optional _remote)
+               (pcase cmd
+                 ("gcc" t)
+                 (_ nil))))
+            ((symbol-function 'call-process)
+             (lambda (_program &optional _infile _destination _display &rest _args)
+               (insert "\
+Using built-in specs.
+COLLECT_GCC=gcc-14
+Target: aarch64-apple-darwin24
+[[[...Emacs test omits some verbose junk from the output here...]]]
+gcc version 14.2.0 (Homebrew GCC 14.2.0_1)
+[[[...more verbose junk...]]]
+#include \"...\" search starts here:
+#include <...> search starts here:
+ /opt/homebrew/Cellar/gcc/14.2.0_1/bin/../lib/gcc/current/gcc/aarch64-apple-darwin24/14/include
+ /opt/homebrew/Cellar/gcc/14.2.0_1/bin/../lib/gcc/current/gcc/aarch64-apple-darwin24/14/include-fixed
+ /Library/Developer/CommandLineTools/SDKs/MacOSX15.sdk/usr/include
+ /Library/Developer/CommandLineTools/SDKs/MacOSX15.sdk/System/Library/Frameworks
+End of search list.
+# 0 \"<stdin>\"
+# 0 \"<built-in>\"
+# 0 \"<command-line>\"
+# 1 \"<stdin>\"\n") 0)))
+    (should (seq-every-p #'stringp (internal--c-header-file-path)))
+    (should (member "/Library/Developer/CommandLineTools/SDKs/MacOSX15.sdk/System/Library/Frameworks"
+                    (internal--c-header-file-path)))
+    (should (= 4 (length (internal--c-header-file-path))))))
 
 (ert-deftest subr-tests-internal--c-header-file-path/clang-mocked ()
   ;; Handle clang 15.0.0 output on macOS 15.2.
-  (cl-letf (((symbol-function 'internal--gcc-is-clang-p) (lambda () t))
+  (cl-letf (((symbol-function 'executable-find)
+             (lambda (cmd &optional _remote)
+               (pcase cmd
+                 ("clang" t)
+                 (_ nil))))
             ((symbol-function 'call-process)
              (lambda (_program &optional _infile _destination _display &rest _args)
                (insert "\
@@ -1439,9 +1454,10 @@ subr-tests-internal--c-header-file-path/clang-mocked
 # 1 \"<built-in>\" 2
 # 1 \"<stdin>\" 2")
                0)))
-    (should (member "/usr/include" (internal--c-header-file-path)))
+    (should (seq-every-p #'stringp (internal--c-header-file-path)))
     (should (member "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/15.0.0/include"
-                    (internal--c-header-file-path)))))
+                    (internal--c-header-file-path)))
+    (should (= 3 (length (internal--c-header-file-path))))))
 
 (provide 'subr-tests)
 ;;; subr-tests.el ends here
-- 
2.47.1

Re: master 888ff3755d4 1/3: New function internal--c-header-file-path

[Stefan Kangas]

Andreas Schwab <schwab@linux-m68k.org> writes:

> On Jan 06 2025, Eli Zaretskii wrote:
>
>> and then look in BUFFER for text that begins with "#include <...>
>> search starts here:" and ends with "End of search list."
>
> Modulo l10n.

I hope the new patch that I just sent can handle l10n.

Please take a look.

Re: master 888ff3755d4 1/3: New function internal--c-header-file-path

[Björn Bidar]

Stefan Kangas <stefankangas@gmail.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> Stefan,
>>
>> Thanks for adding this function.  However, the method it uses to find
>> the include directories is incorrect and unportable.  On most systems
>> it will produce the default "/usr/include", which is most probably
>> wrong.
>>
>> The way to ask GCC to show the list of directories where it looks for
>> header files is like this:
>>
>>    (call-process "gcc" nil BUFFER nil "-v" "-E" "-")
>>
>> and then look in BUFFER for text that begins with "#include <...>
>> search starts here:" and ends with "End of search list."  What's
>> in-between is the list of include directories, one directory per line,
>> which GCC searches for header file, in the order it searches them.
>
> Thanks for helping improve this.  How about the attached patch?
>
> From c8aac6531fbf228d004b15498fd0758e080c784e Mon Sep 17 00:00:00 2001
> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Mon, 6 Jan 2025 22:37:51 +0100
> Subject: [PATCH] Make internal--c-header-file-path more portable
>
> * lisp/subr.el (internal--c-header-file-path): Make more portable.
> Problem reported by Eli Zaretskii <eliz@gnu.org>.
> * test/lisp/subr-tests.el
> (subr-tests-internal--c-header-file-path)
> (subr-tests-internal--c-header-file-path/gcc-mocked)
> (subr-tests-internal--c-header-file-path/clang-mocked): Adjust tests.
> ---
>  lisp/subr.el            | 57 +++++++++++++++------------------------
>  test/lisp/subr-tests.el | 60 ++++++++++++++++++++++++++---------------
>  2 files changed, 59 insertions(+), 58 deletions(-)
>
> diff --git a/lisp/subr.el b/lisp/subr.el
> index 5be8d8f52d4..e0f5542a011 100644
> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -7574,41 +7574,26 @@ internal--c-header-file-path
>    ;; See also (Bug#10702):
>    ;; cc-search-directories, semantic-c-dependency-system-include-path,
>    ;; semantic-gcc-setup
> -  (delete-dups
> -   (let ((base '("/usr/include" "/usr/local/include")))
> -     (cond ((or (internal--gcc-is-clang-p)
> -                (and (executable-find "clang")
> -                     (not (executable-find "gcc"))))
> -            ;; This is either macOS, or a system with clang only.
> -            (with-temp-buffer
> -              (ignore-errors
> -                (call-process (if (internal--gcc-is-clang-p) "gcc" "clang")
> -                              nil t nil
> -                              "-v" "-E" "-"))
> -              (goto-char (point-min))
> -              (narrow-to-region
> -               (save-excursion
> -                 (re-search-forward
> -                  "^#include <\\.\\.\\.> search starts here:\n" nil t)
> -                 (point))
> -               (save-excursion
> -                 (re-search-forward "^End of search list.$" nil t)
> -                 (pos-bol)))
> -              (while (search-forward "(framework directory)" nil t)
> -                (delete-line))
> -              (append base
> -                      (reverse
> -                       (split-string (buffer-substring-no-properties
> -                                      (point-min) (point-max)))))))
> -           ;; Prefer GCC.
> -           ((let ((arch (with-temp-buffer
> -                          (when (eq 0 (ignore-errors
> -                                        (call-process "gcc" nil '(t nil) nil
> -                                                      "-print-multiarch")))
> -                            (goto-char (point-min))
> -                            (buffer-substring (point) (line-end-position))))))
> -              (if (zerop (length arch))
> -                  base
> -                (append base (list (expand-file-name arch "/usr/include"))))))))))
> +  (if (or (executable-find "gcc")
> +          (executable-find "clang"))
why not use if-let here and fallback to cc?
> +      (with-temp-buffer
> +        (ignore-errors
> +          (call-process (if (executable-find "gcc") "gcc" "clang")
> +                        nil t nil
> +                        "-v" "-E" "-"))
> +        (goto-char (point-min))
> +        (narrow-to-region
> +         (save-excursion
> +           (re-search-forward
> +            "^#include <\\.\\.\\.> [[:word:] ]+:\n" nil t)
> +           (point))
> +         (save-excursion
> +           (re-search-forward "^[[:word:] ]+\\.$" nil t)
> +           (pos-bol)))
> +        (while (search-forward "(framework directory)" nil t)
> +          (delete-line))
> +        (split-string (buffer-substring-no-properties
> +                       (point-min) (point-max))))
> +    '("/usr/include" "/usr/include/local")))

Re: master 888ff3755d4 1/3: New function internal--c-header-file-path

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Mon, 6 Jan 2025 15:44:34 -0600
> Cc: emacs-devel@gnu.org
> 
> > The way to ask GCC to show the list of directories where it looks for
> > header files is like this:
> >
> >    (call-process "gcc" nil BUFFER nil "-v" "-E" "-")
> >
> > and then look in BUFFER for text that begins with "#include <...>
> > search starts here:" and ends with "End of search list."  What's
> > in-between is the list of include directories, one directory per line,
> > which GCC searches for header file, in the order it searches them.
> 
> Thanks for helping improve this.  How about the attached patch?

LGTM, thanks.  But I agree with Björn that we should also try "cc".