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

[Philip Kaludercic <philipk@posteo.net>]

Joseph Turner <joseph@breatheoutbreathe.in> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Joseph Turner <joseph@breatheoutbreathe.in>
>>> Cc: Eli Zaretskii <eliz@gnu.org>, 63337@debbugs.gnu.org
>>> 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

Ok, do you have a few example repositories that we can use to test edge-cases?