unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* emacs.pdmp not always rebuilt
@ 2021-10-03 12:41 Lars Ingebrigtsen
  2021-10-03 12:53 ` Eli Zaretskii
  0 siblings, 1 reply; 25+ messages in thread
From: Lars Ingebrigtsen @ 2021-10-03 12:41 UTC (permalink / raw)
  To: Emacs Devel

A frustrating thing that seems to happen sometimes is that I'm editing a
.el file that's going to be dumped -- but this doesn't reliably lead to
the .pdmp file being rebuilt.  (I'm not sure why this doesn't happen.)

Even more frustratingly, deleting the .pdmp file just leads to "make"
not working at all.

Does this sound familiar to anybody?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-03 12:41 emacs.pdmp not always rebuilt Lars Ingebrigtsen
@ 2021-10-03 12:53 ` Eli Zaretskii
  2021-10-03 13:10   ` Lars Ingebrigtsen
  0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2021-10-03 12:53 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: emacs-devel

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Sun, 03 Oct 2021 14:41:33 +0200
> 
> A frustrating thing that seems to happen sometimes is that I'm editing a
> .el file that's going to be dumped -- but this doesn't reliably lead to
> the .pdmp file being rebuilt.  (I'm not sure why this doesn't happen.)

Maybe "make -jN" with N too high causes some issues with time stamps?
Otherwise I don't understand how this could happen: the emacs
executable depends on all the preloaded *.elc files, so recompiling
any of them should re-dump Emacs.

> Even more frustratingly, deleting the .pdmp file just leads to "make"
> not working at all.

You need to delete the emacs executable as well, and touch temacs.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-03 12:53 ` Eli Zaretskii
@ 2021-10-03 13:10   ` Lars Ingebrigtsen
  2021-10-03 14:35     ` Stefan Monnier
  2021-10-03 14:52     ` Eli Zaretskii
  0 siblings, 2 replies; 25+ messages in thread
From: Lars Ingebrigtsen @ 2021-10-03 13:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> Maybe "make -jN" with N too high causes some issues with time stamps?
> Otherwise I don't understand how this could happen: the emacs
> executable depends on all the preloaded *.elc files, so recompiling
> any of them should re-dump Emacs.

That's what I thought, too, but:

larsi@elva:~/src/emacs/trunk$ ls -l src/emacs.pdmp
-rw-r--r-- 2 larsi larsi 10985296 Oct  3 14:47 src/emacs.pdmp
larsi@elva:~/src/emacs/trunk$ touch lisp/loadup.el
larsi@elva:~/src/emacs/trunk$ make
make -C lib all
make[1]: Entering directory '/home/larsi/src/emacs/trunk/lib'
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory '/home/larsi/src/emacs/trunk/lib'
make -C lib-src all
make[1]: Entering directory '/home/larsi/src/emacs/trunk/lib-src'
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory '/home/larsi/src/emacs/trunk/lib-src'
make -C src VCSWITNESS='$(srcdir)/../.git/logs/HEAD' BIN_DESTDIR=''/usr/local/bin/'' \
	 ELN_DESTDIR='/usr/local/lib/emacs/29.0.50/' all
make[1]: Entering directory '/home/larsi/src/emacs/trunk/src'
  GEN      lisp.mk
make -C ../admin/charsets all
[...]
make[2]: Leaving directory '/home/larsi/src/emacs/trunk/admin/charsets'
  GEN      ../etc/DOC
rm -f emacs && cp -f temacs emacs
LC_ALL=C ./temacs -batch  -l loadup --temacs=pdump \
	--bin-dest /usr/local/bin/ --eln-dest /usr/local/lib/emacs/29.0.50/
Loading loadup.el (source)...
Dump mode: pdump
Using load-path (/home/larsi/src/emacs/trunk/lisp)
Loading emacs-lisp/byte-run...
Loading emacs-lisp/backquote...
Loading subr...
Loading version...
Loading widget...
Loading custom...
Loading emacs-lisp/map-ynp...
Loading international/mule...
Loading international/mule-conf...
Loading env...
Loading format...
Loading bindings...
Loading window...
Loading files...
Loading emacs-lisp/macroexp...
Loading cus-face...
Loading faces...
Loading loaddefs.el (source)...
Loading button...
Loading emacs-lisp/nadvice...
Loading emacs-lisp/cl-preloaded...
Loading obarray...
Loading abbrev...
Loading simple...
cp -f emacs.pdmp bootstrap-emacs.pdmp 
make[1]: Leaving directory '/home/larsi/src/emacs/trunk/src'
make -C lisp all
[...]
make[1]: Leaving directory '/home/larsi/src/emacs/trunk/doc/misc'
larsi@elva:~/src/emacs/trunk$ !ls
ls -l src/emacs.pdmp
-rw-r--r-- 2 larsi larsi 10985296 Oct  3 14:47 src/emacs.pdmp
larsi@elva:~/src/emacs/trunk$ date
Sun Oct  3 15:05:37 CEST 2021
larsi@elva:~/src/emacs/trunk$ 

>> Even more frustratingly, deleting the .pdmp file just leads to "make"
>> not working at all.
>
> You need to delete the emacs executable as well, and touch temacs.

If I do that, I just end up with:

Loading abbrev...
Loading simple...
cp -f emacs.pdmp bootstrap-emacs.pdmp 
cp: cannot stat 'emacs.pdmp': No such file or directory

Hm...  perhaps what I'm working on is making the build process bug out
in a way that's not...  communicated?  I'm working on something in
subr.el...

Aha!  That was it.  OK, sorry for the noise -- I was working on a macro
in subr.el, and that make loading simple.elc fail -- but in a way that's
not reported?  That sounds like a bug, but...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-03 13:10   ` Lars Ingebrigtsen
@ 2021-10-03 14:35     ` Stefan Monnier
  2021-10-03 14:38       ` Lars Ingebrigtsen
  2021-10-03 14:52     ` Eli Zaretskii
  1 sibling, 1 reply; 25+ messages in thread
From: Stefan Monnier @ 2021-10-03 14:35 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Eli Zaretskii, emacs-devel

> LC_ALL=C ./temacs -batch  -l loadup --temacs=pdump \
> 	--bin-dest /usr/local/bin/ --eln-dest /usr/local/lib/emacs/29.0.50/
> Loading loadup.el (source)...
> Dump mode: pdump
> Using load-path (/home/larsi/src/emacs/trunk/lisp)
> Loading emacs-lisp/byte-run...
> Loading emacs-lisp/backquote...
> Loading subr...
> Loading version...
> Loading widget...
> Loading custom...
> Loading emacs-lisp/map-ynp...
> Loading international/mule...
> Loading international/mule-conf...
> Loading env...
> Loading format...
> Loading bindings...
> Loading window...
> Loading files...
> Loading emacs-lisp/macroexp...
> Loading cus-face...
> Loading faces...
> Loading loaddefs.el (source)...
> Loading button...
> Loading emacs-lisp/nadvice...
> Loading emacs-lisp/cl-preloaded...
> Loading obarray...
> Loading abbrev...
> Loading simple...
> cp -f emacs.pdmp bootstrap-emacs.pdmp 

This is oddly short: there's clearly something missing between the last
two lines.  Indeed, I think we have a bug when the dump process signals
an error, where it looks like the dump silently exits without any
error report.


        Stefan




^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-03 14:35     ` Stefan Monnier
@ 2021-10-03 14:38       ` Lars Ingebrigtsen
  2021-10-03 23:27         ` Gregory Heytings
  0 siblings, 1 reply; 25+ messages in thread
