bug#62099: 29.0.60; Intentional change to *Help* source code links?

bug#62099: 29.0.60; Intentional change to *Help* source code links?

[Stephen Berman]

I recently noticed a change in the appearance of links to source files
in *Help* buffers for variable and functions definitions in out-of-tree
builds of Emacs.  In Emacs 28 -Q, the first line of *Help* after typing
e.g. `C-h f map-into RET' looks like this on my system:

  map-into is a byte-compiled Lisp function in ‘map.el’.

The link is ‘map.el’.  In current Emacs 29 and master, it looks like
this on my system:

  map-into is a byte-compiled Lisp function in
  ‘/../home/steve/src/emacs/emacs-29/lisp/emacs-lisp/map.el’.

In Emacs 28, the *Help* buffer generated by `C-h v
emacs-repository-version RET' starts like this:

  emacs-repository-version is a variable defined in ‘/datadisk/steve/src/emacs/emacs-28/lisp/version.el’.

In current Emacs 29 and master, it looks like this my system:

emacs-repository-version is a variable defined in ‘/../home/steve/src/emacs/emacs-29/lisp/version.el’.

I regularly build emacs out-of-tree, but I made an in-tree test build of
emacs-29, and there the *Help* buffers look like those in Emacs 28.  I
also note that the files names '/home/steve/src/emacs/...' are symlinks
to '/datadisk/steve/src/emacs/...'.  That the symlinks are prepended
with '/..' in *Help* seems odd.

I bisected this change to the following commit:

43b0210f83c38fb91cfcfc5a2d4a8c3131331476 is the first bad commit
commit 43b0210f83c38fb91cfcfc5a2d4a8c3131331476
Author: Lars Ingebrigtsen <larsi@gnus.org>
Date:   Thu Jun 2 13:52:58 2022 +0200

    Fix out-of-tree build problems with loaddefs.el
    
    * lisp/Makefile.in ($(lisp)/loaddefs.el): Use the new function.
    
    * lisp/emacs-lisp/loaddefs-gen.el (loaddefs-generate): Pass in
    whether to inhibit a partial build (to make the code more general).
    (loaddefs-generate--emacs-batch): Add a new function specially for
    the Emacs build that has the special rules needed.  (This also
    fixes out-of-tree builds.)
    loaddefs-generate-batch can be used in general for packages etc.
    (loaddefs-generate-batch): Remove the special code for Emacs builds.

