Subject: bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements

From: Joseph Turner <[email-masked]>
Date: Tue, 09 May 2023 16:49:24 -0700

Eli Zaretskii <[email-masked]> writes:

>> From: Joseph Turner <[email-masked]>
>> Cc: Eli Zaretskii <[email-masked]>, [email-masked]
>> Date: Mon, 08 May 2023 12:05:51 -0700
>>
>> Note about the following two lines:
>>
>> + (file-path (expand-file-name file (package-desc-dir pkg-desc)))
>> + (default-directory (expand-file-name (file-name-directory file-path)))
>>
>> (package-desc-dir pkg-desc) may return a relative path with or without a
>> directory, e.g. "doc/manual.org" or "manual.org". In the latter case,
>> (file-name-directory "manual.org") would return `nil' and
>> (expand-file-name nil) would signal an error.
>>
>> Therefore, in the `file-path' `let'-binding, we first expand the return
>> value of (package-desc-dir pkg-desc) to ensure that it contains a directory.
>
> Please don't use "path" for anything that is not a PATH-style list of
> directory: the GNU Coding Standards frown on such usage.  We use
> file-name instead.  For the same reasons, please don't give your
> variables names that include "path" unless they are lists of
> directories.

Good to know, thank you! I changed `file-path' to `file-name'.

>> --- a/lisp/emacs-lisp/package-vc.el
>> +++ b/lisp/emacs-lisp/package-vc.el
>> @@ -376,14 +376,17 @@ Package specs are loaded from trusted package archives."
>>  FILE can be an Org file, indicated by its \".org\" extension,
>>  otherwise it's assumed to be an Info file."
>>    (let* ((pkg-name (package-desc-name pkg-desc))
>> -         (default-directory (package-desc-dir pkg-desc))
>> +         (file-path (expand-file-name file (package-desc-dir pkg-desc)))
>> +         ;; `let'-bind `default-directory' to the directory containing the .org or .info FILE
>> +         ;; so that makeinfo can resolve relative @include statements in the docs directory.
>> +         (default-directory (expand-file-name (file-name-directory file-path)))
>
> There should be no reason to call expand-file-name in the last line,
> since the argument of file-name-directory is already expanded.

Good catch! Fixed.

> Also, please make the comment lines shorter, preferably less than 75
> columns.

Done.

Thank you!!

Joseph

>From c670f47ef55b41265c064a2d4ab1e56c46e57272 Mon Sep 17 00:00:00 2001
From: Joseph Turner <[email-masked]>
Date: Sat, 6 May 2023 14:49:43 -0700
Subject: [PATCH] Fix: (package-vc--build-documentation) Relative @include
 statements

---
 lisp/emacs-lisp/package-vc.el | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
index 95e12fc829a..efcfd635e98 100644
--- a/lisp/emacs-lisp/package-vc.el
+++ b/lisp/emacs-lisp/package-vc.el
@@ -377,14 +377,18 @@ Package specs are loaded from trusted package archives."
 FILE can be an Org file, indicated by its \".org\" extension,
 otherwise it's assumed to be an Info file."
   (let* ((pkg-name (package-desc-name pkg-desc))
-         (default-directory (package-desc-dir pkg-desc))
+         (file-name (expand-file-name file (package-desc-dir pkg-desc)))
+         ;; `let'-bind `default-directory' to the directory containing
+         ;; the .org or .info FILE so that makeinfo can resolve
+         ;; relative @include statements in the docs directory.
+         (default-directory (file-name-directory file-name))
          (output (expand-file-name (format "%s.info" pkg-name)))
          clean-up)
     (when (string-match-p "\\.org\\'" file)
       (require 'ox)
       (require 'ox-texinfo)
       (with-temp-buffer
-        (insert-file-contents file)
+        (insert-file-contents file-name)
         (setq file (make-temp-file "ox-texinfo-"))
         (org-export-to-file 'texinfo file)
         (setq clean-up t)))
-- 
2.39.2