From: Lars Ingebrigtsen @ 2021-10-03 14:38 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> This is oddly short: there's clearly something missing between the last
> two lines.  Indeed, I think we have a bug when the dump process signals
> an error, where it looks like the dump silently exits without any
> error report.

Yup.  And it depends on the error.  Put the following into simple.el,
and you'll get a warning from the byte compiler, but you won't get any
errors when building:

(error)

Loading abbrev...
Loading simple...
cp -f emacs.pdmp bootstrap-emacs.pdmp 

With

(error "foo")

instead, you get the expected error:

Loading abbrev...
Loading simple...
foo
make[1]: *** [Makefile:587: emacs.pdmp] Error 255
make[1]: Leaving directory '/home/larsi/src/emacs/trunk/src'
make: *** [Makefile:449: src] Error 2


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-03 13:10   ` Lars Ingebrigtsen
  2021-10-03 14:35     ` Stefan Monnier
@ 2021-10-03 14:52     ` Eli Zaretskii
  1 sibling, 0 replies; 25+ messages in thread
From: Eli Zaretskii @ 2021-10-03 14:52 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: emacs-devel

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: emacs-devel@gnu.org
> Date: Sun, 03 Oct 2021 15:10:53 +0200
> 
> Hm...  perhaps what I'm working on is making the build process bug out
> in a way that's not...  communicated?  I'm working on something in
> subr.el...
> 
> Aha!  That was it.  OK, sorry for the noise -- I was working on a macro
> in subr.el, and that make loading simple.elc fail -- but in a way that's
> not reported?

Yes, see how the loadup'ed list is way too short:

> LC_ALL=C ./temacs -batch  -l loadup --temacs=pdump \
> 	--bin-dest /usr/local/bin/ --eln-dest /usr/local/lib/emacs/29.0.50/
> Loading loadup.el (source)...
> Dump mode: pdump
> Using load-path (/home/larsi/src/emacs/trunk/lisp)
> Loading emacs-lisp/byte-run...
> Loading emacs-lisp/backquote...
> Loading subr...
> Loading version...
> Loading widget...
> Loading custom...
> Loading emacs-lisp/map-ynp...
> Loading international/mule...
> Loading international/mule-conf...
> Loading env...
> Loading format...
> Loading bindings...
> Loading window...
> Loading files...
> Loading emacs-lisp/macroexp...
> Loading cus-face...
> Loading faces...
> Loading loaddefs.el (source)...
> Loading button...
> Loading emacs-lisp/nadvice...
> Loading emacs-lisp/cl-preloaded...
> Loading obarray...
> Loading abbrev...
> Loading simple...
> cp -f emacs.pdmp bootstrap-emacs.pdmp 

This means something happened while loading simple.elc

> That sounds like a bug, but...

Yes, it would be good to avoid these "silent failures", they gave me
my share of trouble.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-03 14:38       ` Lars Ingebrigtsen
@ 2021-10-03 23:27         ` Gregory Heytings
  2021-10-04  0:45           ` Stefan Kangas
  2021-10-04  2:28           ` Eli Zaretskii
  0 siblings, 2 replies; 25+ messages in thread
From: Gregory Heytings @ 2021-10-03 23:27 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Eli Zaretskii, Stefan Monnier, emacs-devel

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


>
> And it depends on the error.  Put the following into simple.el, and 
> you'll get a warning from the byte compiler, but you won't get any 
> errors when building:
>

This is because of dcf9cd47ae.  Patch attached, which will hopefully fix 
the other silent failures you experience.

[-- Attachment #2: Type: text/x-diff, Size: 1109 bytes --]

From dac10e81b344483b61ac4086725909a85be67ad2 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Sun, 3 Oct 2021 23:21:32 +0000
Subject: [PATCH] Do not use the Elisp version of substitute-command-keys in
 batch mode.

src/print.c (print_error_message): Do not use the Elisp version of
substitute-command-keys in batch mode.
---
 src/print.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/print.c b/src/print.c
index d4301fd7b6..f83e5d9ece 100644
--- a/src/print.c
+++ b/src/print.c
@@ -941,7 +941,10 @@ print_error_message (Lisp_Object data, Lisp_Object stream, const char *context,
   else
     {
       Lisp_Object error_conditions = Fget (errname, Qerror_conditions);
-      errmsg = call1 (Qsubstitute_command_keys, Fget (errname, Qerror_message));
+      if (noninteractive)
+	errmsg = Fsubstitute_command_keys_old (Fget (errname, Qerror_message));
+      else
+	errmsg = call1 (Qsubstitute_command_keys, Fget (errname, Qerror_message));
       file_error = Fmemq (Qfile_error, error_conditions);
     }
 
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-03 23:27         ` Gregory Heytings
@ 2021-10-04  0:45           ` Stefan Kangas
  2021-10-04  2:28           ` Eli Zaretskii
  1 sibling, 0 replies; 25+ messages in thread
