unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#55056: [PATCH] Delete temporary Flymake Elisp files
@ 2022-04-21 16:42 Philip Kaludercic
  2022-04-22 11:43 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 7+ messages in thread
From: Philip Kaludercic @ 2022-04-21 16:42 UTC (permalink / raw)
  To: 55056

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

Tags: patch


My temporary directory appears to always be littered with flymake files.
The reason for this appears to be that the Elisp Flymake backend doesn't
always clean up after itself, when it should.  In the worst case I can
run out of space and that can mess up a lot of things on a system.

The below patch adds a check to the sentinel to do so.  I was a bit
paranoid and considered more process-states that might be necessary, but
I hope that someone with a better understanding of the process interface
will be able to help here.

In GNU Emacs 29.0.50 (build 7, x86_64-pc-linux-gnu, GTK+ Version 2.24.33, cairo version 1.16.0)
 of 2022-04-18 built on icterid
Repository revision: 7b1881682bfbd1ff83d47b88fa8cca22c0290c17
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Debian GNU/Linux 11 (bullseye)

Configured using:
 'configure --with-native-compilation --with-cairo --with-harfbuzz
 LDFLAGS=-flto 'CFLAGS=-march=native -mtune=native -pipe''


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Delete-temporary-Flymake-Elisp-files-every-time-a-pr.patch --]
[-- Type: text/patch, Size: 1302 bytes --]

From 26eba81b8a05e394d495336749da171e1cdf355e Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Thu, 21 Apr 2022 17:04:19 +0200
Subject: [PATCH] Delete temporary Flymake Elisp files every time a process
 ends

* elisp-mode.el (elisp-flymake-byte-compile): Handle terminal process
states besides exit by also removing the temporary file.
---
 lisp/progmodes/elisp-mode.el | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index 0dfff32f20..c99cf34db0 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -2101,7 +2101,11 @@ elisp-flymake-byte-compile
                            :explanation
                            (format "byte-compile process %s died" proc))))
               (ignore-errors (delete-file temp-file))
-              (kill-buffer output-buffer))))
+              (kill-buffer output-buffer)))
+          (when (memq (process-status proc) '(failed closed signal))
+            (message "Elisp flymake [%s]: %S" (buffer-file-name) (process-status proc))
+            (ignore-errors (delete-file temp-file))
+            (kill-buffer output-buffer)))
         :stderr " *stderr of elisp-flymake-byte-compile*"
         :noquery t)))))
 
-- 
2.30.2


