Very annoying Flymake regression in Emacs 26.1

Very annoying Flymake regression in Emacs 26.1

[João Távora]

Eli, John,

I have noticed that I introduced quite an annoying regression in the
Flymake redesign that's about to make it it 26.1.

The bug happens like this. There's nothing wrong until 5.

 1. Navigate to a foo.c file
 2. Turn on M-x flymake-mode
 3. Because the legacy "proc" backend is active by default, flymake
 calls it and it makes its usual foo_flymake.c copies before doing its
 thing which is to call "CHK_SOURCES=foo_flymake.c make check-syntax"
 4. If there isn't such a target in the Makefile, or if there isn't any Makefile,
 the backend reports a failure and flymake.el considers it disabled.
 5. Nothing wrong until here, the problem is that the foo_flymake.c in
 step 3 is never cleaned up, and this starts littering the filesystem
 everytime you visit a .c file.

It's a very simple fix of moving an `unwind-protect' a level up the
tree. The diff looks big because of the indentation.  I attach it after
the sig. Sorry for noticing this so late in the game.

João

diff --git a/lisp/progmodes/flymake-proc.el b/lisp/progmodes/flymake-proc.el
index c5bb79fee6..4792a94530 100644
--- a/lisp/progmodes/flymake-proc.el
+++ b/lisp/progmodes/flymake-proc.el
@@ -772,43 +772,43 @@ flymake-proc-legacy-flymake
         (flymake-proc--clear-buildfile-cache)
         (flymake-proc--clear-project-include-dirs-cache)
 
-        (let* ((cleanup-f (flymake-proc--get-cleanup-function buffer-file-name))
-               (cmd-and-args (funcall init-f))
-               (cmd          (nth 0 cmd-and-args))
-               (args         (nth 1 cmd-and-args))
-               (dir          (nth 2 cmd-and-args))
-               (success nil))
+        (let ((cleanup-f (flymake-proc--get-cleanup-function buffer-file-name))
+              (success nil))
           (unwind-protect
-              (cond
-               ((not cmd-and-args)
-                (flymake-log 1 "init function %s for %s failed, cleaning up"
-                             init-f buffer-file-name))
-               (t
-                (setq proc
-                      (let ((default-directory (or dir default-directory)))
-                        (when dir
-                          (flymake-log 3 "starting process on dir %s" dir))
-                        (make-process
-                         :name "flymake-proc"
-                         :buffer (current-buffer)
-                         :command (cons cmd args)
-                         :noquery t
-                         :filter
-                         (lambda (proc string)
-                           (let ((flymake-proc--report-fn report-fn))
-                             (flymake-proc--process-filter proc string)))
-                         :sentinel
-                         (lambda (proc event)
-                           (let ((flymake-proc--report-fn report-fn))
-                             (flymake-proc--process-sentinel proc event))))))
-                (process-put proc 'flymake-proc--output-buffer
-                             (generate-new-buffer
-                              (format " *flymake output for %s*" (current-buffer))))
-                (setq flymake-proc--current-process proc)
-                (flymake-log 2 "started process %d, command=%s, dir=%s"
-                             (process-id proc) (process-command proc)
-                             default-directory)
-                (setq success t)))
+              (let* ((cmd-and-args (funcall init-f))
+                     (cmd          (nth 0 cmd-and-args))
+                     (args         (nth 1 cmd-and-args))
+                     (dir          (nth 2 cmd-and-args)))
+                (cond
+                 ((not cmd-and-args)
+                  (flymake-log 1 "init function %s for %s failed, cleaning up"
+                               init-f buffer-file-name))
+                 (t
+                  (setq proc
+                        (let ((default-directory (or dir default-directory)))
+                          (when dir
+                            (flymake-log 3 "starting process on dir %s" dir))
+                          (make-process
+                           :name "flymake-proc"
+                           :buffer (current-buffer)
+                           :command (cons cmd args)
+                           :noquery t
+                           :filter
+                           (lambda (proc string)
+                             (let ((flymake-proc--report-fn report-fn))
+                               (flymake-proc--process-filter proc string)))
+                           :sentinel
+                           (lambda (proc event)
+                             (let ((flymake-proc--report-fn report-fn))
+                               (flymake-proc--process-sentinel proc event))))))
+                  (process-put proc 'flymake-proc--output-buffer
+                               (generate-new-buffer
+                                (format " *flymake output for %s*" (current-buffer))))
+                  (setq flymake-proc--current-process proc)
+                  (flymake-log 2 "started process %d, command=%s, dir=%s"
+                               (process-id proc) (process-command proc)
+                               default-directory)
+                  (setq success t))))
             (unless success
               (funcall cleanup-f))))))))
 