From: Stefan Kangas @ 2021-10-04  0:45 UTC (permalink / raw)
  To: Gregory Heytings, Lars Ingebrigtsen
  Cc: Eli Zaretskii, Stefan Monnier, emacs-devel

Gregory Heytings <gregory@heytings.org> writes:

> This is because of dcf9cd47ae.  Patch attached, which will hopefully fix
> the other silent failures you experience.

Thanks for looking into this.

That patch unfortunately won't compile on current master.  What's your
analysis of this error?

If `substitute-command-keys' is problematic in batch-mode or during
bootstrapping, I think it's fine not to run it at all in that case:
unless there are command substitutions in your error messages it will
anyways only gives you ‘fancy quotes’ (and I guess not even that,
depending on your terminal).



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-03 23:27         ` Gregory Heytings
  2021-10-04  0:45           ` Stefan Kangas
@ 2021-10-04  2:28           ` Eli Zaretskii
  2021-10-04  8:57             ` Gregory Heytings
  1 sibling, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2021-10-04  2:28 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: larsi, monnier, emacs-devel

> Date: Sun, 03 Oct 2021 23:27:51 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Stefan Monnier <monnier@iro.umontreal.ca>, Eli Zaretskii <eliz@gnu.org>, 
>     emacs-devel@gnu.org
> 
> > And it depends on the error.  Put the following into simple.el, and 
> > you'll get a warning from the byte compiler, but you won't get any 
> > errors when building:
> >
> 
> This is because of dcf9cd47ae.  Patch attached, which will hopefully fix 
> the other silent failures you experience.

Thanks, but why condition that on batch mode?  Does the problem happen
in any batch-mode invocation, and if so, why?



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-04  2:28           ` Eli Zaretskii
@ 2021-10-04  8:57             ` Gregory Heytings
  2021-10-04  9:19               ` Lars Ingebrigtsen
  2021-10-04 12:50               ` Eli Zaretskii
  0 siblings, 2 replies; 25+ messages in thread
From: Gregory Heytings @ 2021-10-04  8:57 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, monnier, emacs-devel

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


>>> And it depends on the error.  Put the following into simple.el, and 
>>> you'll get a warning from the byte compiler, but you won't get any 
>>> errors when building:
>>
>> This is because of dcf9cd47ae.  Patch attached, which will hopefully 
>> fix the other silent failures you experience.
>
> Thanks, but why condition that on batch mode?  Does the problem happen 
> in any batch-mode invocation, and if so, why?
>

When Emacs has been built, the problem does not seem to exist anymore. 
FWIW, I'm not sure it makes sense to call substitute-command-keys there, 
even in non-batch mode, but it should probably be okay to limit the scope 
of the fix to something narrower.  Updated patch attached, this time 
against the current trunk.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=Do-not-use-substitute-command-keys-during-bootstrap.patch, Size: 1063 bytes --]

From d47559b0fd888ce5ea242a51d449262b3d636e8e Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Mon, 4 Oct 2021 08:34:44 +0000
Subject: [PATCH] Do not use substitute-command-keys during bootstrap.

src/print.c (print_error_message): Do not use substitute-command-keys
during bootstrap.
---
 src/print.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/print.c b/src/print.c
index d4301fd7b6..ef01738436 100644
--- a/src/print.c
+++ b/src/print.c
@@ -941,7 +941,10 @@ print_error_message (Lisp_Object data, Lisp_Object stream, const char *context,
   else
     {
       Lisp_Object error_conditions = Fget (errname, Qerror_conditions);
-      errmsg = call1 (Qsubstitute_command_keys, Fget (errname, Qerror_message));
+      if (will_bootstrap_p () || will_dump_p ())
+	errmsg = Fget (errname, Qerror_message);
+      else
+	errmsg = call1 (Qsubstitute_command_keys, Fget (errname, Qerror_message));
       file_error = Fmemq (Qfile_error, error_conditions);
     }
 
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-04  8:57             ` Gregory Heytings
@ 2021-10-04  9:19               ` Lars Ingebrigtsen
  2021-10-04 11:14                 ` Stefan Kangas
  2021-10-04 12:53                 ` Eli Zaretskii
  2021-10-04 12:50               ` Eli Zaretskii
  1 sibling, 2 replies; 25+ messages in thread