[-- Attachment #3: Type: text/plain, Size: 24 bytes --]


-- 
	Philip Kaludercic

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

* bug#55056: [PATCH] Delete temporary Flymake Elisp files
  2022-04-21 16:42 bug#55056: [PATCH] Delete temporary Flymake Elisp files Philip Kaludercic
@ 2022-04-22 11:43 ` Lars Ingebrigtsen
  2022-04-22 11:57   ` João Távora
  0 siblings, 1 reply; 7+ messages in thread
From: Lars Ingebrigtsen @ 2022-04-22 11:43 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: 55056, João Távora

Philip Kaludercic <philipk@posteo.net> writes:

> The below patch adds a check to the sentinel to do so.  I was a bit
> paranoid and considered more process-states that might be necessary, but
> I hope that someone with a better understanding of the process interface
> will be able to help here.

[...]

> +          (when (memq (process-status proc) '(failed closed signal))
> +            (message "Elisp flymake [%s]: %S" (buffer-file-name) (process-status proc))
> +            (ignore-errors (delete-file temp-file))
> +            (kill-buffer output-buffer)))

I'm not very familiar with the flymake machinery, but isn't the problem
a bit further up?  That is:

        :sentinel
        (lambda (proc _event)
          (when (eq (process-status proc) 'exit)

Shouldn't that just be (unless (process-live-p proc) ...)?

Perhaps João has a comment; added to the CCs.

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





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

* bug#55056: [PATCH] Delete temporary Flymake Elisp files
  2022-04-22 11:43 ` Lars Ingebrigtsen
@ 2022-04-22 11:57   ` João Távora
  2022-04-22 12:37     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 7+ messages in thread
From: João Távora @ 2022-04-22 11:57 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Philip Kaludercic, 55056

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

Lars, your idea sounds pretty reasonable to me.  In fact, you recently
touched
the Ruby backend example checker in the manual.

commit f5b4bb4a6fa3adcb653cab5dc760745b896320bb
Author: Lars Ingebrigtsen <larsi@gnus.org>
Date:   Mon Oct 25 01:25:13 2021 +0200

    Fix flymake example backend conditions in the manual

    * doc/misc/flymake.texi (An annotated example backend): Also react
    to `signal' process statuses (bug#51380).

diff --git a/doc/misc/flymake.texi b/doc/misc/flymake.texi
--- a/doc/misc/flymake.texi
+++ b/doc/misc/flymake.texi
@@ -777,1 +777,1 @@
-          (when (eq 'exit (process-status proc))
+          (when (memq (process-status proc) '(exit signal))

The only question is why both I and you didn't venture to just use
process-live-p
back then as you propose now.  Because it seems to make sense. I personally
can't recall a reason other than my ignorance/oversight,  but maybe you can?

João

On Fri, Apr 22, 2022 at 12:43 PM Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Philip Kaludercic <philipk@posteo.net> writes:
>
> > The below patch adds a check to the sentinel to do so.  I was a bit
> > paranoid and considered more process-states that might be necessary, but
> > I hope that someone with a better understanding of the process interface
> > will be able to help here.
>
> [...]
>
> > +          (when (memq (process-status proc) '(failed closed signal))
> > +            (message "Elisp flymake [%s]: %S" (buffer-file-name)
> (process-status proc))
> > +            (ignore-errors (delete-file temp-file))
> > +            (kill-buffer output-buffer)))
>
> I'm not very familiar with the flymake machinery, but isn't the problem
> a bit further up?  That is:
>
>         :sentinel
>         (lambda (proc _event)
>           (when (eq (process-status proc) 'exit)
>
> Shouldn't that just be (unless (process-live-p proc) ...)?
>
> Perhaps João has a comment; added to the CCs.
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no
>


-- 
João Távora

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

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

* bug#55056: [PATCH] Delete temporary Flymake Elisp files
  2022-04-22 11:57   ` João Távora
@ 2022-04-22 12:37     ` Lars Ingebrigtsen
  2022-04-23 12:53       ` Lars Ingebrigtsen
  0 siblings, 1 reply; 7+ messages in thread
From: Lars Ingebrigtsen @ 2022-04-22 12:37 UTC (permalink / raw)
  To: João Távora; +Cc: Philip Kaludercic, 55056

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

> The only question is why both I and you didn't venture to just use
> process-live-p back then as you propose now.  Because it seems to make
> sense. I personally can't recall a reason other than my
> ignorance/oversight, but maybe you can?

Possibly because it's "new" -- process-live-p is from 2011, I think.

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





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

* bug#55056: [PATCH] Delete temporary Flymake Elisp files
  2022-04-22 12:37     ` Lars Ingebrigtsen
@ 2022-04-23 12:53       ` Lars Ingebrigtsen
  2022-04-25  7:34         ` Philip Kaludercic
  0 siblings, 1 reply; 7+ messages in thread
From: Lars Ingebrigtsen @ 2022-04-23 12:53 UTC (permalink / raw)
  To: João Távora; +Cc: Philip Kaludercic, 55056

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> The only question is why both I and you didn't venture to just use
>> process-live-p back then as you propose now.  Because it seems to make
>> sense. I personally can't recall a reason other than my
>> ignorance/oversight, but maybe you can?
>
> Possibly because it's "new" -- process-live-p is from 2011, I think.

So I've now done this change in Emacs 29.  Philip, does it fix the
issues you were seeing?

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





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

* bug#55056: [PATCH] Delete temporary Flymake Elisp files
  2022-04-23 12:53       ` Lars Ingebrigtsen
@ 2022-04-25  7:34         ` Philip Kaludercic
  2022-04-25  7:42           ` Lars Ingebrigtsen
  0 siblings, 1 reply; 7+ messages in thread
From: Philip Kaludercic @ 2022-04-25  7:34 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 55056, João Távora

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>>> The only question is why both I and you didn't venture to just use
>>> process-live-p back then as you propose now.  Because it seems to make
>>> sense. I personally can't recall a reason other than my
>>> ignorance/oversight, but maybe you can?
>>
>> Possibly because it's "new" -- process-live-p is from 2011, I think.
>
> So I've now done this change in Emacs 29.  Philip, does it fix the
> issues you were seeing?

Yes, I can confirm that this solves the issue as well!

-- 
	Philip Kaludercic





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

* bug#55056: [PATCH] Delete temporary Flymake Elisp files
  2022-04-25  7:34         ` Philip Kaludercic
@ 2022-04-25  7:42           ` Lars Ingebrigtsen
  0 siblings, 0 replies; 7+ messages in thread
From: Lars Ingebrigtsen @ 2022-04-25  7:42 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: 55056, João Távora

Philip Kaludercic <philipk@posteo.net> writes:

> Yes, I can confirm that this solves the issue as well!

Thanks for checking; I'm closing this bug report, then.

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





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

end of thread, other threads:[~2022-04-25  7:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-21 16:42 bug#55056: [PATCH] Delete temporary Flymake Elisp files Philip Kaludercic
2022-04-22 11:43 ` Lars Ingebrigtsen
2022-04-22 11:57   ` João Távora
2022-04-22 12:37     ` Lars Ingebrigtsen
2022-04-23 12:53       ` Lars Ingebrigtsen
2022-04-25  7:34         ` Philip Kaludercic
2022-04-25  7:42           ` Lars Ingebrigtsen

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).