Re: Very annoying Flymake regression in Emacs 26.1

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Fri, 11 May 2018 22:35:50 +0100
> 
>  1. Navigate to a foo.c file
>  2. Turn on M-x flymake-mode
>  3. Because the legacy "proc" backend is active by default, flymake
>  calls it and it makes its usual foo_flymake.c copies before doing its
>  thing which is to call "CHK_SOURCES=foo_flymake.c make check-syntax"
>  4. If there isn't such a target in the Makefile, or if there isn't any Makefile,
>  the backend reports a failure and flymake.el considers it disabled.
>  5. Nothing wrong until here, the problem is that the foo_flymake.c in
>  step 3 is never cleaned up, and this starts littering the filesystem
>  everytime you visit a .c file.

Is the above scenario the usual way of using Flymake, or is it some
variation that could be rare?  If it isn't rare, I wonder how come no
one noticed until now that we are littering the filesystem with these
files.

Thanks.

Re: Very annoying Flymake regression in Emacs 26.1

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

> Is the above scenario the usual way of using Flymake, or is it some
> variation that could be rare?

M-x flymake and (add-hook 'c-mode-hook 'flymake-mode) are the two most
mundane ways I can think of for starting flymake.

I've dug a little more and it happens when the visited file is in a
project without a dominating file called "Makefile". So I wrote

> "If there isn't such a target in the Makefile, or if there isn't any Makefile"

But only the second part is true. But it's still pretty common. Notice
for example, how that includes visiting files in a clean Emacs checkout
before the configure step.

> If it isn't rare, I wonder how come no one noticed until now that we
> are littering the filesystem with these

Good question! I guess few people are using it, or they're using it
with special care, or they are used to the _flymake.c garbage
anyway because the old flymake used to make it from time to time (though
not in such a trivial case)

João

Re: Very annoying Flymake regression in Emacs 26.1

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Cc: emacs-devel@gnu.org,  johnw@gnu.org
> Date: Sat, 12 May 2018 10:01:43 +0100
> 
> > If it isn't rare, I wonder how come no one noticed until now that we
> > are littering the filesystem with these
> 
> Good question! I guess few people are using it, or they're using it
> with special care, or they are used to the _flymake.c garbage
> anyway because the old flymake used to make it from time to time (though
> not in such a trivial case)

OK, please cherry-pick the commit you made to master to the release
branch.

Thanks.

Re: Very annoying Flymake regression in Emacs 26.1

[João Távora]

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

Done, it's 934bb475b9a729d0be4d78cd89c1d22d032ee3d7 in
emacs-26 (which I hope is the "release branch" you're talking of
, if it isn't please let me know).

In master I will later address the cause of the error, which is a bug in
itself, but this needn't make it to the release branch.

João

On Sat, May 12, 2018 at 10:33 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: João Távora <joaotavora@gmail.com>
> > Cc: emacs-devel@gnu.org,  johnw@gnu.org
> > Date: Sat, 12 May 2018 10:01:43 +0100
> >
> > > If it isn't rare, I wonder how come no one noticed until now that we
> > > are littering the filesystem with these
> >
> > Good question! I guess few people are using it, or they're using it
> > with special care, or they are used to the _flymake.c garbage
> > anyway because the old flymake used to make it from time to time (though
> > not in such a trivial case)
>
> OK, please cherry-pick the commit you made to master to the release
> branch.
>
> Thanks.
>



-- 
João Távora

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

Re: Very annoying Flymake regression in Emacs 26.1

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Sat, 12 May 2018 11:41:07 +0100
> Cc: emacs-devel <emacs-devel@gnu.org>, johnw@gnu.org
> 
> Done, it's 934bb475b9a729d0be4d78cd89c1d22d032ee3d7 in
> emacs-26 (which I hope is the "release branch" you're talking of
> , if it isn't please let me know).

Thanks, that's the right branch.

Re: Very annoying Flymake regression in Emacs 26.1

[John Wiegley]

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

> Done, it's 934bb475b9a729d0be4d78cd89c1d22d032ee3d7 in
> emacs-26 (which I hope is the "release branch" you're talking of
> , if it isn't please let me know).

> In master I will later address the cause of the error, which is a bug in 
> itself, but this needn't make it to the release branch.

Thank you, João.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2