From: Lars Ingebrigtsen @ 2021-10-04  9:19 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: Eli Zaretskii, monnier, emacs-devel

Gregory Heytings <gregory@heytings.org> writes:

> When Emacs has been built, the problem does not seem to exist
> anymore. FWIW, I'm not sure it makes sense to call
> substitute-command-keys there, even in non-batch mode, but it should
> probably be okay to limit the scope of the fix to something narrower.
> Updated patch attached, this time against the current trunk.

Thanks; seems to work very well, so I pushed it to the trunk (with an
additional comment).

If there are no reported issues with this, perhaps it should be
cherry-picked for emacs-28?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-04  9:19               ` Lars Ingebrigtsen
@ 2021-10-04 11:14                 ` Stefan Kangas
  2021-10-04 12:53                 ` Eli Zaretskii
  1 sibling, 0 replies; 25+ messages in thread
From: Stefan Kangas @ 2021-10-04 11:14 UTC (permalink / raw)
  To: Lars Ingebrigtsen
  Cc: Gregory Heytings, Emacs developers, Eli Zaretskii, Stefan Monnier

Lars Ingebrigtsen <larsi@gnus.org> writes:

> If there are no reported issues with this, perhaps it should be
> cherry-picked for emacs-28?

FWIW, I think that would make sense; in many cases the change won't
matter and it fixes a real problem.

If someone is parsing the build output and are looking for markers
produced by 'substitute-command-keys', I guess they might have a
problem, but that seems minor in comparison to the issue this solves.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-04  8:57             ` Gregory Heytings
  2021-10-04  9:19               ` Lars Ingebrigtsen
@ 2021-10-04 12:50               ` Eli Zaretskii
  2021-10-04 13:05                 ` Gregory Heytings
  1 sibling, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2021-10-04 12:50 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: larsi, monnier, emacs-devel

> Date: Mon, 04 Oct 2021 08:57:04 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: larsi@gnus.org, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> >> This is because of dcf9cd47ae.  Patch attached, which will hopefully 
> >> fix the other silent failures you experience.
> >
> > Thanks, but why condition that on batch mode?  Does the problem happen 
> > in any batch-mode invocation, and if so, why?
> 
> When Emacs has been built, the problem does not seem to exist anymore. 
> FWIW, I'm not sure it makes sense to call substitute-command-keys there, 
> even in non-batch mode, but it should probably be okay to limit the scope 
> of the fix to something narrower.  Updated patch attached, this time 
> against the current trunk.

But what is the underlying problem?  Is it perhaps that
substitute-command-keys is defined in help.el, and help.el is loaded
by loadup only after simple.el?  If so, I think there could be a
better solution to this: either change the order of loading, or simply
test if substitute-command-keys is fboundp, and avoid using it if not.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-04  9:19               ` Lars Ingebrigtsen
  2021-10-04 11:14                 ` Stefan Kangas
@ 2021-10-04 12:53                 ` Eli Zaretskii
  1 sibling, 0 replies; 25+ messages in thread
