Subject: bug#34114: 27.0.50: pdumper and themes with Emacs daemon

From: Daniel Colascione <[email-masked]>
Date: Mon, 21 Jan 2019 17:59:46 -0500

On 1/19/19 2:05 AM, Eli Zaretskii wrote:
>> From: Karl Otness <[email-masked]>
>> Date: Fri, 18 Jan 2019 23:02:14 +0000
>> Cc: Kaushal Modi <[email-masked]>
>>
>> I think I have found a change that fixes this issue. I haven't tested
>> it too thoroughly, but it seems to work for me. The change is to call
>> init_faces_initial from init_display_interactive even when Emacs is
>> running as daemon.
>>
>> In init_display_interactive in dispnew.c moving the lines:
>>
>>> /* Set up faces of the initial terminal frame.  */
>>> if (!noninteractive && NILP (Vinitial_window_system))
>>>    init_faces_initial ();
>>
>> from the end of the function so that they occur before the early
>> return in the IS_DAEMON check (around line 6039) seems to fix the
>> theme loading problem.
> 
> Thanks for identifying the code that's involved in this.
> 
> However, I'm not yet sure the solution you propose is the correct
> one.  The way the code is written, it relies on the faces of the
> initial frame to be initialized when Emacs is built, and then recorded
> in the dumped Emacs.  With the pdumper approach, the record should be
> in the emacs.pdmp file, and I'd expect it to be restored from there.
> 
> Why isn't this working?
> 
> If we want to repeat this initialization in the daemon when it starts
> up, I'd like first to better understand the considerations for this
> kind of situations: when we should fix such problems by recording and
> restoring them to/from the dump file, and when we should run the
> initialization code anew in the dumped Emacs.  The latter might mean
> we cause more trouble elsewhere, because a lot of the code that runs
> during startup expects stuff done in temacs to be available and ready
> to use, not recomputed anew.
> 
> Daniel, can you comment on this aspect of the problem, including on
> the more general issue?
> 
> In any case, if the proposed change is installed, it should not affect
> the unexeced Emacs, so it IMO should be conditioned on
> dumped_with_pdumper_p.  Unexeced Emacs should continue behaving as it
> did before.

In general, we should try do as much work as possible in the initial 
dump, just as we do in unexeced Emacs. Frames, however, are special 
because we can't actually save and restore frames. Any frame object 
comes back, after pdumper load, as an all-nil object. Since faces are 
attached to frames, and since after pdumper load, we have all-new 
frames, we need to re-add the default faces. Kaushal's change seems like 
the right sort of fix. All of this is very confusing and we should 
probably totally rethink it after we remove unexec.