bug#69809: 30.0.50; flymake: error in process sentinel

bug#69809: 30.0.50; flymake: error in process sentinel

[Gerd Möllmann]

In master, I am sometimes getting errors like these:

  error in process sentinel: flymake--handle-report: Can’t find state for flymake-cc in ‘flymake--state’
  error in process sentinel: Can’t find state for flymake-cc in ‘flymake--state’
  error in process sentinel: flymake--handle-report: Can’t find state for flymake-cc in ‘flymake--state’
  error in process sentinel: Can’t find state for flymake-cc in ‘flymake--state’
  error in process sentinel: flymake--handle-report: Can’t find state for flymake-cc in ‘flymake--state’
  error in process sentinel: Can’t find state for flymake-cc in ‘flymake--state’

when working with C files.

I haven't configured anything for Flymake myself. I think Flymake gets
involved by using Eglot. The errors apparently don't prevent flymake
from working later on.

I have looked around in flymake docs and source, but I can't figure out
what's wrong.

bug#69809: 30.0.50; flymake: error in process sentinel

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Date: Fri, 15 Mar 2024 08:09:40 +0100
> 
> In master, I am sometimes getting errors like these:
> 
>   error in process sentinel: flymake--handle-report: Can’t find state for flymake-cc in ‘flymake--state’
>   error in process sentinel: Can’t find state for flymake-cc in ‘flymake--state’
>   error in process sentinel: flymake--handle-report: Can’t find state for flymake-cc in ‘flymake--state’
>   error in process sentinel: Can’t find state for flymake-cc in ‘flymake--state’
>   error in process sentinel: flymake--handle-report: Can’t find state for flymake-cc in ‘flymake--state’
>   error in process sentinel: Can’t find state for flymake-cc in ‘flymake--state’
> 
> when working with C files.
> 
> I haven't configured anything for Flymake myself. I think Flymake gets
> involved by using Eglot. The errors apparently don't prevent flymake
> from working later on.
> 
> I have looked around in flymake docs and source, but I can't figure out
> what's wrong.

Spencer, any suggestions?

bug#69809: 30.0.50; flymake: error in process sentinel

[sbaugh]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Date: Fri, 15 Mar 2024 08:09:40 +0100
> 
> In master, I am sometimes getting errors like these:
> 
>   error in process sentinel: flymake--handle-report: Can’t find state for flymake-cc in ‘flymake--state’
>   error in process sentinel: Can’t find state for flymake-cc in ‘flymake--state’
>   error in process sentinel: flymake--handle-report: Can’t find state for flymake-cc in ‘flymake--state’
>   error in process sentinel: Can’t find state for flymake-cc in ‘flymake--state’
>   error in process sentinel: flymake--handle-report: Can’t find state for flymake-cc in ‘flymake--state’
>   error in process sentinel: Can’t find state for flymake-cc in ‘flymake--state’
> 
> when working with C files.
> 
> I haven't configured anything for Flymake myself. I think Flymake gets
> involved by using Eglot. The errors apparently don't prevent flymake
> from working later on.
> 
> I have looked around in flymake docs and source, but I can't figure out
> what's wrong.

It would be helpful if you could provide a minimal reproduction starting
from "emacs -q".

My immediate suspicion is that flymake-mode is (somehow) enabled in your
C files while flymake-diagnostic-functions is set to contain flymake-cc,
which causes flymake-cc to start up a background process.  Then,
flymake-mode is enabled again by eglot--managed-mode, which causes
flymake--state to be cleared, so when flymake-cc tries to report
diagnostics from that background process through flymake--handle-report,
it fails.

But I can't be sure whether this is due to a bug in Emacs or due to a
bug in your config without a more minimal reproduction.

bug#69809: 30.0.50; flymake: error in process sentinel

[Gerd Möllmann]

sbaugh@catern.com writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Date: Fri, 15 Mar 2024 08:09:40 +0100
>>
>> In master, I am sometimes getting errors like these:
>>
>>   error in process sentinel: flymake--handle-report: Can’t find state for flymake-cc in ‘flymake--state’
>>   error in process sentinel: Can’t find state for flymake-cc in ‘flymake--state’
>>   error in process sentinel: flymake--handle-report: Can’t find state for flymake-cc in ‘flymake--state’
>>   error in process sentinel: Can’t find state for flymake-cc in ‘flymake--state’
>>   error in process sentinel: flymake--handle-report: Can’t find state for flymake-cc in ‘flymake--state’
>>   error in process sentinel: Can’t find state for flymake-cc in ‘flymake--state’
>>
>> when working with C files.
>>
>> I haven't configured anything for Flymake myself. I think Flymake gets
>> involved by using Eglot. The errors apparently don't prevent flymake
>> from working later on.
>>
>> I have looked around in flymake docs and source, but I can't figure out
>> what's wrong.
>
> It would be helpful if you could provide a minimal reproduction starting
> from "emacs -q".

I know, but I can't reproduce it at will. And debug-on-error didn't help
catch it in the act, maybe there is some condition-case involved
somewhere that should have better been a condition-case-unless-debug.

> My immediate suspicion is that flymake-mode is (somehow) enabled in your
> C files while flymake-diagnostic-functions is set to contain flymake-cc,
> which causes flymake-cc to start up a background process.  Then,
> flymake-mode is enabled again by eglot--managed-mode, which causes
> flymake--state to be cleared, so when flymake-cc tries to report
> diagnostics from that background process through flymake--handle-report,
> it fails.
>
> But I can't be sure whether this is due to a bug in Emacs or due to a
> bug in your config without a more minimal reproduction.

No config using flymake and none for eglot. The only thing I did is
put eglot-ensure on c-mode-common-hook.

Maybe I can catch it in LLDB somehow, but that will have to wait a bit,
unfortunately.

Anyway, thanks for the replay.

bug#69809: 30.0.50; flymake: error in process sentinel

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi,

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> sbaugh@catern.com writes:
>
>>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>>> Date: Fri, 15 Mar 2024 08:09:40 +0100
>>>
>>> In master, I am sometimes getting errors like these:
>>>
>>>   error in process sentinel: flymake--handle-report: Can’t find state for flymake-cc in ‘flymake--state’
>>>   error in process sentinel: Can’t find state for flymake-cc in ‘flymake--state’
>>>   error in process sentinel: flymake--handle-report: Can’t find state for flymake-cc in ‘flymake--state’
>>>   error in process sentinel: Can’t find state for flymake-cc in ‘flymake--state’
>>>   error in process sentinel: flymake--handle-report: Can’t find state for flymake-cc in ‘flymake--state’
>>>   error in process sentinel: Can’t find state for flymake-cc in ‘flymake--state’
>>>
>>> when working with C files.
>>>
>>> I haven't configured anything for Flymake myself. I think Flymake gets
>>> involved by using Eglot. The errors apparently don't prevent flymake
>>> from working later on.
>>>
>>> I have looked around in flymake docs and source, but I can't figure out
>>> what's wrong.
>>
>> It would be helpful if you could provide a minimal reproduction starting
>> from "emacs -q".
>
> I know, but I can't reproduce it at will. And debug-on-error didn't help
> catch it in the act, maybe there is some condition-case involved
> somewhere that should have better been a condition-case-unless-debug.
>
>> My immediate suspicion is that flymake-mode is (somehow) enabled in your
>> C files while flymake-diagnostic-functions is set to contain flymake-cc,
>> which causes flymake-cc to start up a background process.  Then,
>> flymake-mode is enabled again by eglot--managed-mode, which causes
>> flymake--state to be cleared, so when flymake-cc tries to report
>> diagnostics from that background process through flymake--handle-report,
>> it fails.
>>
>> But I can't be sure whether this is due to a bug in Emacs or due to a
>> bug in your config without a more minimal reproduction.
>
> No config using flymake and none for eglot. The only thing I did is
> put eglot-ensure on c-mode-common-hook.
>
> Maybe I can catch it in LLDB somehow, but that will have to wait a bit,
> unfortunately.
>
> Anyway, thanks for the replay.