From: Eli Zaretskii @ 2021-10-04 12:53 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: gregory, monnier, emacs-devel

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  monnier@iro.umontreal.ca,
>   emacs-devel@gnu.org
> Date: Mon, 04 Oct 2021 11:19:18 +0200
> 
> Gregory Heytings <gregory@heytings.org> writes:
> 
> > When Emacs has been built, the problem does not seem to exist
> > anymore. FWIW, I'm not sure it makes sense to call
> > substitute-command-keys there, even in non-batch mode, but it should
> > probably be okay to limit the scope of the fix to something narrower.
> > Updated patch attached, this time against the current trunk.
> 
> Thanks; seems to work very well, so I pushed it to the trunk (with an
> additional comment).

I think it's premature, because we don't really understand the problem
we are fixing.

> If there are no reported issues with this, perhaps it should be
> cherry-picked for emacs-28?

After we find a proper fix, I indeed think it should be installed on
the release branch (unless, by some chance, the proper fix turns out
to be very complex and unsafe, which I find hard to believe).



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-04 12:50               ` Eli Zaretskii
@ 2021-10-04 13:05                 ` Gregory Heytings
  2021-10-04 13:25                   ` Gregory Heytings
  2021-10-04 13:30                   ` Eli Zaretskii
  0 siblings, 2 replies; 25+ messages in thread
From: Gregory Heytings @ 2021-10-04 13:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, monnier, emacs-devel


>
> But what is the underlying problem?
>

It's obviously that the Elisp substitute-command-keys is not defined 
during the first steps of loadup.

>
> Is it perhaps that substitute-command-keys is defined in help.el, and 
> help.el is loaded by loadup only after simple.el?  If so, I think there 
> could be a better solution to this: either change the order of loading,
>

No, because that would require to load help.el before all other files 
(e.g. the error could happen in subr.el), and actually even before itself 
(because the error could also happen inside help.el).

>
> or simply test if substitute-command-keys is fboundp, and avoid using it 
> if not.
>

