Subject: bug#31981: Fix 'flymake-proc-legacy-flymake' temporary file deletion bug

From: Joel Rosdahl <[email-masked]>
Date: Wed, 28 Aug 2019 21:18:33 +0200

On Wed, 28 Aug 2019 at 01:38, Noam Postavsky <[email-masked]> wrote:
> If I'm reading flymake-proc-init-create-temp-buffer-copy correctly, it
> actually makes a new temporary file (say T1), but the file name is stored in
> a single variable, so Emacs effectively forgets about the original file T.

The create-temp-f parameter to flymake-proc-init-create-temp-buffer-copy is in
practice flymake-proc-create-temp-inplace, which always creates the same result
for a given file-name and prefix. And file-name and prefix are the same for
both modification A and B (since it's the same file). Or am I missing
something?

> So here it deletes T1, leaving the original T. And then B will fail, since it
> needs T1, not T.

Nope, it's T both times, but it's true that only generating a unique filename
wouldn't fix the issue since, as you said, the filename is stored in a single
variable.

> Would it work to store the filename in the process object instead, with
> process-put (and then the sentinel can retreive it with process-get)?

Yes, flymake-proc-create-temp-inplace could create a unique filename and store
it in the process, but that wouldn't help since the cleanup function doesn't
have access to the process (since the process is stored in a single variable as
well). Passing the process or filename(s) to the cleanup function would work,
but it would break the interface to existing custom cleanup functions which
don't take a parameter, so I wrote off that solution earlier.

...unless we can pass the value(s) using dynamic binding to the cleanup
function. I didn't consider this before since lexical binding would prevent
such a solution, but I just learnt to my surprise that global defvar-ed
variables are still dynamically scoped.

Attached is a new proposed solution as sketched above.

-- Joel
From 81f885381343f4f08d0aef1fd0589e76f841bdb8 Mon Sep 17 00:00:00 2001
From: Joel Rosdahl <[email-masked]>
Date: Wed, 28 Aug 2019 21:00:00 +0200
Subject: [PATCH] Fix flymake-proc temporary file deletion bug

* list/progmodes/flymake-proc.el (flymake-proc-create-temp-inplace):
Include a time string part (hour + minute + second + nanosecond) in the
temporary name to make it unique enough.

* list/progmodes/flymake-proc.el (flymake-proc-legacy-flymake): Store
temporary file names in the process for usage in the sentinel.

* list/progmodes/flymake-proc.el (flymake-proc--process-sentinel): Bind
values of temporary file names dynamically to values stored in the
process so that the cleanup function will delete the correct temporary
file(s).

Fixes bug#31981.
---
 lisp/progmodes/flymake-proc.el | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/flymake-proc.el b/lisp/progmodes/flymake-proc.el
index 2d5a47a07978..f08ba2f36817 100644
--- a/lisp/progmodes/flymake-proc.el
+++ b/lisp/progmodes/flymake-proc.el
@@ -654,7 +654,14 @@ flymake-proc--process-sentinel
                    (let ((cleanup-f (flymake-proc--get-cleanup-function
                                      (buffer-file-name))))
                      (flymake-log 3 "cleaning up using %s" cleanup-f)
-                     (funcall cleanup-f))))
+                     ;; Make cleanup-f see the temporary file names
+                     ;; created by its corresponding init function
+                     ;; (bug#31981).
+                     (let ((flymake-proc--temp-source-file-name
+                            (process-get proc 'flymake-proc--temp-source-file-name))
+                           (flymake-proc--temp-master-file-name
+                            (process-get proc 'flymake-proc--temp-master-file-name)))
+                       (funcall cleanup-f)))))
                (kill-buffer output-buffer)))))))
 
 (defun flymake-proc--panic (problem explanation)
@@ -824,6 +831,10 @@ flymake-proc-legacy-flymake
                   (process-put proc 'flymake-proc--output-buffer
                                (generate-new-buffer
                                 (format " *flymake output for %s*" (current-buffer))))
+                  (process-put proc 'flymake-proc--temp-source-file-name
+                               flymake-proc--temp-source-file-name)
+                  (process-put proc 'flymake-proc--temp-master-file-name
+                               flymake-proc--temp-master-file-name)
                   (setq flymake-proc--current-process proc)
                   (flymake-log 2 "started process %d, command=%s, dir=%s"
                                (process-id proc) (process-command proc)
@@ -865,6 +876,7 @@ flymake-proc-create-temp-inplace
   (let* ((ext (file-name-extension file-name))
 	 (temp-name (file-truename
 		     (concat (file-name-sans-extension file-name)
+                             "_" (format-time-string "%H%M%S%N")
 			     "_" prefix
 			     (and ext (concat "." ext))))))
     (flymake-log 3 "create-temp-inplace: file=%s temp=%s" file-name temp-name)
-- 
2.20.1