This issue bothered me as well.  Here's a recipe for reproducing on
master, with emacs -Q:

1. (add-hook 'c-mode-hook 'flymake-mode)
2. (add-hook 'c-mode-hook 'eglot-ensure)
3. Find some C file

This happens because Eglot _restarts_ flymake-mode while flymake-cc's
process is already running.  Here's a simple fix:

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index a893a8d749a..c9e1bb7b52d 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -2040,7 +2040,7 @@ eglot--managed-mode
     (unless (eglot--stay-out-of-p 'imenu)
       (add-function :before-until (local 'imenu-create-index-function)
                     #'eglot-imenu))
-    (unless (eglot--stay-out-of-p 'flymake) (flymake-mode 1))
+    (unless (or (eglot--stay-out-of-p 'flymake) flymake-mode) (flymake-mode 1))
     (unless (eglot--stay-out-of-p 'eldoc)
       (add-hook 'eldoc-documentation-functions #'eglot-hover-eldoc-function
                 nil t)

bug#69809: 30.0.50; flymake: error in process sentinel

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eshel Yaron <me@eshelyaron.com> writes:

[...]

> This issue bothered me as well.  Here's a recipe for reproducing on
> master, with emacs -Q:
>
> 1. (add-hook 'c-mode-hook 'flymake-mode)
> 2. (add-hook 'c-mode-hook 'eglot-ensure)
> 3. Find some C file
>
> This happens because Eglot _restarts_ flymake-mode while flymake-cc's
> process is already running.  Here's a simple fix:
>
> diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> index a893a8d749a..c9e1bb7b52d 100644
> --- a/lisp/progmodes/eglot.el
> +++ b/lisp/progmodes/eglot.el
> @@ -2040,7 +2040,7 @@ eglot--managed-mode
>      (unless (eglot--stay-out-of-p 'imenu)
>        (add-function :before-until (local 'imenu-create-index-function)
>                      #'eglot-imenu))
> -    (unless (eglot--stay-out-of-p 'flymake) (flymake-mode 1))
> +    (unless (or (eglot--stay-out-of-p 'flymake) flymake-mode) (flymake-mode 1))
>      (unless (eglot--stay-out-of-p 'eldoc)
>        (add-hook 'eldoc-documentation-functions #'eglot-hover-eldoc-function
>                  nil t)

I realized that the change above has the downside of no longer
immediately initiating a Flymake analysis with Eglot in place.  To
preserve that behavior, maybe something like the following is better:

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index a893a8d749a..6cd48917d47 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -2040,7 +2040,8 @@ eglot--managed-mode
     (unless (eglot--stay-out-of-p 'imenu)
       (add-function :before-until (local 'imenu-create-index-function)
                     #'eglot-imenu))
-    (unless (eglot--stay-out-of-p 'flymake) (flymake-mode 1))
+    (unless (eglot--stay-out-of-p 'flymake)
+      (if flymake-mode (flymake-start) (flymake-mode 1)))
     (unless (eglot--stay-out-of-p 'eldoc)
       (add-hook 'eldoc-documentation-functions #'eglot-hover-eldoc-function
                 nil t)

bug#69809: 30.0.50; flymake: error in process sentinel

[Gerd Möllmann]

Eshel Yaron <me@eshelyaron.com> writes:

> I realized that the change above has the downside of no longer
> immediately initiating a Flymake analysis with Eglot in place.  To
> preserve that behavior, maybe something like the following is better:
>
> diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> index a893a8d749a..6cd48917d47 100644
> --- a/lisp/progmodes/eglot.el
> +++ b/lisp/progmodes/eglot.el
> @@ -2040,7 +2040,8 @@ eglot--managed-mode
>      (unless (eglot--stay-out-of-p 'imenu)
>        (add-function :before-until (local 'imenu-create-index-function)
>                      #'eglot-imenu))
> -    (unless (eglot--stay-out-of-p 'flymake) (flymake-mode 1))
> +    (unless (eglot--stay-out-of-p 'flymake)
> +      (if flymake-mode (flymake-start) (flymake-mode 1)))
>      (unless (eglot--stay-out-of-p 'eldoc)
>        (add-hook 'eldoc-documentation-functions #'eglot-hover-eldoc-function
>                  nil t)

Makes sense 👍. Thanks Eshel!

bug#69809: 30.0.50; flymake: error in process sentinel

[Eli Zaretskii]

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: sbaugh@catern.com,  Spencer Baugh <sbaugh@janestreet.com>,  Eli
>  Zaretskii <eliz@gnu.org>,  69809@debbugs.gnu.org
> Date: Thu, 11 Jul 2024 13:15:41 +0200
> 
> Eshel Yaron <me@eshelyaron.com> writes:
> 
> [...]
> 
> > This issue bothered me as well.  Here's a recipe for reproducing on
> > master, with emacs -Q:
> >
> > 1. (add-hook 'c-mode-hook 'flymake-mode)
> > 2. (add-hook 'c-mode-hook 'eglot-ensure)
> > 3. Find some C file
> >
> > This happens because Eglot _restarts_ flymake-mode while flymake-cc's
> > process is already running.  Here's a simple fix:
> >
> > diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> > index a893a8d749a..c9e1bb7b52d 100644
> > --- a/lisp/progmodes/eglot.el
> > +++ b/lisp/progmodes/eglot.el
> > @@ -2040,7 +2040,7 @@ eglot--managed-mode
> >      (unless (eglot--stay-out-of-p 'imenu)
> >        (add-function :before-until (local 'imenu-create-index-function)
> >                      #'eglot-imenu))
> > -    (unless (eglot--stay-out-of-p 'flymake) (flymake-mode 1))
> > +    (unless (or (eglot--stay-out-of-p 'flymake) flymake-mode) (flymake-mode 1))
> >      (unless (eglot--stay-out-of-p 'eldoc)
> >        (add-hook 'eldoc-documentation-functions #'eglot-hover-eldoc-function
> >                  nil t)
> 
> I realized that the change above has the downside of no longer
> immediately initiating a Flymake analysis with Eglot in place.  To
> preserve that behavior, maybe something like the following is better:
> 
> diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> index a893a8d749a..6cd48917d47 100644
> --- a/lisp/progmodes/eglot.el
> +++ b/lisp/progmodes/eglot.el
> @@ -2040,7 +2040,8 @@ eglot--managed-mode
>      (unless (eglot--stay-out-of-p 'imenu)
>        (add-function :before-until (local 'imenu-create-index-function)
>                      #'eglot-imenu))
> -    (unless (eglot--stay-out-of-p 'flymake) (flymake-mode 1))
> +    (unless (eglot--stay-out-of-p 'flymake)
> +      (if flymake-mode (flymake-start) (flymake-mode 1)))
>      (unless (eglot--stay-out-of-p 'eldoc)
>        (add-hook 'eldoc-documentation-functions #'eglot-hover-eldoc-function
>                  nil t)

Spencer, any comments?

From where I stand, this is okay for the emacs-30 release branch,
unless you think it could break some legitimate workflow.

bug#69809: 30.0.50; flymake: error in process sentinel

[Spencer Baugh]

Eli Zaretskii <eliz@gnu.org> writes:
>> From: Eshel Yaron <me@eshelyaron.com>
>> Cc: sbaugh@catern.com,  Spencer Baugh <sbaugh@janestreet.com>,  Eli
>>  Zaretskii <eliz@gnu.org>,  69809@debbugs.gnu.org
>> Date: Thu, 11 Jul 2024 13:15:41 +0200
>> 
>> Eshel Yaron <me@eshelyaron.com> writes:
>> 
>> [...]
>> 
>> > This issue bothered me as well.  Here's a recipe for reproducing on
>> > master, with emacs -Q:
>> >
>> > 1. (add-hook 'c-mode-hook 'flymake-mode)
>> > 2. (add-hook 'c-mode-hook 'eglot-ensure)
>> > 3. Find some C file
>> >
>> > This happens because Eglot _restarts_ flymake-mode while flymake-cc's
>> > process is already running.  Here's a simple fix:
>> >
>> > diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
>> > index a893a8d749a..c9e1bb7b52d 100644
>> > --- a/lisp/progmodes/eglot.el
>> > +++ b/lisp/progmodes/eglot.el
>> > @@ -2040,7 +2040,7 @@ eglot--managed-mode
>> >      (unless (eglot--stay-out-of-p 'imenu)
>> >        (add-function :before-until (local 'imenu-create-index-function)
>> >                      #'eglot-imenu))
>> > -    (unless (eglot--stay-out-of-p 'flymake) (flymake-mode 1))
>> > +    (unless (or (eglot--stay-out-of-p 'flymake) flymake-mode) (flymake-mode 1))
>> >      (unless (eglot--stay-out-of-p 'eldoc)
>> >        (add-hook 'eldoc-documentation-functions #'eglot-hover-eldoc-function
>> >                  nil t)
>> 
>> I realized that the change above has the downside of no longer
>> immediately initiating a Flymake analysis with Eglot in place.  To
>> preserve that behavior, maybe something like the following is better:
>> 
>> diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
>> index a893a8d749a..6cd48917d47 100644
>> --- a/lisp/progmodes/eglot.el
>> +++ b/lisp/progmodes/eglot.el
>> @@ -2040,7 +2040,8 @@ eglot--managed-mode
>>      (unless (eglot--stay-out-of-p 'imenu)
>>        (add-function :before-until (local 'imenu-create-index-function)
>>                      #'eglot-imenu))
>> -    (unless (eglot--stay-out-of-p 'flymake) (flymake-mode 1))
>> +    (unless (eglot--stay-out-of-p 'flymake)
>> +      (if flymake-mode (flymake-start) (flymake-mode 1)))
>>      (unless (eglot--stay-out-of-p 'eldoc)
>>        (add-hook 'eldoc-documentation-functions #'eglot-hover-eldoc-function
>>                  nil t)
>
> Spencer, any comments?
>
> From where I stand, this is okay for the emacs-30 release branch,
> unless you think it could break some legitimate workflow.

Yes, this seems good for emacs-30.  Thanks Eshel!

bug#69809: 30.0.50; flymake: error in process sentinel

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Spencer Baugh <sbaugh@janestreet.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Eshel Yaron <me@eshelyaron.com>
>>>
>>> [...]maybe something like the following is better:
>>> 
>>> diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
>>> index a893a8d749a..6cd48917d47 100644
>>> --- a/lisp/progmodes/eglot.el
>>> +++ b/lisp/progmodes/eglot.el
>>> @@ -2040,7 +2040,8 @@ eglot--managed-mode
>>>      (unless (eglot--stay-out-of-p 'imenu)
>>>        (add-function :before-until (local 'imenu-create-index-function)
>>>                      #'eglot-imenu))
>>> -    (unless (eglot--stay-out-of-p 'flymake) (flymake-mode 1))
>>> +    (unless (eglot--stay-out-of-p 'flymake)
>>> +      (if flymake-mode (flymake-start) (flymake-mode 1)))
>>>      (unless (eglot--stay-out-of-p 'eldoc)
>>>        (add-hook 'eldoc-documentation-functions #'eglot-hover-eldoc-function
>>>                  nil t)
>>
>> Spencer, any comments?
>>
>> From where I stand, this is okay for the emacs-30 release branch,
>> unless you think it could break some legitimate workflow.
>
> Yes, this seems good for emacs-30.  Thanks Eshel!

Great, thanks.  Since this is a change in eglot.el, let me also ask João
before installing: João, any objections to the change above?


Cheers,

Eshel

bug#69809: 30.0.50; flymake: error in process sentinel

[João Távora]

On Wed, Jul 17, 2024 at 7:12 AM Eshel Yaron <me@eshelyaron.com> wrote:

> > Yes, this seems good for emacs-30.  Thanks Eshel!
> Great, thanks.  Since this is a change in eglot.el, let me also ask João
> before installing: João, any objections to the change above?

I'd like to understand what problem it is solving.

João

bug#69809: 30.0.50; flymake: error in process sentinel

[João Távora]

On Wed, Jul 17, 2024 at 9:20 AM João Távora <joaotavora@gmail.com> wrote:
>
> On Wed, Jul 17, 2024 at 7:12 AM Eshel Yaron <me@eshelyaron.com> wrote:
>
> > > Yes, this seems good for emacs-30.  Thanks Eshel!
> > Great, thanks.  Since this is a change in eglot.el, let me also ask João
> > before installing: João, any objections to the change above?
>
> I'd like to understand what problem it is solving.

I've read a bit of the thread.  There seems to be an error involved,
but I didn't see a backtrace for this error.  Can someone produce it?

There's also some conjecture about interference related to
eglot-ensure.  Is it_only_ related to `eglot-ensure`?  How?

This part of Eglot is extremely delicate. I spent many hours making
sure the checks start only when they should, results of previous checks
are properly erased, etc.  This is because the Eglot Flymake backend
doesn't real work like other backends in that it cannot issue an order
to the LSP server (at least in most servers it can't) to provide diagnostics.

So I need to understand the problem and its impact in detail.

João

bug#69809: 30.0.50; flymake: error in process sentinel

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi João,

João Távora <joaotavora@gmail.com> writes:

> On Wed, Jul 17, 2024 at 9:20 AM João Távora <joaotavora@gmail.com> wrote:
>>
>> On Wed, Jul 17, 2024 at 7:12 AM Eshel Yaron <me@eshelyaron.com> wrote:
>>
>> > > Yes, this seems good for emacs-30.  Thanks Eshel!
>> > Great, thanks.  Since this is a change in eglot.el, let me also ask João
>> > before installing: João, any objections to the change above?
>>
>> I'd like to understand what problem it is solving.
>
> I've read a bit of the thread.  There seems to be an error involved,
> but I didn't see a backtrace for this error.  Can someone produce it?

Sure, here's one (also see the recipe I posted upthread):

--8<---------------cut here---------------start------------->8---
Debugger entered--Lisp error: (error "Can’t find state for flymake-cc in ‘flymake--state’")
  signal(error ("Can’t find state for flymake-cc in ‘flymake--state’"))
  error("Can't find state for %s in `flymake--state'" flymake-cc)
  (or (gethash backend flymake--state) (error "Can't find state for %s in `flymake--state'" backend))
  (let ((state (or (gethash backend flymake--state) (error "Can't find state for %s in `flymake--state'" backend))) expected-token) (cond ((null state) (flymake-error "Unexpected report from unknown backend %s" backend)) ((let* ((cl-x state)) (progn (or (let* ((cl-x cl-x)) (progn (and (memq (type-of cl-x) cl-struct-flymake--state-tags) t))) (signal 'wrong-type-argument (list 'flymake--state cl-x))) (aref cl-x 3))) (flymake-error "Unexpected report from disabled backend %s" backend)) ((progn (setq expected-token (let* ((cl-x state)) (progn (or (let* ((cl-x cl-x)) (progn (and (memq (type-of cl-x) cl-struct-flymake--state-tags) t))) (signal 'wrong-type-argument (list 'flymake--state cl-x))) (aref cl-x 1)))) (null expected-token)) (flymake-error "Unexpected report from stopped backend %s" backend)) ((not (or (eq expected-token token) force)) (flymake-error "Obsolete report from backend %s with explanation %s" backend explanation)) ((eq :panic report-action) (flymake--disable-backend backend explanation)) ((not (listp report-action)) (flymake--disable-backend backend (format "Unknown action %S" report-action)) (flymake-error "Expected report, but got unknown key %s" report-action)) (t (flymake--publish-diagnostics report-action :backend backend :state state :region region) (if flymake-check-start-time (progn (flymake--log-1 :debug 'flymake "backend %s reported %d diagnostics in %.2f second(s)" backend (length report-action) (float-time (time-since flymake-check-start-time))))))) (let* ((cl-x state)) (or (let* ((cl-x cl-x)) (progn (and (memq (type-of cl-x) cl-struct-flymake--state-tags) t))) (signal 'wrong-type-argument (list 'flymake--state cl-x))) (let* ((v cl-x)) (aset v 2 t))) (if (and flymake-show-diagnostics-at-end-of-line (not (cl-set-difference (flymake-running-backends) (flymake-reporting-backends)))) (progn (flymake--update-eol-overlays))) (flymake--update-diagnostics-listings (current-buffer)))
  (let* ((explanation (car (cdr (plist-member --cl-rest-- ':explanation)))) (force (car (cdr (plist-member --cl-rest-- ':force)))) (region (car (cdr (plist-member --cl-rest-- ':region))))) (let ((state (or (gethash backend flymake--state) (error "Can't find state for %s in `flymake--state'" backend))) expected-token) (cond ((null state) (flymake-error "Unexpected report from unknown backend %s" backend)) ((let* ((cl-x state)) (progn (or (let* ((cl-x cl-x)) (progn (and (memq (type-of cl-x) cl-struct-flymake--state-tags) t))) (signal 'wrong-type-argument (list 'flymake--state cl-x))) (aref cl-x 3))) (flymake-error "Unexpected report from disabled backend %s" backend)) ((progn (setq expected-token (let* ((cl-x state)) (progn (or (let* ((cl-x cl-x)) (progn (and (memq (type-of cl-x) cl-struct-flymake--state-tags) t))) (signal 'wrong-type-argument (list 'flymake--state cl-x))) (aref cl-x 1)))) (null expected-token)) (flymake-error "Unexpected report from stopped backend %s" backend)) ((not (or (eq expected-token token) force)) (flymake-error "Obsolete report from backend %s with explanation %s" backend explanation)) ((eq :panic report-action) (flymake--disable-backend backend explanation)) ((not (listp report-action)) (flymake--disable-backend backend (format "Unknown action %S" report-action)) (flymake-error "Expected report, but got unknown key %s" report-action)) (t (flymake--publish-diagnostics report-action :backend backend :state state :region region) (if flymake-check-start-time (progn (flymake--log-1 :debug 'flymake "backend %s reported %d diagnostics in %.2f second(s)" backend (length report-action) (float-time (time-since flymake-check-start-time))))))) (let* ((cl-x state)) (or (let* ((cl-x cl-x)) (progn (and (memq (type-of cl-x) cl-struct-flymake--state-tags) t))) (signal 'wrong-type-argument (list 'flymake--state cl-x))) (let* ((v cl-x)) (aset v 2 t))) (if (and flymake-show-diagnostics-at-end-of-line (not (cl-set-difference (flymake-running-backends) (flymake-reporting-backends)))) (progn (flymake--update-eol-overlays))) (flymake--update-diagnostics-listings (current-buffer))))
  flymake--handle-report(flymake-cc backend-token6 nil)
  apply(flymake--handle-report flymake-cc backend-token6 nil)
  (save-current-buffer (set-buffer buffer) (apply #'flymake--handle-report backend token args))
  (progn (save-current-buffer (set-buffer buffer) (apply #'flymake--handle-report backend token args)))
  (if (buffer-live-p buffer) (progn (save-current-buffer (set-buffer buffer) (apply #'flymake--handle-report backend token args))))
  #f(lambda (&rest args) [(buffer #<buffer search.c>) (token backend-token6) (backend flymake-cc)] (if (buffer-live-p buffer) (progn (save-current-buffer (set-buffer buffer) (apply #'flymake--handle-report backend token args)))))(nil)
  funcall(#f(lambda (&rest args) [(buffer #<buffer search.c>) (token backend-token6) (backend flymake-cc)] (if (buffer-live-p buffer) (progn (save-current-buffer (set-buffer buffer) (apply #'flymake--handle-report backend token args))))) nil)
  (if (or diags (= 0 (process-exit-status p))) (funcall report-fn diags) (funcall report-fn :panic :explanation (buffer-substring (point-min) (progn (goto-char (point-min)) (line-end-position)))))
  (let ((diags (flymake-cc--make-diagnostics source))) (if (or diags (= 0 (process-exit-status p))) (funcall report-fn diags) (funcall report-fn :panic :explanation (buffer-substring (point-min) (progn (goto-char (point-min)) (line-end-position))))))
  (save-current-buffer (set-buffer (process-buffer p)) (goto-char (point-min)) (let ((diags (flymake-cc--make-diagnostics source))) (if (or diags (= 0 (process-exit-status p))) (funcall report-fn diags) (funcall report-fn :panic :explanation (buffer-substring (point-min) (progn (goto-char (point-min)) (line-end-position)))))))
  (progn (save-current-buffer (set-buffer (process-buffer p)) (goto-char (point-min)) (let ((diags (flymake-cc--make-diagnostics source))) (if (or diags (= 0 (process-exit-status p))) (funcall report-fn diags) (funcall report-fn :panic :explanation (buffer-substring (point-min) (progn (goto-char (point-min)) (line-end-position))))))))
  (if (save-current-buffer (set-buffer source) (eq p flymake-cc--proc)) (progn (save-current-buffer (set-buffer (process-buffer p)) (goto-char (point-min)) (let ((diags (flymake-cc--make-diagnostics source))) (if (or diags (= 0 (process-exit-status p))) (funcall report-fn diags) (funcall report-fn :panic :explanation (buffer-substring (point-min) (progn (goto-char (point-min)) (line-end-position)))))))))
  (progn (if (save-current-buffer (set-buffer source) (eq p flymake-cc--proc)) (progn (save-current-buffer (set-buffer (process-buffer p)) (goto-char (point-min)) (let ((diags (flymake-cc--make-diagnostics source))) (if (or diags (= 0 (process-exit-status p))) (funcall report-fn diags) (funcall report-fn :panic :explanation (buffer-substring (point-min) (progn (goto-char (point-min)) (line-end-position))))))))))
  (if (eq 'exit (process-status p)) (progn (if (save-current-buffer (set-buffer source) (eq p flymake-cc--proc)) (progn (save-current-buffer (set-buffer (process-buffer p)) (goto-char (point-min)) (let ((diags (flymake-cc--make-diagnostics source))) (if (or diags (= 0 (process-exit-status p))) (funcall report-fn diags) (funcall report-fn :panic :explanation (buffer-substring (point-min) (progn (goto-char (point-min)) (line-end-position)))))))))))
  (unwind-protect (if (eq 'exit (process-status p)) (progn (if (save-current-buffer (set-buffer source) (eq p flymake-cc--proc)) (progn (save-current-buffer (set-buffer (process-buffer p)) (goto-char (point-min)) (let ((diags (flymake-cc--make-diagnostics source))) (if (or diags (= 0 (process-exit-status p))) (funcall report-fn diags) (funcall report-fn :panic :explanation (buffer-substring (point-min) (progn (goto-char (point-min)) (line-end-position))))))))))) (if (process-live-p p) nil (kill-buffer (process-buffer p))))
  #f(lambda (p _ev) [(source #<buffer search.c>) (report-fn #f(lambda (&rest args) [(buffer #<buffer search.c>) (token backend-token6) (backend flymake-cc)] (if (buffer-live-p buffer) (progn (save-current-buffer (set-buffer buffer) (apply #'flymake--handle-report backend token args))))))] (unwind-protect (if (eq 'exit (process-status p)) (progn (if (save-current-buffer (set-buffer source) (eq p flymake-cc--proc)) (progn (save-current-buffer (set-buffer (process-buffer p)) (goto-char (point-min)) (let ((diags (flymake-cc--make-diagnostics source))) (if (or diags (= 0 (process-exit-status p))) (funcall report-fn diags) (funcall report-fn :panic :explanation (buffer-substring (point-min) (progn (goto-char (point-min)) (line-end-position))))))))))) (if (process-live-p p) nil (kill-buffer (process-buffer p)))))(#<process gcc-flymake> "finished\n")
--8<---------------cut here---------------end--------------->8---

bug#69809: 30.0.50; flymake: error in process sentinel

[João Távora]

On Wed, Jul 17, 2024 at 2:08 PM Eshel Yaron <me@eshelyaron.com> wrote:
>
> Hi João,
>
> João Távora <joaotavora@gmail.com> writes:
>
> > On Wed, Jul 17, 2024 at 9:20 AM João Távora <joaotavora@gmail.com> wrote:
> >>
> >> On Wed, Jul 17, 2024 at 7:12 AM Eshel Yaron <me@eshelyaron.com> wrote:
> >>
> >> > > Yes, this seems good for emacs-30.  Thanks Eshel!
> >> > Great, thanks.  Since this is a change in eglot.el, let me also ask João
> >> > before installing: João, any objections to the change above?
> >>
> >> I'd like to understand what problem it is solving.
> >
> > I've read a bit of the thread.  There seems to be an error involved,
> > but I didn't see a backtrace for this error.  Can someone produce it?
>
> Sure, here's one (also see the recipe I posted upthread):

Thanks. Is the backtrace below what's unequivocally (or close)
produced by that recipe?

Anyway, can you try this patch?

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index e72f25fd0cd..74db9b56dd9 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -991,7 +991,7 @@ flymake--highlight-line
 ;; third-party compatibility.
 (define-obsolete-function-alias 'flymake-display-warning 'message-box "26.1")

-(defvar-local flymake--state nil
+(defvar-local flymake--state (make-hash-table)
   "State of a buffer's multiple Flymake backends.
 The keys to this hash table are functions as found in
 `flymake-diagnostic-functions'.  The values are structures
@@ -1396,7 +1396,6 @@ flymake-mode
     ;; reinitializing `flymake--state' in the next line.
     ;; See https://github.com/joaotavora/eglot/issues/223.
     (mapc #'flymake--delete-overlay (flymake--really-all-overlays))
-    (setq flymake--state (make-hash-table))
     (setq flymake--recent-changes nil)

     (when flymake-start-on-flymake-mode (flymake-start t))

bug#69809: 30.0.50; flymake: error in process sentinel

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

João Távora <joaotavora@gmail.com> writes:

> On Wed, Jul 17, 2024 at 2:08 PM Eshel Yaron <me@eshelyaron.com> wrote:
>>
>> Hi João,
>>
>> João Távora <joaotavora@gmail.com> writes:
>>
>> > On Wed, Jul 17, 2024 at 9:20 AM João Távora <joaotavora@gmail.com> wrote:
>> >>
>> >> On Wed, Jul 17, 2024 at 7:12 AM Eshel Yaron <me@eshelyaron.com> wrote:
>> >>
>> >> > > Yes, this seems good for emacs-30.  Thanks Eshel!
>> >> > Great, thanks.  Since this is a change in eglot.el, let me also ask João
>> >> > before installing: João, any objections to the change above?
>> >>
>> >> I'd like to understand what problem it is solving.
>> >
>> > I've read a bit of the thread.  There seems to be an error involved,
>> > but I didn't see a backtrace for this error.  Can someone produce it?
>>
>> Sure, here's one (also see the recipe I posted upthread):
>
> Thanks. Is the backtrace below what's unequivocally (or close)
> produced by that recipe?

Yes, that's what I see.

> Anyway, can you try this patch?

That seems to work too :)

> diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
> index e72f25fd0cd..74db9b56dd9 100644
> --- a/lisp/progmodes/flymake.el
> +++ b/lisp/progmodes/flymake.el
> @@ -991,7 +991,7 @@ flymake--highlight-line
>  ;; third-party compatibility.
>  (define-obsolete-function-alias 'flymake-display-warning 'message-box "26.1")
>
> -(defvar-local flymake--state nil
> +(defvar-local flymake--state (make-hash-table)
>    "State of a buffer's multiple Flymake backends.
>  The keys to this hash table are functions as found in
>  `flymake-diagnostic-functions'.  The values are structures
> @@ -1396,7 +1396,6 @@ flymake-mode
>      ;; reinitializing `flymake--state' in the next line.
>      ;; See https://github.com/joaotavora/eglot/issues/223.
>      (mapc #'flymake--delete-overlay (flymake--really-all-overlays))
> -    (setq flymake--state (make-hash-table))
>      (setq flymake--recent-changes nil)
>
>      (when flymake-start-on-flymake-mode (flymake-start t))

bug#69809: 30.0.50; flymake: error in process sentinel

[João Távora]

On Wed, Jul 17, 2024 at 6:25 PM Eshel Yaron <me@eshelyaron.com> wrote:
>
> João Távora <joaotavora@gmail.com> writes:
>
> > On Wed, Jul 17, 2024 at 2:08 PM Eshel Yaron <me@eshelyaron.com> wrote:
> >>
> >> Hi João,
> >>
> >> João Távora <joaotavora@gmail.com> writes:
> >>
> >> > On Wed, Jul 17, 2024 at 9:20 AM João Távora <joaotavora@gmail.com> wrote:
> >> >>
> >> >> On Wed, Jul 17, 2024 at 7:12 AM Eshel Yaron <me@eshelyaron.com> wrote:
> >> >>
> >> >> > > Yes, this seems good for emacs-30.  Thanks Eshel!
> >> >> > Great, thanks.  Since this is a change in eglot.el, let me also ask João
> >> >> > before installing: João, any objections to the change above?
> >> >>
> >> >> I'd like to understand what problem it is solving.
> >> >
> >> > I've read a bit of the thread.  There seems to be an error involved,
> >> > but I didn't see a backtrace for this error.  Can someone produce it?
> >>
> >> Sure, here's one (also see the recipe I posted upthread):
> >
> > Thanks. Is the backtrace below what's unequivocally (or close)
> > produced by that recipe?
>
> Yes, that's what I see.
>
> > Anyway, can you try this patch?
>
> That seems to work too :)

I understand the source of _this_ problem, and the line I changed
addresses it.  My worry is that my fix also creates more problems,
but it seems cleaner.  It has to be tested, particularly with Eglot reconnects.

Anyway the fix that someone proposed -- to refrain from issuing `flymake-mode`
when flymake-mode  is already active -- isn't right.  It's just
papering over a bug
waiting to appear again when someone does that in another mode hook.

The correct fix is similar to what I did, fixing the state management/cleanup
in flymake.el.  Maybe the reason for brutally resetting flymake--state doesn't
apply anymore: it doesn't seem right at all.

João

bug#69809: 30.0.50; flymake: error in process sentinel

[João Távora]

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

On Wed, Jul 17, 2024 at 6:38 PM João Távora <joaotavora@gmail.com> wrote:

> > > Anyway, can you try this patch?
> >
> > That seems to work too :)
>
> I understand the source of _this_ problem, and the line I changed
> addresses it.  My worry is that my fix also creates more problems,
> but it seems cleaner.

Indeed it did create some subtle problems with "foreign diagnostics".
I made a better patch, attached. It should fix the Eglot/flymake-cc
scenario and be a net improvement for Flymake.  Also adds a new
Flymake test.

 Spencer please have a look and push it if you agree.

João

[-- Attachment #2: 0001-Flymake-improve-idempotence-of-flymake-mode-bug-6980.patch --]
[-- Type: text/x-patch, Size: 5548 bytes --]

From bec56f895c7bc328fc49db04ea700cafcbad837c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora@gmail.com>
Date: Thu, 18 Jul 2024 00:45:20 +0100
Subject: [PATCH] Flymake: improve idempotence of flymake-mode (bug#69809)

* lisp/progmodes/flymake.el (flymake-mode): Don't smash
flymake--state.  Add some comments.  No need to check for
flymake--state nil.
(flymake--project-diagnostics): No need to check for
flymake--state nil.

* test/lisp/progmodes/flymake-tests.el (foreign-diagnostics): New
test.
---
 lisp/progmodes/flymake.el            | 34 +++++++++++++++++-----------
 test/lisp/progmodes/flymake-tests.el | 20 ++++++++++++++++
 2 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index e72f25fd0cd..93d8691838e 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -1391,12 +1391,21 @@ flymake-mode
     ;; AutoResize margins.
     (flymake--resize-margins)
 
-    ;; If Flymake happened to be already ON, we must cleanup
-    ;; existing diagnostic overlays, lest we forget them by blindly
-    ;; reinitializing `flymake--state' in the next line.
-    ;; See https://github.com/joaotavora/eglot/issues/223.
+    ;; We can't just `clrhash' `flymake--state': there may be in
+    ;; in-transit requests from other backends if `flymake-mode' was
+    ;; already active.  I.e. `flymake-mode' function should be as
+    ;; idempotent as possible.  See bug#69809.
+    (unless flymake--state (setq flymake--state (make-hash-table)))
+
+    ;; On a related note to bug#69809, deleting all Flymake overlays is
+    ;; a violation of that idempotence.  This could be addressed in the
+    ;; future.  However, there is at least one known reason for doing so
+    ;; currently: since "foreign diagnostics" are created here, we opt
+    ;; to delete everything to avoid duplicating overlays.  In
+    ;; principle, the next `flymake-start' should re-synch everything
+    ;; (and with high likelyhood that is right around the corner if
+    ;; `flymake-start-on-flymake-mode' is t).
     (mapc #'flymake--delete-overlay (flymake--really-all-overlays))
-    (setq flymake--state (make-hash-table))
     (setq flymake--recent-changes nil)
 
     (when flymake-start-on-flymake-mode (flymake-start t))
@@ -1411,14 +1420,14 @@ flymake-mode
       ;; via the brand new `flymake-mode' setup.  For simplicity's
       ;; sake, we have opted to leave the backend for now.
       nil
-      ;; 2. other buffers where a backend has created "foreign"
-      ;; diagnostics and pointed them here.  We must highlight them in
+      ;; 2. other buffers where a backend has created "foreign
+      ;; diagnostics" and pointed them here.  We must highlight them in
       ;; this buffer, i.e. create overlays for them.  Those other
       ;; buffers and backends are still responsible for them, i.e. the
       ;; current buffer does not "own" these foreign diags.
       (dolist (buffer (buffer-list))
         (with-current-buffer buffer
-          (when (and flymake-mode flymake--state)
+          (when flymake-mode
             (maphash (lambda (_backend state)
                        (maphash (lambda (file diags)
                                   (when (or (eq file source)
@@ -1446,10 +1455,9 @@ flymake-mode
       (cancel-timer flymake-timer)
       (setq flymake-timer nil))
     (mapc #'flymake--delete-overlay (flymake--really-all-overlays))
-    (when flymake--state
-      (maphash (lambda (_backend state)
-                 (flymake--clear-foreign-diags state))
-               flymake--state))))
+    (maphash (lambda (_backend state)
+               (flymake--clear-foreign-diags state))
+             flymake--state)))
    ;; turning Flymake on or off has consequences for listings
    (flymake--update-diagnostics-listings (current-buffer)))
 
@@ -2040,7 +2048,7 @@ flymake--project-diagnostics
     (cl-loop
      for buf in visited-buffers
      do (with-current-buffer buf
-          (when (and flymake-mode flymake--state)
+          (when flymake-mode
             (maphash
              (lambda (_backend state)
                (maphash
diff --git a/test/lisp/progmodes/flymake-tests.el b/test/lisp/progmodes/flymake-tests.el
index 93bc9028031..8f824ff5009 100644
--- a/test/lisp/progmodes/flymake-tests.el
+++ b/test/lisp/progmodes/flymake-tests.el
@@ -183,6 +183,26 @@ included-c-header-files
         ("no-problems.h")
       (should-error (flymake-goto-next-error nil nil t)))))
 
+(ert-deftest foreign-diagnostics ()
+  "Test Flymake in one file impacts another"
+  (skip-unless (and (executable-find "gcc")
+                    (not (ert-gcc-is-clang-p))
+                    (executable-find "make")))
+  (flymake-tests--with-flymake
+      ("another-problematic-file.c")
+    (flymake-tests--with-flymake
+        ("some-problems.h")
+      (search-forward "frob")
+      (backward-char 1)
+      (should (eq 'flymake-note (face-at-point)))
+      (let ((diags (flymake-diagnostics (point))))
+        (should (= 1 (length diags)))
+        (should (eq :note (flymake-diagnostic-type (car diags))))
+        ;; This note would never been here if it werent' a foreign
+        ;; diagnostic sourced in 'another-problematic-file.c'.
+        (should (string-match "previous declaration"
+                              (flymake-diagnostic-text (car diags))))))))
+
 (defmacro flymake-tests--assert-set (set
                                      should
                                      should-not)
-- 
2.45.2

bug#69809: 30.0.50; flymake: error in process sentinel

[João Távora]

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

And here's another more ambitious cleanup patch.
Be more careful with this one, test it with as many
Flymake backends as you can find.

On Thu, Jul 18, 2024 at 12:54 AM João Távora <joaotavora@gmail.com> wrote:
>
> On Wed, Jul 17, 2024 at 6:38 PM João Távora <joaotavora@gmail.com> wrote:
>
> > > > Anyway, can you try this patch?
> > >
> > > That seems to work too :)
> >
> > I understand the source of _this_ problem, and the line I changed
> > addresses it.  My worry is that my fix also creates more problems,
> > but it seems cleaner.
>
> Indeed it did create some subtle problems with "foreign diagnostics".
> I made a better patch, attached. It should fix the Eglot/flymake-cc
> scenario and be a net improvement for Flymake.  Also adds a new
> Flymake test.
>
>  Spencer please have a look and push it if you agree.
>
> João



-- 
João Távora

[-- Attachment #2: 0001-Flymake-more-ambitious-cleanup-in-flymake-mode-bug-6.patch --]
[-- Type: text/x-patch, Size: 6218 bytes --]

From 6def8bd5bd221ed401c843bb9c7014efb78ed28f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora@gmail.com>
Date: Thu, 18 Jul 2024 01:09:10 +0100
Subject: [PATCH] Flymake: more ambitious cleanup in flymake-mode (bug#69809)

Should be more idempotent than before, because it doesn't nuke
existing overlays.  This means multiple flymake-mode does the
same as one with minimal or no side effects, which is good for
people with lots of 'flymake-mode' in hooks.

The foreign diagnostic importation has been moved to the "really
start" section of 'flymake-start'.  The duplication problem
appears to be avoided by some heuristics in
flymake-highlight-line.

* lisp/progmodes/flymake.el (flymake--import-foreign-diagnostics): New helper
(flymake-start): Use it.
(flymake-mode): Don't nuke overlays here.
---
 lisp/progmodes/flymake.el | 77 +++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 43 deletions(-)

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index 93d8691838e..5488230ae23 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -1257,6 +1257,37 @@ flymake--recent-changes
   "Recent changes collected by `flymake-after-change-function'.")
 (defvar flymake-mode)
 
+(defun flymake--import-foreign-diagnostics ()
+  ;; Other diagnostic sources may already target this buffer's file
+  ;; before we turned on: these sources may be of two types...
+  (let ((source (current-buffer))
+        (bfn buffer-file-name))
+    ;; 1. For `flymake-list-only-diagnostics': here, we do nothing.
+    ;; FIXME: We could remove the corresponding entry from that
+    ;; variable, as we assume that new diagnostics will come in soon
+    ;; via the brand new `flymake-mode' setup.  For simplicity's
+    ;; sake, we have opted to leave the backend for now.
+    nil
+    ;; 2. other buffers where a backend has created "foreign
+    ;; diagnostics" and pointed them here.  We must highlight them in
+    ;; this buffer, i.e. create overlays for them.  Those other
+    ;; buffers and backends are still responsible for them, i.e. the
+    ;; current buffer does not "own" these foreign diags.
+    (dolist (buffer (buffer-list))
+      (with-current-buffer buffer
+        (when flymake-mode
+          (maphash (lambda (_backend state)
+                     (maphash (lambda (file diags)
+                                (when (or (eq file source)
+                                          (string= bfn (expand-file-name file)))
+                                  (with-current-buffer source
+                                    (mapc (lambda (diag)
+                                            (flymake--highlight-line diag
+                                                                     'foreign))
+                                          diags))))
+                              (flymake--state-foreign-diags state)))
+                   flymake--state))))))
+
 (defun flymake-start (&optional deferred force)
   "Start a syntax check for the current buffer.
 DEFERRED is a list of symbols designating conditions to wait for
@@ -1330,7 +1361,8 @@ flymake-start
                                  backend))
                    (t
                     (flymake--run-backend backend backend-args)))
-                  nil))))))))
+                  nil)))
+             (flymake--import-foreign-diagnostics))))))
 
 (defvar flymake-mode-map
   (let ((map (make-sparse-keymap)))
@@ -1396,49 +1428,8 @@ flymake-mode
     ;; already active.  I.e. `flymake-mode' function should be as
     ;; idempotent as possible.  See bug#69809.
     (unless flymake--state (setq flymake--state (make-hash-table)))
-
-    ;; On a related note to bug#69809, deleting all Flymake overlays is
-    ;; a violation of that idempotence.  This could be addressed in the
-    ;; future.  However, there is at least one known reason for doing so
-    ;; currently: since "foreign diagnostics" are created here, we opt
-    ;; to delete everything to avoid duplicating overlays.  In
-    ;; principle, the next `flymake-start' should re-synch everything
-    ;; (and with high likelyhood that is right around the corner if
-    ;; `flymake-start-on-flymake-mode' is t).
-    (mapc #'flymake--delete-overlay (flymake--really-all-overlays))
     (setq flymake--recent-changes nil)
-
-    (when flymake-start-on-flymake-mode (flymake-start t))
-
-    ;; Other diagnostic sources may already target this buffer's file
-    ;; before we turned on: these sources may be of two types...
-    (let ((source (current-buffer))
-          (bfn buffer-file-name))
-      ;; 1. For `flymake-list-only-diagnostics': here, we do nothing.
-      ;; FIXME: We could remove the corresponding entry from that
-      ;; variable, as we assume that new diagnostics will come in soon
-      ;; via the brand new `flymake-mode' setup.  For simplicity's
-      ;; sake, we have opted to leave the backend for now.
-      nil
-      ;; 2. other buffers where a backend has created "foreign
-      ;; diagnostics" and pointed them here.  We must highlight them in
-      ;; this buffer, i.e. create overlays for them.  Those other
-      ;; buffers and backends are still responsible for them, i.e. the
-      ;; current buffer does not "own" these foreign diags.
-      (dolist (buffer (buffer-list))
-        (with-current-buffer buffer
-          (when flymake-mode
-            (maphash (lambda (_backend state)
-                       (maphash (lambda (file diags)
-                                  (when (or (eq file source)
-                                            (string= bfn (expand-file-name file)))
-                                    (with-current-buffer source
-                                      (mapc (lambda (diag)
-                                              (flymake--highlight-line diag
-                                                                       'foreign))
-                                            diags))))
-                                (flymake--state-foreign-diags state)))
-                     flymake--state))))))
+    (when flymake-start-on-flymake-mode (flymake-start t)))
 
    ;; Turning the mode OFF.
    (t
-- 
2.45.2

bug#69809: 30.0.50; flymake: error in process sentinel

[Spencer Baugh]

João Távora <joaotavora@gmail.com> writes:
> On Wed, Jul 17, 2024 at 6:38 PM João Távora <joaotavora@gmail.com> wrote:
>
>> > > Anyway, can you try this patch?
>> >
>> > That seems to work too :)
>>
>> I understand the source of _this_ problem, and the line I changed
>> addresses it.  My worry is that my fix also creates more problems,
>> but it seems cleaner.
>
> Indeed it did create some subtle problems with "foreign diagnostics".
> I made a better patch, attached. It should fix the Eglot/flymake-cc
> scenario and be a net improvement for Flymake.  Also adds a new
> Flymake test.
>
>  Spencer please have a look and push it if you agree.

Yes, this seems good to me, thank you for the improved patch!

I unfortunately don't have commit access, so perhaps someone else can
install the patch.

bug#69809: 30.0.50; flymake: error in process sentinel

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: Eshel Yaron <me@eshelyaron.com>,  gerd.moellmann@gmail.com,  Eli
>   Zaretskii <eliz@gnu.org>,  69809@debbugs.gnu.org,  sbaugh@catern.com
> Date: Wed, 24 Jul 2024 12:25:00 -0400
> 
> >  Spencer please have a look and push it if you agree.
> 
> Yes, this seems good to me, thank you for the improved patch!
> 
> I unfortunately don't have commit access, so perhaps someone else can
> install the patch.

I tried installing the last patch posted by João, but it failed to
apply, even with the -3 option and with options that ignore whitespace
changes.

João, please either install this on the emacs-30 branch or post an
updated patch that will apply cleanly.

Thanks.

bug#69809: 30.0.50; flymake: error in process sentinel

[João Távora]

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

On Thu, Jul 25, 2024 at 8:28 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Spencer Baugh <sbaugh@janestreet.com>
> > Cc: Eshel Yaron <me@eshelyaron.com>,  gerd.moellmann@gmail.com,  Eli
> >   Zaretskii <eliz@gnu.org>,  69809@debbugs.gnu.org,  sbaugh@catern.com
> > Date: Wed, 24 Jul 2024 12:25:00 -0400
> >
> > >  Spencer please have a look and push it if you agree.
> >
> > Yes, this seems good to me, thank you for the improved patch!
> >
> > I unfortunately don't have commit access, so perhaps someone else can
> > install the patch.
>
> I tried installing the last patch posted by João, but it failed to
> apply, even with the -3 option and with options that ignore whitespace
> changes.
>
> João, please either install this on the emacs-30 branch or post an
> updated patch that will apply cleanly.
>

I posted two patches (as attached .patch files) Maybe that's the issue.
Not 100% what patches Spencer has tested or greenlighting.

[-- Attachment #2: Type: text/html, Size: 1713 bytes --]

bug#69809: 30.0.50; flymake: error in process sentinel

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Thu, 25 Jul 2024 08:45:12 +0100
> Cc: Spencer Baugh <sbaugh@janestreet.com>, me@eshelyaron.com, gerd.moellmann@gmail.com, 
> 	69809@debbugs.gnu.org, sbaugh@catern.com
> 
> On Thu, Jul 25, 2024 at 8:28 AM Eli Zaretskii <eliz@gnu.org> wrote:
> 
>  > From: Spencer Baugh <sbaugh@janestreet.com>
>  > Cc: Eshel Yaron <me@eshelyaron.com>,  gerd.moellmann@gmail.com,  Eli
>  >   Zaretskii <eliz@gnu.org>,  69809@debbugs.gnu.org,  sbaugh@catern.com
>  > Date: Wed, 24 Jul 2024 12:25:00 -0400
>  > 
>  > >  Spencer please have a look and push it if you agree.
>  > 
>  > Yes, this seems good to me, thank you for the improved patch!
>  > 
>  > I unfortunately don't have commit access, so perhaps someone else can
>  > install the patch.
> 
>  I tried installing the last patch posted by João, but it failed to
>  apply, even with the -3 option and with options that ignore whitespace
>  changes.
> 
>  João, please either install this on the emacs-30 branch or post an
>  updated patch that will apply cleanly.
> 
> I posted two patches (as attached .patch files) Maybe that's the issue.  
> Not 100% what patches Spencer has tested or greenlighting.

I tried to install the latter one, since you said it was an
improvement over the previous one.  If you suggest to install the
previous one, the one that Spencer approved, I can try doing that
instead.

bug#69809: 30.0.50; flymake: error in process sentinel

[João Távora]

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

On Thu, Jul 25, 2024 at 11:51 AM Eli Zaretskii <eliz@gnu.org> wrote:

>
> I tried to install the latter one, since you said it was an
> improvement over the previous one.  If you suggest to install the
> previous one, the one that Spencer approved, I can try doing that
> instead.
>

Yes, it's an improvement, but it needs the earlier one.  And it's "more
ambitious", i.e. I would
think it requires more testing. Tests passed as far as I remember, though.

[-- Attachment #2: Type: text/html, Size: 779 bytes --]

bug#69809: 30.0.50; flymake: error in process sentinel

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Thu, 25 Jul 2024 12:49:17 +0100
> Cc: sbaugh@janestreet.com, me@eshelyaron.com, gerd.moellmann@gmail.com, 
> 	69809@debbugs.gnu.org, sbaugh@catern.com
> 
> On Thu, Jul 25, 2024 at 11:51 AM Eli Zaretskii <eliz@gnu.org> wrote:
> 
>  I tried to install the latter one, since you said it was an
>  improvement over the previous one.  If you suggest to install the
>  previous one, the one that Spencer approved, I can try doing that
>  instead.
> 
> Yes, it's an improvement, but it needs the earlier one.  And it's "more ambitious", i.e. I would 
> think it requires more testing. Tests passed as far as I remember, though.

Feel free to install the changes you think will fix this.  If you are
not sure some changes are safe or tested enough, they should go to
master, not the emacs-30.