That would be another option, yes.  I don't think it's much better though, 
as Stefan K said a few hours ago the potential effect on 
substitute-command-keys on error messages during the build is minor.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-04 13:05                 ` Gregory Heytings
@ 2021-10-04 13:25                   ` Gregory Heytings
  2021-10-04 13:37                     ` Eli Zaretskii
  2021-10-04 13:30                   ` Eli Zaretskii
  1 sibling, 1 reply; 25+ messages in thread
From: Gregory Heytings @ 2021-10-04 13:25 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, monnier, emacs-devel


>> or simply test if substitute-command-keys is fboundp, and avoid using 
>> it if not.
>
> That would be another option, yes.  I don't think it's much better 
> though, as Stefan K said a few hours ago the potential effect on 
> substitute-command-keys on error messages during the build is minor.
>

And after thinking a few seconds more about this, IMO it would not be a 
better option: the fact that substitute-command-keys is fboundp does not 
guarantee that it will work correctly, because it could be the function 
that is currently being debugged or updated.  So IMO it's better to simply 
avoid using substitute-command-keys on error messages during bootstrap, 
which is what the patch does.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-04 13:05                 ` Gregory Heytings
  2021-10-04 13:25                   ` Gregory Heytings
@ 2021-10-04 13:30                   ` Eli Zaretskii
  2021-10-04 14:03                     ` Stefan Kangas
  1 sibling, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2021-10-04 13:30 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: larsi, monnier, emacs-devel

> Date: Mon, 04 Oct 2021 13:05:11 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: larsi@gnus.org, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> > or simply test if substitute-command-keys is fboundp, and avoid using it 
> > if not.
> 
> That would be another option, yes.  I don't think it's much better though, 
> as Stefan K said a few hours ago the potential effect on 
> substitute-command-keys on error messages during the build is minor.

I don't think that's what Stefan meant to convey, but if he did, I
disagree.  I think avoiding the call if the function is not available
is the cleanest solution, one that we use elsewhere in Emacs in
similar cases.  "Punishing" the entire loadup process because of a
small window where the function is not yet known is sub-optimal, IMO.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-04 13:25                   ` Gregory Heytings
@ 2021-10-04 13:37                     ` Eli Zaretskii
  2021-10-04 14:18                       ` Gregory Heytings
  0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2021-10-04 13:37 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: larsi, monnier, emacs-devel

> Date: Mon, 04 Oct 2021 13:25:03 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: larsi@gnus.org, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> And after thinking a few seconds more about this, IMO it would not be a 
> better option: the fact that substitute-command-keys is fboundp does not 
> guarantee that it will work correctly, because it could be the function 
> that is currently being debugged or updated.  So IMO it's better to simply 
> avoid using substitute-command-keys on error messages during bootstrap, 
> which is what the patch does.

What do you mean by "debugged or updated", and how would that affect
what we do at loadup time, but not what we do at any other time,
including in interactive sessions?

Or are you saying that we should move substitute-command-keys back to
C?



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-04 13:30                   ` Eli Zaretskii
@ 2021-10-04 14:03                     ` Stefan Kangas
  2021-10-04 14:19                       ` Gregory Heytings
  0 siblings, 1 reply; 25+ messages in thread
From: Stefan Kangas @ 2021-10-04 14:03 UTC (permalink / raw)
  To: Eli Zaretskii
  Cc: Gregory Heytings, Emacs developers, Lars Ingebrigtsen,
	Stefan Monnier

Eli Zaretskii <eliz@gnu.org> writes:

> I think avoiding the call if the function is not available
> is the cleanest solution, one that we use elsewhere in Emacs in
> similar cases.

That sounds reasonable, yes.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-04 13:37                     ` Eli Zaretskii
@ 2021-10-04 14:18                       ` Gregory Heytings
  2021-10-04 14:34                         ` Eli Zaretskii
  0 siblings, 1 reply; 25+ messages in thread
From: Gregory Heytings @ 2021-10-04 14:18 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, monnier, emacs-devel


>
> What do you mean by "debugged or updated", and how would that affect 
> what we do at loadup time, but not what we do at any other time, 
> including in interactive sessions?
>

The problem that Lars, you and possibly others experienced (make 
continuing when it should have stopped) was due to the fact that "call1 
(Qsubstitute_command_keys, Fget (errname, Qerror_message));" in 
print_error_message() failed when it was called while loading the first 
files in loadup.  Are you sure that this call to call1 cannot possibly 
fail when fboundp is t?

What could happen during interactive sessions is something else.

>
> Or are you saying that we should move substitute-command-keys back to C?
>

I don't know.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-04 14:03                     ` Stefan Kangas
@ 2021-10-04 14:19                       ` Gregory Heytings
  2021-10-04 15:10                         ` Eli Zaretskii
  0 siblings, 1 reply; 25+ messages in thread