(Why it took me nine months to consciously register the effect of this
commit is a mystery to me, since I use `C-h f' etc. daily.)

While there is no bug report associated with this commit, I found a
thread in emacs-devel that leads up to this commit
(https://lists.gnu.org/archive/html/emacs-devel/2022-06/msg00088.html).
Nothing in that thread indicates that *Help* links were intentionally
changed, so I assume this was not intentional.

I note that the links work, it's just the appearance that changed.
The Emacs 28 appearance of function definition links seems best to me.
Why variable definition links are absolute file names I don't know;
I'd prefer for them to look like the function definition links.  And in
fact, they do look that that in Emacs 26 and 27:

  emacs-repository-version is a variable defined in ‘version.el’.

(I haven't tried to track down when the appearance of variable
definition links changed.)


In GNU Emacs 29.0.60 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.36, cairo version 1.17.6) of 2023-03-07 built on strobelfs
Repository revision: bd07cec844257ba8ae95b2ab2e66982360576c9d
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12101007
System Description: Linux From Scratch r11.2-332

Configured using:
 'configure -C --with-xwidgets 'CFLAGS=-Og -g3'
 PKG_CONFIG_PATH=/usr/local/lib/pkgconfig:/opt/qt5/lib/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG
SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER WEBP
X11 XDBE XIM XINPUT2 XPM XWIDGETS GTK3 ZLIB

bug#62099: 29.0.60; Intentional change to *Help* source code links?

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Date: Fri, 10 Mar 2023 18:10:49 +0100
> 
> I recently noticed a change in the appearance of links to source files
> in *Help* buffers for variable and functions definitions in out-of-tree
> builds of Emacs.  In Emacs 28 -Q, the first line of *Help* after typing
> e.g. `C-h f map-into RET' looks like this on my system:
> 
>   map-into is a byte-compiled Lisp function in ‘map.el’.
> 
> The link is ‘map.el’.  In current Emacs 29 and master, it looks like
> this on my system:
> 
>   map-into is a byte-compiled Lisp function in
>   ‘/../home/steve/src/emacs/emacs-29/lisp/emacs-lisp/map.el’.
> 
> In Emacs 28, the *Help* buffer generated by `C-h v
> emacs-repository-version RET' starts like this:
> 
>   emacs-repository-version is a variable defined in ‘/datadisk/steve/src/emacs/emacs-28/lisp/version.el’.
> 
> In current Emacs 29 and master, it looks like this my system:
> 
> emacs-repository-version is a variable defined in ‘/../home/steve/src/emacs/emacs-29/lisp/version.el’.
> 
> I regularly build emacs out-of-tree, but I made an in-tree test build of
> emacs-29, and there the *Help* buffers look like those in Emacs 28.  I
> also note that the files names '/home/steve/src/emacs/...' are symlinks
> to '/datadisk/steve/src/emacs/...'.  That the symlinks are prepended
> with '/..' in *Help* seems odd.
> 
> I bisected this change to the following commit:
> 
> 43b0210f83c38fb91cfcfc5a2d4a8c3131331476 is the first bad commit
> commit 43b0210f83c38fb91cfcfc5a2d4a8c3131331476
> Author: Lars Ingebrigtsen <larsi@gnus.org>
> Date:   Thu Jun 2 13:52:58 2022 +0200
> 
>     Fix out-of-tree build problems with loaddefs.el
>     
>     * lisp/Makefile.in ($(lisp)/loaddefs.el): Use the new function.
>     
>     * lisp/emacs-lisp/loaddefs-gen.el (loaddefs-generate): Pass in
>     whether to inhibit a partial build (to make the code more general).
>     (loaddefs-generate--emacs-batch): Add a new function specially for
>     the Emacs build that has the special rules needed.  (This also
>     fixes out-of-tree builds.)
>     loaddefs-generate-batch can be used in general for packages etc.
>     (loaddefs-generate-batch): Remove the special code for Emacs builds.
> 
> (Why it took me nine months to consciously register the effect of this
> commit is a mystery to me, since I use `C-h f' etc. daily.)
> 
> While there is no bug report associated with this commit, I found a
> thread in emacs-devel that leads up to this commit
> (https://lists.gnu.org/archive/html/emacs-devel/2022-06/msg00088.html).
> Nothing in that thread indicates that *Help* links were intentionally
> changed, so I assume this was not intentional.
> 
> I note that the links work, it's just the appearance that changed.
> The Emacs 28 appearance of function definition links seems best to me.
> Why variable definition links are absolute file names I don't know;
> I'd prefer for them to look like the function definition links.  And in
> fact, they do look that that in Emacs 26 and 27:
> 
>   emacs-repository-version is a variable defined in ‘version.el’.
> 
> (I haven't tried to track down when the appearance of variable
> definition links changed.)

Thank you for your detailed report.  However, I'm not sure I
understand the bottom line: is there a bug here or isn't there?  If
the links work, and the only issue is how they look, why is that a
problem?

bug#62099: 29.0.60; Intentional change to *Help* source code links?

[Stephen Berman]

On Sat, 11 Mar 2023 14:11:36 +0200 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Date: Fri, 10 Mar 2023 18:10:49 +0100
>> 
>> I recently noticed a change in the appearance of links to source files
>> in *Help* buffers for variable and functions definitions in out-of-tree
>> builds of Emacs.  In Emacs 28 -Q, the first line of *Help* after typing
>> e.g. `C-h f map-into RET' looks like this on my system:
>> 
>>   map-into is a byte-compiled Lisp function in ‘map.el’.
>> 
>> The link is ‘map.el’.  In current Emacs 29 and master, it looks like
>> this on my system:
>> 
>>   map-into is a byte-compiled Lisp function in
>>   ‘/../home/steve/src/emacs/emacs-29/lisp/emacs-lisp/map.el’.
>> 
>> In Emacs 28, the *Help* buffer generated by `C-h v
>> emacs-repository-version RET' starts like this:
>> 
>>   emacs-repository-version is a variable defined in ‘/datadisk/steve/src/emacs/emacs-28/lisp/version.el’.
>> 
>> In current Emacs 29 and master, it looks like this my system:
>> 
>> emacs-repository-version is a variable defined in ‘/../home/steve/src/emacs/emacs-29/lisp/version.el’.
>> 
>> I regularly build emacs out-of-tree, but I made an in-tree test build of
>> emacs-29, and there the *Help* buffers look like those in Emacs 28.  I
>> also note that the files names '/home/steve/src/emacs/...' are symlinks
>> to '/datadisk/steve/src/emacs/...'.  That the symlinks are prepended
>> with '/..' in *Help* seems odd.
>> 
>> I bisected this change to the following commit:
>> 
>> 43b0210f83c38fb91cfcfc5a2d4a8c3131331476 is the first bad commit
>> commit 43b0210f83c38fb91cfcfc5a2d4a8c3131331476
>> Author: Lars Ingebrigtsen <larsi@gnus.org>
>> Date:   Thu Jun 2 13:52:58 2022 +0200
>> 
>>     Fix out-of-tree build problems with loaddefs.el
>>     
>>     * lisp/Makefile.in ($(lisp)/loaddefs.el): Use the new function.
>>     
>>     * lisp/emacs-lisp/loaddefs-gen.el (loaddefs-generate): Pass in
>>     whether to inhibit a partial build (to make the code more general).
>>     (loaddefs-generate--emacs-batch): Add a new function specially for
>>     the Emacs build that has the special rules needed.  (This also
>>     fixes out-of-tree builds.)
>>     loaddefs-generate-batch can be used in general for packages etc.
>>     (loaddefs-generate-batch): Remove the special code for Emacs builds.
>> 
>> (Why it took me nine months to consciously register the effect of this
>> commit is a mystery to me, since I use `C-h f' etc. daily.)
>> 
>> While there is no bug report associated with this commit, I found a
>> thread in emacs-devel that leads up to this commit
>> (https://lists.gnu.org/archive/html/emacs-devel/2022-06/msg00088.html).
>> Nothing in that thread indicates that *Help* links were intentionally
>> changed, so I assume this was not intentional.
>> 
>> I note that the links work, it's just the appearance that changed.
>> The Emacs 28 appearance of function definition links seems best to me.
>> Why variable definition links are absolute file names I don't know;
>> I'd prefer for them to look like the function definition links.  And in
>> fact, they do look that that in Emacs 26 and 27:
>> 
>>   emacs-repository-version is a variable defined in ‘version.el’.
>> 
>> (I haven't tried to track down when the appearance of variable
>> definition links changed.)
>
> Thank you for your detailed report.  However, I'm not sure I
> understand the bottom line: is there a bug here or isn't there?  If
> the links work, and the only issue is how they look, why is that a
> problem?

It does seem to be an aesthetic issue, not a functional one, but if it
is an unintended consequence of the above commit, as it seems to be, and
if it's also an undesirable change, as I think it is (the '/..' prefix
is at least confusing), then I think it qualifies as a bug, though not a
high-priority one.  If you agree, then I would ask for it to be left
open, in case someone wants to try fixing it.  I might try myself,
though I also have more pressing things to do.

Steve Berman

bug#62099: 29.0.60; Intentional change to *Help* source code links?

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: 62099@debbugs.gnu.org
> Date: Sat, 11 Mar 2023 13:36:58 +0100
> 
> > Thank you for your detailed report.  However, I'm not sure I
> > understand the bottom line: is there a bug here or isn't there?  If
> > the links work, and the only issue is how they look, why is that a
> > problem?
> 
> It does seem to be an aesthetic issue, not a functional one, but if it
> is an unintended consequence of the above commit, as it seems to be, and
> if it's also an undesirable change, as I think it is (the '/..' prefix
> is at least confusing), then I think it qualifies as a bug, though not a
> high-priority one.  If you agree, then I would ask for it to be left
> open, in case someone wants to try fixing it.  I might try myself,
> though I also have more pressing things to do.

Maybe Lars (CC'ed) can tell if this is intended or not.

bug#62099: 29.0.60; Intentional change to *Help* source code links?

[Stephen Berman]

On Sat, 11 Mar 2023 13:36:58 +0100 Stephen Berman <stephen.berman@gmx.net> wrote:

> On Sat, 11 Mar 2023 14:11:36 +0200 Eli Zaretskii <eliz@gnu.org> wrote:
>
>>> From: Stephen Berman <stephen.berman@gmx.net>
>>> Date: Fri, 10 Mar 2023 18:10:49 +0100
>>> 
>>> I recently noticed a change in the appearance of links to source files
>>> in *Help* buffers for variable and functions definitions in out-of-tree
>>> builds of Emacs.  In Emacs 28 -Q, the first line of *Help* after typing
>>> e.g. `C-h f map-into RET' looks like this on my system:
>>> 
>>>   map-into is a byte-compiled Lisp function in ‘map.el’.
>>> 
>>> The link is ‘map.el’.  In current Emacs 29 and master, it looks like
>>> this on my system:
>>> 
>>>   map-into is a byte-compiled Lisp function in
>>>   ‘/../home/steve/src/emacs/emacs-29/lisp/emacs-lisp/map.el’.
>>> 
>>> In Emacs 28, the *Help* buffer generated by `C-h v
>>> emacs-repository-version RET' starts like this:
>>> 
>>>   emacs-repository-version is a variable defined in ‘/datadisk/steve/src/emacs/emacs-28/lisp/version.el’.
>>> 
>>> In current Emacs 29 and master, it looks like this my system:
>>> 
>>> emacs-repository-version is a variable defined in ‘/../home/steve/src/emacs/emacs-29/lisp/version.el’.
>>> 
>>> I regularly build emacs out-of-tree, but I made an in-tree test build of
>>> emacs-29, and there the *Help* buffers look like those in Emacs 28.  I
>>> also note that the files names '/home/steve/src/emacs/...' are symlinks
>>> to '/datadisk/steve/src/emacs/...'.  That the symlinks are prepended
>>> with '/..' in *Help* seems odd.
>>> 
>>> I bisected this change to the following commit:
>>> 
>>> 43b0210f83c38fb91cfcfc5a2d4a8c3131331476 is the first bad commit
>>> commit 43b0210f83c38fb91cfcfc5a2d4a8c3131331476
>>> Author: Lars Ingebrigtsen <larsi@gnus.org>
>>> Date:   Thu Jun 2 13:52:58 2022 +0200
>>> 
>>>     Fix out-of-tree build problems with loaddefs.el
>>>     
>>>     * lisp/Makefile.in ($(lisp)/loaddefs.el): Use the new function.
>>>     
>>>     * lisp/emacs-lisp/loaddefs-gen.el (loaddefs-generate): Pass in
>>>     whether to inhibit a partial build (to make the code more general).
>>>     (loaddefs-generate--emacs-batch): Add a new function specially for
>>>     the Emacs build that has the special rules needed.  (This also
>>>     fixes out-of-tree builds.)
>>>     loaddefs-generate-batch can be used in general for packages etc.
>>>     (loaddefs-generate-batch): Remove the special code for Emacs builds.
>>> 
>>> (Why it took me nine months to consciously register the effect of this
>>> commit is a mystery to me, since I use `C-h f' etc. daily.)
>>> 
>>> While there is no bug report associated with this commit, I found a
>>> thread in emacs-devel that leads up to this commit
>>> (https://lists.gnu.org/archive/html/emacs-devel/2022-06/msg00088.html).
>>> Nothing in that thread indicates that *Help* links were intentionally
>>> changed, so I assume this was not intentional.
>>> 
>>> I note that the links work, it's just the appearance that changed.
>>> The Emacs 28 appearance of function definition links seems best to me.
>>> Why variable definition links are absolute file names I don't know;
>>> I'd prefer for them to look like the function definition links.  And in
>>> fact, they do look that that in Emacs 26 and 27:
>>> 
>>>   emacs-repository-version is a variable defined in ‘version.el’.
>>> 
>>> (I haven't tried to track down when the appearance of variable
>>> definition links changed.)
>>
>> Thank you for your detailed report.  However, I'm not sure I
>> understand the bottom line: is there a bug here or isn't there?  If
>> the links work, and the only issue is how they look, why is that a
>> problem?
>
> It does seem to be an aesthetic issue, not a functional one, but if it
> is an unintended consequence of the above commit, as it seems to be, and
> if it's also an undesirable change, as I think it is (the '/..' prefix
> is at least confusing), then I think it qualifies as a bug, though not a
> high-priority one.  If you agree, then I would ask for it to be left
> open, in case someone wants to try fixing it.  I might try myself,
> though I also have more pressing things to do.

A week or so ago I noticed that source links in *Help* buffers no longer
have the unexpected appearance I described above when I start Emacs with
-Q: the links now just show the source file base name plus extension.
However, I still do see the problem when I start Emacs with my init
file.

Then I rebuilt Emacs from both 43b0210f83c and the immediately preceding
commit c7b7c9d40f9, and in both builds, with -Q, *Help* source links of
functions look like in Emacs 28 (i.e. just base name plus extension),
but while links of variables in 43b0210f83c also show just base name
plus extension (as in Emacs 27), in c7b7c9d40f9 they show the full
absolute file name (as in Emacs 28).

But with my init file, in 43b0210f83c the *Help* links of both functions
and variables begin with '/..' as described above, while in c7b7c9d40f9
the *Help* appear the same as with -Q.  So this reaffirms that the
problem is due to 43b0210f83c.  Perhaps I was mistaken in my OP in
reporting the problem as also occurring with -Q (though I have a clear
memory of testing with -Q).  Be that as it may, I have now bisected my
init file and found the culprit:

(add-to-list 'load-path "~/.emacs.d/srb")

The directory "srb" contains files of my personal Emacs customization
code.  I then symlinked that directory to a location outside of
~/.emacs.d and changed the add-to-list sexp accordingly, and with that
change, the *Help* links in 43b0210f83c (and also in the current code
base) appear the same as with -Q.

In fact, it's not necessary to have an existing directory under
~/.emacs.d; I created ~/.emacs containing just the following line:

(add-to-list 'load-path "~/.emacs.d/test")

where there is no file (or directory) "test" under ~/.emacs.d, and with
this init file the *Help* links are shown starting with '/..' in
43b0210f83c (and the current code base).  I note again that this only
happens with an out-of-tree build.  I haven't yet been able to determine
what part of the change in 43b0210f83c causes this issue.

Steve Berman

bug#62099: 29.0.60; Intentional change to *Help* source code links?

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: 62099@debbugs.gnu.org, Lars Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 11 May 2023 23:14:37 +0200
> 
> In fact, it's not necessary to have an existing directory under
> ~/.emacs.d; I created ~/.emacs containing just the following line:
> 
> (add-to-list 'load-path "~/.emacs.d/test")
> 
> where there is no file (or directory) "test" under ~/.emacs.d, and with
> this init file the *Help* links are shown starting with '/..' in
> 43b0210f83c (and the current code base).  I note again that this only
> happens with an out-of-tree build.  I haven't yet been able to determine
> what part of the change in 43b0210f83c causes this issue.

Well, if you could find out the offending code, it will be
appreciated.

Thanks.

bug#62099: 29.0.60; Intentional change to *Help* source code links?

[Stephen Berman]

On Fri, 12 May 2023 08:51:22 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Cc: 62099@debbugs.gnu.org, Lars Ingebrigtsen <larsi@gnus.org>
>> Date: Thu, 11 May 2023 23:14:37 +0200
>>
>> In fact, it's not necessary to have an existing directory under
>> ~/.emacs.d; I created ~/.emacs containing just the following line:
>>
>> (add-to-list 'load-path "~/.emacs.d/test")
>>
>> where there is no file (or directory) "test" under ~/.emacs.d, and with
>> this init file the *Help* links are shown starting with '/..' in
>> 43b0210f83c (and the current code base).  I note again that this only
>> happens with an out-of-tree build.  I haven't yet been able to determine
>> what part of the change in 43b0210f83c causes this issue.
>
> Well, if you could find out the offending code, it will be
> appreciated.

43b0210f83c only changed how loaddefs.el is generated, and for
out-of-tree builds, also how the names of the files under lisp are
represented in loaddefs.el.  Prior to the change, in out-of-tree builds
these file names appeared the same as those in in-tree builds, e.g.:

;;; Generated autoloads from play/5x5.el

But after 43b0210f83c, in my out-of-tree builds, they now look like
this:

;;; Generated autoloads from ../../../../../../home/steve/src/emacs/test-29/lisp/play/5x5.el

What I guess happens is that some code (but which?  I haven't found it)
truncates such file names to produce standard absolute file names, which
are then further truncated by help-fns-short-filename to produce the
links in *Help* buffers.  If this is right, then it works with -Q, but
when the init file adds a file or directory under ~/.emacs.d to
load-path, this somehow breaks the truncation, so
help-fns-short-filename fails to produce the desired link appearance.
But I have no idea how this breakage happens and why that initialization
triggers it.

I traced the problematic appearance of links in *Help* through the call
chain describe-function -> describe-function-1 ->
help-fns-function-description-header -> find-lisp-object-file-name ->
symbol-file, and symbol-file gets the file name from load-history.  In a
fresh Emacs, started with a triggering init file, load-history contains
only standard absolute file names, but when I type `C-h f' and then
evaluate load-history again, it now contains problematic files names
like "/../home/steve/src/emacs/test-29/lisp/help-fns.elc".  I set a
breakpoint in gdb on build_load_history, but when I hit it after typing
`C-h f' and started stepping through the function, the first opportunity
I had to evaluate its argument `filename' already showed the problematic
form.  Then I set a breakpoint on readevalloop, the caller of
build_load_history in lread.c, but again, the first opportunity I had to
evaluate readevalloop's argument `sourcename', which is passed to
build_load_history, already showed the problematic form.  I don't know
where else to look to try and find out how such forms get into
load-history.

If the reason for the aberrant file names in load-history can't be
found, or if avoiding them has more serious disadvantageous, I see two
alternatives: one is to do nothing, since this is apparently only a
cosmetic bug and there's an easy way to avoid triggering it, namely, by
not adding files under ~/.emacs.d to load-path, for which it suffices to
leave such files where they are and just symlink them to a location
outside of ~/.emacs.d and add that location to load-path; that's what
I've currently done in my init.  Or as a workaround fix in Emacs,
adjusting help-fns-short-filename as follows seems to do the trick, and
AFAICT has no unwanted side-effects:

diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index 1966193d1a7..9204f45210c 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -954,7 +954,10 @@ help-fns--mention-shortdoc-groups
         (goto-char (point-max))))))

 (defun help-fns-short-filename (filename)
-  (let* ((abbrev (abbreviate-file-name filename))
+  (let* ((filename (if (string-match "^/\\.\\." filename)
+ 		       (substring filename 3)
+                     filename))
+         (abbrev (abbreviate-file-name filename))
          (short abbrev))
     (dolist (dir load-path)
       (let ((rel (file-relative-name filename dir)))

(This could, of course, also be used as advice in the init file, as
another alternative for individual users.)

Steve Berman

bug#62099: 29.0.60; Intentional change to *Help* source code links?

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: 62099@debbugs.gnu.org,  larsi@gnus.org
> Date: Sun, 14 May 2023 18:18:59 +0200
> 
> > Well, if you could find out the offending code, it will be
> > appreciated.
> 
> 43b0210f83c only changed how loaddefs.el is generated, and for
> out-of-tree builds, also how the names of the files under lisp are
> represented in loaddefs.el.  Prior to the change, in out-of-tree builds
> these file names appeared the same as those in in-tree builds, e.g.:
> 
> ;;; Generated autoloads from play/5x5.el
> 
> But after 43b0210f83c, in my out-of-tree builds, they now look like
> this:
> 
> ;;; Generated autoloads from ../../../../../../home/steve/src/emacs/test-29/lisp/play/5x5.el

Isn't the above difference the root cause, right there?  Why should
the output in loaddefs.el depend on whether this is an in-tree or
out-of-tree build?  Eventually, when the produced loaddefs.el is
installed, the *.el files from the tree and the *.elc files from both
the tree and out-of-tree will end up in the same directory, so there
should be no ../../../..  etc. in the file names recorded in
loaddefs.el, they should be all relative to <source-directory>/lisp,
no?  It seems like loaddefs-gen.el does this instead:

            (let* ((relfile (file-relative-name
                             (cadar section)
                             (file-name-directory loaddefs-file)))
                   (head (concat "\n\f\n;;; Generated autoloads from "
                                 relfile "\n\n")))

which uses the wrong directory as the second arg of
file-relative-name.

Am I missing something?

Or maybe step through the code in autoloads.el and see how it succeeds
in computing the correct relative file name even in out-of-tree
builds, and let's take it from there.

> I traced the problematic appearance of links in *Help* through the call
> chain describe-function -> describe-function-1 ->
> help-fns-function-description-header -> find-lisp-object-file-name ->
> symbol-file, and symbol-file gets the file name from load-history.  In a
> fresh Emacs, started with a triggering init file, load-history contains
> only standard absolute file names, but when I type `C-h f' and then
> evaluate load-history again, it now contains problematic files names
> like "/../home/steve/src/emacs/test-29/lisp/help-fns.elc".  I set a
> breakpoint in gdb on build_load_history, but when I hit it after typing
> `C-h f' and started stepping through the function, the first opportunity
> I had to evaluate its argument `filename' already showed the problematic
> form.  Then I set a breakpoint on readevalloop, the caller of
> build_load_history in lread.c, but again, the first opportunity I had to
> evaluate readevalloop's argument `sourcename', which is passed to
> build_load_history, already showed the problematic form.  I don't know
> where else to look to try and find out how such forms get into
> load-history.

It is possible that these are computed by temacs as part of the build,
so the dumped Emacs gets them already cooked.

However, I think we should concentrate on the first place with the
deviation, which I think is loaddefs.el.

bug#62099: 29.0.60; Intentional change to *Help* source code links?

[Stephen Berman]

On Sun, 14 May 2023 20:41:20 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Cc: 62099@debbugs.gnu.org,  larsi@gnus.org
>> Date: Sun, 14 May 2023 18:18:59 +0200
>>
>> > Well, if you could find out the offending code, it will be
>> > appreciated.
>>
>> 43b0210f83c only changed how loaddefs.el is generated, and for
>> out-of-tree builds, also how the names of the files under lisp are
>> represented in loaddefs.el.  Prior to the change, in out-of-tree builds
>> these file names appeared the same as those in in-tree builds, e.g.:
>>
>> ;;; Generated autoloads from play/5x5.el
>>
>> But after 43b0210f83c, in my out-of-tree builds, they now look like
>> this:
>>
>> ;;; Generated autoloads from
>> ../../../../../../home/steve/src/emacs/test-29/lisp/play/5x5.el
>
> Isn't the above difference the root cause, right there?  Why should
> the output in loaddefs.el depend on whether this is an in-tree or
> out-of-tree build?  Eventually, when the produced loaddefs.el is
> installed, the *.el files from the tree and the *.elc files from both
> the tree and out-of-tree will end up in the same directory, so there
> should be no ../../../..  etc. in the file names recorded in
> loaddefs.el, they should be all relative to <source-directory>/lisp,
> no?

I'm not sure I understand: there are no *.elc files in the out-of-tree
build directory, all byte-compiled files reside in the source tree
together with the *.el files.  Also, FWIW I do not install my Emacs
builds and haven't tested whether the problematic file names appear in
an installed build.

>      It seems like loaddefs-gen.el does this instead:
>
>             (let* ((relfile (file-relative-name
>                              (cadar section)
>                              (file-name-directory loaddefs-file)))
>                    (head (concat "\n\f\n;;; Generated autoloads from "
>                                  relfile "\n\n")))
>
> which uses the wrong directory as the second arg of
> file-relative-name.
>
> Am I missing something?

Well, for one thing this part of the code was not touched by
43b0210f83c.  Also, while I haven't found a way to step through
loaddefs-generate while loaddefs.el is being generated, since that's
part of the build process, note that evaluating the following sexp:

(file-relative-name "/datadisk/steve/src/emacs/test-29/lisp/play/5x5.el" "~/build/test-29/lisp/play")

produces the same number of "../" as appear in loaddefs.el:

"../../../../../../datadisk/steve/src/emacs/test-29/lisp/play/5x5.el"

The difference is that loaddefs.el has "home" instead of "datadisk".
But on my system, ~/src is a symlink to /datadisk/steve/src.  Perhaps
this an irrelevant coincidence, but I found it striking in this context.

> Or maybe step through the code in autoloads.el and see how it succeeds
> in computing the correct relative file name even in out-of-tree
> builds, and let's take it from there.

Do you mean autoload.el?  But this file was obsoleted in commit
22a2ad13f50 not long after commit 43b0210f83c and I believe
loaddefs-gen.el is meant to be its replacement.

>> I traced the problematic appearance of links in *Help* through the call
>> chain describe-function -> describe-function-1 ->
>> help-fns-function-description-header -> find-lisp-object-file-name ->
>> symbol-file, and symbol-file gets the file name from load-history.  In a
>> fresh Emacs, started with a triggering init file, load-history contains
>> only standard absolute file names, but when I type `C-h f' and then
>> evaluate load-history again, it now contains problematic files names
>> like "/../home/steve/src/emacs/test-29/lisp/help-fns.elc".  I set a
>> breakpoint in gdb on build_load_history, but when I hit it after typing
>> `C-h f' and started stepping through the function, the first opportunity
>> I had to evaluate its argument `filename' already showed the problematic
>> form.  Then I set a breakpoint on readevalloop, the caller of
>> build_load_history in lread.c, but again, the first opportunity I had to
>> evaluate readevalloop's argument `sourcename', which is passed to
>> build_load_history, already showed the problematic form.  I don't know
>> where else to look to try and find out how such forms get into
>> load-history.
>
> It is possible that these are computed by temacs as part of the build,
> so the dumped Emacs gets them already cooked.

I don't think this can be the case, since the problematic file names do
not show up in load-history when Emacs is started with -Q but only when
started with a triggering init file, and it's the same temacs in both
cases.

> However, I think we should concentrate on the first place with the
> deviation, which I think is loaddefs.el.

I'm not sure: it seems to me those file names with "../../../../../../"
are intended, in order, as the commit message of 43b0210f83c says, to
"Fix out-of-tree build problems with loaddefs.el".  I don't remember if
the discussion in emacs-devel leading up to that commit clarifies the
matter; I need to read through it again.

Steve Berman

bug#62099: 29.0.60; Intentional change to *Help* source code links?

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: 62099@debbugs.gnu.org,  larsi@gnus.org
> Date: Mon, 15 May 2023 01:11:05 +0200
> 
> On Sun, 14 May 2023 20:41:20 +0300 Eli Zaretskii <eliz@gnu.org> wrote:
> 
> >> 43b0210f83c only changed how loaddefs.el is generated, and for
> >> out-of-tree builds, also how the names of the files under lisp are
> >> represented in loaddefs.el.  Prior to the change, in out-of-tree builds
> >> these file names appeared the same as those in in-tree builds, e.g.:
> >>
> >> ;;; Generated autoloads from play/5x5.el
> >>
> >> But after 43b0210f83c, in my out-of-tree builds, they now look like
> >> this:
> >>
> >> ;;; Generated autoloads from
> >> ../../../../../../home/steve/src/emacs/test-29/lisp/play/5x5.el
> >
> > Isn't the above difference the root cause, right there?  Why should
> > the output in loaddefs.el depend on whether this is an in-tree or
> > out-of-tree build?  Eventually, when the produced loaddefs.el is
> > installed, the *.el files from the tree and the *.elc files from both
> > the tree and out-of-tree will end up in the same directory, so there
> > should be no ../../../..  etc. in the file names recorded in
> > loaddefs.el, they should be all relative to <source-directory>/lisp,
> > no?
> 
> I'm not sure I understand: there are no *.elc files in the out-of-tree
> build directory, all byte-compiled files reside in the source tree
> together with the *.el files.  Also, FWIW I do not install my Emacs
> builds and haven't tested whether the problematic file names appear in
> an installed build.

These aspects don't matter.  My point is that the format of file names
in loaddefs.el should not depend on whether it is an in-tree or
out-of-tree build.  If you don't agree, please explain why.

> >      It seems like loaddefs-gen.el does this instead:
> >
> >             (let* ((relfile (file-relative-name
> >                              (cadar section)
> >                              (file-name-directory loaddefs-file)))
> >                    (head (concat "\n\f\n;;; Generated autoloads from "
> >                                  relfile "\n\n")))
> >
> > which uses the wrong directory as the second arg of
> > file-relative-name.
> >
> > Am I missing something?
> 
> Well, for one thing this part of the code was not touched by
> 43b0210f83c.

I don't think this matters, either.  If the same code produces two
different results in these two revisions, then either the first or the
second argument (or both) to file-relative-name were modified by
commit 43b0210f83c.  I think we should find out which change caused
that, and then try to fix that so that the results are identical.

> Also, while I haven't found a way to step through
> loaddefs-generate while loaddefs.el is being generated, since that's
> part of the build process, note that evaluating the following sexp:
> 
> (file-relative-name "/datadisk/steve/src/emacs/test-29/lisp/play/5x5.el" "~/build/test-29/lisp/play")
> 
> produces the same number of "../" as appear in loaddefs.el:
> 
> "../../../../../../datadisk/steve/src/emacs/test-29/lisp/play/5x5.el"

Why is that important?  Wed don't suspect a bug in file-relative-name,
do we?

Again, my point is that the second argument to file-relative-name
should not be the build directory, it should be the source directory.

> The difference is that loaddefs.el has "home" instead of "datadisk".
> But on my system, ~/src is a symlink to /datadisk/steve/src.  Perhaps
> this an irrelevant coincidence, but I found it striking in this context.

Why striking?  And why do you think it's important for fixing this
problem?

> > Or maybe step through the code in autoloads.el and see how it succeeds
> > in computing the correct relative file name even in out-of-tree
> > builds, and let's take it from there.
> 
> Do you mean autoload.el?  But this file was obsoleted in commit
> 22a2ad13f50 not long after commit 43b0210f83c and I believe
> loaddefs-gen.el is meant to be its replacement.

Sure, but it did the job, didn't it?

You could as an alternative step through loaddefs-gen.el before commit
43b0210f83c, and see what happened there.  The idea is to see what
should be the correct value of relfile and why doesn't the current
code yield that.

> > However, I think we should concentrate on the first place with the
> > deviation, which I think is loaddefs.el.
> 
> I'm not sure: it seems to me those file names with "../../../../../../"
> are intended, in order, as the commit message of 43b0210f83c says, to
> "Fix out-of-tree build problems with loaddefs.el".

What are those problems?  And why does using "../../.." fix them?