From: Gregory Heytings @ 2021-10-04 14:19 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: eliz, larsi, monnier, emacs-devel

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


>> I think avoiding the call if the function is not available is the 
>> cleanest solution, one that we use elsewhere in Emacs in similar cases.
>
> That sounds reasonable, yes.
>

And here is the patch.  As I just said, I'm not entirely convinced that 
it's better, but it isn't much worse either.

[-- Attachment #2: Type: text/x-diff, Size: 983 bytes --]

From 0d7dbb4801d914be00ac9b9685fdb5c7b3d619ab Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Mon, 4 Oct 2021 14:13:46 +0000
Subject: [PATCH] Fix problem with outputting error messages while dumping
 Emacs better

* src/print.c (print_error_message): Don't call substitute-command-keys
when it isn't bound.
---
 src/print.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/print.c b/src/print.c
index ef01738436..221e7b8c2a 100644
--- a/src/print.c
+++ b/src/print.c
@@ -941,7 +941,7 @@ print_error_message (Lisp_Object data, Lisp_Object stream, const char *context,
   else
     {
       Lisp_Object error_conditions = Fget (errname, Qerror_conditions);
-      if (will_bootstrap_p () || will_dump_p ())
+      if (NILP (Fboundp (Qsubstitute_command_keys)))
 	errmsg = Fget (errname, Qerror_message);
       else
 	errmsg = call1 (Qsubstitute_command_keys, Fget (errname, Qerror_message));
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-04 14:18                       ` Gregory Heytings
@ 2021-10-04 14:34                         ` Eli Zaretskii
  2021-10-04 16:02                           ` Gregory Heytings
  0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2021-10-04 14:34 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: larsi, monnier, emacs-devel

> Date: Mon, 04 Oct 2021 14:18:41 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: larsi@gnus.org, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> > What do you mean by "debugged or updated", and how would that affect 
> > what we do at loadup time, but not what we do at any other time, 
> > including in interactive sessions?
> 
> The problem that Lars, you and possibly others experienced (make 
> continuing when it should have stopped) was due to the fact that "call1 
> (Qsubstitute_command_keys, Fget (errname, Qerror_message));" in 
> print_error_message() failed when it was called while loading the first 
> files in loadup.  Are you sure that this call to call1 cannot possibly 
> fail when fboundp is t?

If fboundp returns non-nil, it means the symbol's function cell is not
void, i.e. the symbol can be used as a function.  Right?  So the call
can only fail if the function itself has a bug, right?  And in the
latter case, how is that different from any other Lisp function called
during loadup?

> What could happen during interactive sessions is something else.

I think it's the same.  IOW, any problem that could happen at loadup
time could also happen during a regular session, and will have the
same results.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-04 14:19                       ` Gregory Heytings
@ 2021-10-04 15:10                         ` Eli Zaretskii
  0 siblings, 0 replies; 25+ messages in thread
From: Eli Zaretskii @ 2021-10-04 15:10 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: larsi, stefan, monnier, emacs-devel

> Date: Mon, 04 Oct 2021 14:19:52 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: eliz@gnu.org, emacs-devel@gnu.org, larsi@gnus.org, 
>     monnier@iro.umontreal.ca
> 
> >> I think avoiding the call if the function is not available is the 
> >> cleanest solution, one that we use elsewhere in Emacs in similar cases.
> >
> > That sounds reasonable, yes.
> >
> 
> And here is the patch.  As I just said, I'm not entirely convinced that 
> it's better, but it isn't much worse either.

Thanks, installed on the emacs-28 branch (with a couple of minor
tweaks).

I also reverted the previous fix on master, since this fix will be
merged to master shortly, and it is best to avoid merge conflicts.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-04 14:34                         ` Eli Zaretskii
@ 2021-10-04 16:02                           ` Gregory Heytings
  2021-10-04 16:56                             ` Eli Zaretskii
  0 siblings, 1 reply; 25+ messages in thread
From: Gregory Heytings @ 2021-10-04 16:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, monnier, emacs-devel


>>> What do you mean by "debugged or updated", and how would that affect 
>>> what we do at loadup time, but not what we do at any other time, 
>>> including in interactive sessions?
>>
>> The problem that Lars, you and possibly others experienced (make 
>> continuing when it should have stopped) was due to the fact that "call1 
>> (Qsubstitute_command_keys, Fget (errname, Qerror_message));" in 
>> print_error_message() failed when it was called while loading the first 
>> files in loadup.  Are you sure that this call to call1 cannot possibly 
>> fail when fboundp is t?
>
> If fboundp returns non-nil, it means the symbol's function cell is not 
> void, i.e. the symbol can be used as a function.  Right?  So the call 
> can only fail if the function itself has a bug, right?  And in the 
> latter case, how is that different from any other Lisp function called 
> during loadup?
>

It's different, precisely because it is not a function among other 
functions, it's the function that signals an error.  And as we've just 
seen, if the function that signals an error itself fails, unexpected 
behavior can happen, especially when this happens during bootstrap.

I would have taken extra care to avoid that possibility, especially given 
that the importance of substitute-command-keys on error messages during 
bootstrap is close to zero.  Perhaps I did not look close enough, but I 
could not find a single error message where substitute-command-keys would 
have had an effect, and even if there are a few, seeing "Command 
\[foobar\] blah blah" instead of "Command M-x foobar blah blah" or 
"Command C-x % blah blah" does not make a big difference.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: emacs.pdmp not always rebuilt
  2021-10-04 16:02                           ` Gregory Heytings
@ 2021-10-04 16:56                             ` Eli Zaretskii
  0 siblings, 0 replies; 25+ messages in thread
From: Eli Zaretskii @ 2021-10-04 16:56 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: larsi, monnier, emacs-devel

> Date: Mon, 04 Oct 2021 16:02:18 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: larsi@gnus.org, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> > If fboundp returns non-nil, it means the symbol's function cell is not 
> > void, i.e. the symbol can be used as a function.  Right?  So the call 
> > can only fail if the function itself has a bug, right?  And in the 
> > latter case, how is that different from any other Lisp function called 
> > during loadup?
> 
> It's different, precisely because it is not a function among other 
> functions, it's the function that signals an error.  And as we've just 
> seen, if the function that signals an error itself fails, unexpected 
> behavior can happen, especially when this happens during bootstrap.
> 
> I would have taken extra care to avoid that possibility, especially given 
> that the importance of substitute-command-keys on error messages during 
> bootstrap is close to zero.  Perhaps I did not look close enough, but I 
> could not find a single error message where substitute-command-keys would 
> have had an effect, and even if there are a few, seeing "Command 
> \[foobar\] blah blah" instead of "Command M-x foobar blah blah" or 
> "Command C-x % blah blah" does not make a big difference.

Ah, that argument...  That train has left the station long ago, and
your argument is then not with me.  I'm just the janitor, cleaning up
after the train that left, trying to have a tidy station nonetheless.



^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2021-10-04 16:56 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-03 12:41 emacs.pdmp not always rebuilt Lars Ingebrigtsen
2021-10-03 12:53 ` Eli Zaretskii
2021-10-03 13:10   ` Lars Ingebrigtsen
2021-10-03 14:35     ` Stefan Monnier
2021-10-03 14:38       ` Lars Ingebrigtsen
2021-10-03 23:27         ` Gregory Heytings
2021-10-04  0:45           ` Stefan Kangas
2021-10-04  2:28           ` Eli Zaretskii
2021-10-04  8:57             ` Gregory Heytings
2021-10-04  9:19               ` Lars Ingebrigtsen
2021-10-04 11:14                 ` Stefan Kangas
2021-10-04 12:53                 ` Eli Zaretskii
2021-10-04 12:50               ` Eli Zaretskii
2021-10-04 13:05                 ` Gregory Heytings
2021-10-04 13:25                   ` Gregory Heytings
2021-10-04 13:37                     ` Eli Zaretskii
2021-10-04 14:18                       ` Gregory Heytings
2021-10-04 14:34                         ` Eli Zaretskii
2021-10-04 16:02                           ` Gregory Heytings
2021-10-04 16:56                             ` Eli Zaretskii
2021-10-04 13:30                   ` Eli Zaretskii
2021-10-04 14:03                     ` Stefan Kangas
2021-10-04 14:19                       ` Gregory Heytings
2021-10-04 15:10                         ` Eli Zaretskii
2021-10-03 14:52     ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).