GNU bug report logs - #61668
Bug in flymake-proc with fix

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 61668 in the body.
You can then email your comments to 61668 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Camden Narzt <c.narzt <at> me.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Bug in flymake-proc with fix
Date: Mon, 20 Feb 2023 11:23:02 -0700

I’m sorry if this is not the correct place to report a flymake-proc bug, but since flymake is included in the emacs git repo I figured it might be ok.

I’m currently seeing incorrect behaviour from the `flymake-proc--delete-temp-directory` function. The path is parsed and then reassembled incorrectly as the following backtrace extract demonstrates:

flymake-proc--safe-delete-directory("/private/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T/000gn/T/Users/camdennarzt/Developer/Java/getargv.java/src/main/java/cam/narzt/getargv")
flymake-proc--delete-temp-directory("/private/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T/Users/camdennarzt/Developer/Java/getargv.java/src/main/java/cam/narzt/getargv/“)

As you can see the `000gn/T/` segment of the path gets duplicated when `flymake-proc--safe-delete-directory` gets called.

This is because in `flymake-proc--delete-temp-directory` when the `suffix` variable is declared it is assumed that `(directory-file-name temporary-file-directory)` is a prefix of the `dir-name` argument, however `(directory-file-name temporary-file-directory)` doesn’t seem to resolve symlinks in the path whereas `dir-name` seems to have symlinks already resolved, so they don’t necessarily match. On my system the difference is:

(directory-file-name temporary-file-directory) → "/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T”
dir-name → "/private/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T/Users/camdennarzt/Developer/Java/getargv.java/src/main/java/cam/narzt/getargv/“

Note the "/private” prefix on the `dir-name` path. That difference in length causes the suffix to be incorrectly determined and then causes a bunch of errors while any subsequent function call tries to work with a path with the `000gn/T/` segment duplicated which obviously doesn’t exist in the fs.

Changing the suffix variable to be computed as follows fixes the bug:

(substring dir-name (1+ (length (file-truename (expand-file-name (directory-file-name temp-dir))))))


If this can be fixed that’s great, if there’s somewhere else I should report this I’d love to know.

Cheers,

Camden Narzt

Message #8 received at 61668 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Camden Narzt <c.narzt <at> me.com>, João Távora
 <joaotavora <at> gmail.com>
Cc: 61668 <at> debbugs.gnu.org
Subject: Re: bug#61668: Bug in flymake-proc with fix
Date: Tue, 21 Feb 2023 14:35:08 +0200

Adding João.

> Date: Mon, 20 Feb 2023 11:23:02 -0700
> From:  Camden Narzt via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> I’m sorry if this is not the correct place to report a flymake-proc bug, but since flymake is included in the emacs git repo I figured it might be ok.
> 
> I’m currently seeing incorrect behaviour from the `flymake-proc--delete-temp-directory` function. The path is parsed and then reassembled incorrectly as the following backtrace extract demonstrates:
> 
> flymake-proc--safe-delete-directory("/private/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T/000gn/T/Users/camdennarzt/Developer/Java/getargv.java/src/main/java/cam/narzt/getargv")
> flymake-proc--delete-temp-directory("/private/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T/Users/camdennarzt/Developer/Java/getargv.java/src/main/java/cam/narzt/getargv/“)
> 
> As you can see the `000gn/T/` segment of the path gets duplicated when `flymake-proc--safe-delete-directory` gets called.
> 
> This is because in `flymake-proc--delete-temp-directory` when the `suffix` variable is declared it is assumed that `(directory-file-name temporary-file-directory)` is a prefix of the `dir-name` argument, however `(directory-file-name temporary-file-directory)` doesn’t seem to resolve symlinks in the path whereas `dir-name` seems to have symlinks already resolved, so they don’t necessarily match. On my system the difference is:
> 
> (directory-file-name temporary-file-directory) → "/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T”
> dir-name → "/private/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T/Users/camdennarzt/Developer/Java/getargv.java/src/main/java/cam/narzt/getargv/“
> 
> Note the "/private” prefix on the `dir-name` path. That difference in length causes the suffix to be incorrectly determined and then causes a bunch of errors while any subsequent function call tries to work with a path with the `000gn/T/` segment duplicated which obviously doesn’t exist in the fs.
> 
> Changing the suffix variable to be computed as follows fixes the bug:
> 
> (substring dir-name (1+ (length (file-truename (expand-file-name (directory-file-name temp-dir))))))
> 
> 
> If this can be fixed that’s great, if there’s somewhere else I should report this I’d love to know.
> 
> Cheers,
> 
> Camden Narzt
> 
> 
> 

Message #11 received at 61668 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Camden Narzt <c.narzt <at> me.com>, 61668 <at> debbugs.gnu.org
Subject: Re: bug#61668: Bug in flymake-proc with fix
Date: Tue, 21 Feb 2023 12:42:15 +0000

Hi Camden,

Flymake-proc is a deprecated part of Flymake.  I haven't used or tested
it in a long time (there were never automated tests for it).  When I
redesigned Flymake years ago, in which flymake-proc became another
backend, I tested some basic use cases, but I didn't change the
underlying code, including the directory calculating parts and helpers.

In other words, this is not "my code" and I never understood it.

So if you think the change makes sense, go for it i.e. provide a patch
that we can push. I think there are very few users of this backend
anyway.

João

On Tue, Feb 21, 2023 at 12:35 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> Adding João.
>
> > Date: Mon, 20 Feb 2023 11:23:02 -0700
> > From:  Camden Narzt via "Bug reports for GNU Emacs,
> >  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> >
> > I’m sorry if this is not the correct place to report a flymake-proc bug, but since flymake is included in the emacs git repo I figured it might be ok.
> >
> > I’m currently seeing incorrect behaviour from the `flymake-proc--delete-temp-directory` function. The path is parsed and then reassembled incorrectly as the following backtrace extract demonstrates:
> >
> > flymake-proc--safe-delete-directory("/private/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T/000gn/T/Users/camdennarzt/Developer/Java/getargv.java/src/main/java/cam/narzt/getargv")
> > flymake-proc--delete-temp-directory("/private/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T/Users/camdennarzt/Developer/Java/getargv.java/src/main/java/cam/narzt/getargv/“)
> >
> > As you can see the `000gn/T/` segment of the path gets duplicated when `flymake-proc--safe-delete-directory` gets called.
> >
> > This is because in `flymake-proc--delete-temp-directory` when the `suffix` variable is declared it is assumed that `(directory-file-name temporary-file-directory)` is a prefix of the `dir-name` argument, however `(directory-file-name temporary-file-directory)` doesn’t seem to resolve symlinks in the path whereas `dir-name` seems to have symlinks already resolved, so they don’t necessarily match. On my system the difference is:
> >
> > (directory-file-name temporary-file-directory) → "/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T”
> > dir-name → "/private/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T/Users/camdennarzt/Developer/Java/getargv.java/src/main/java/cam/narzt/getargv/“
> >
> > Note the "/private” prefix on the `dir-name` path. That difference in length causes the suffix to be incorrectly determined and then causes a bunch of errors while any subsequent function call tries to work with a path with the `000gn/T/` segment duplicated which obviously doesn’t exist in the fs.
> >
> > Changing the suffix variable to be computed as follows fixes the bug:
> >
> > (substring dir-name (1+ (length (file-truename (expand-file-name (directory-file-name temp-dir))))))
> >
> >
> > If this can be fixed that’s great, if there’s somewhere else I should report this I’d love to know.
> >
> > Cheers,
> >
> > Camden Narzt
> >
> >
> >



-- 
João Távora

Message #14 received at 61668 <at> debbugs.gnu.org (full text, mbox):

From: Camden Narzt <c.narzt <at> me.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 61668 <at> debbugs.gnu.org
Subject: Re: bug#61668: Bug in flymake-proc with fix
Date: Tue, 21 Feb 2023 09:15:01 -0700

[Message part 1 (text/plain, inline)]

Hi João,

Yeah I don’t use flymake-proc directly, I think lsp-mode is using it or something. Anyway I’ve attached a patch.

[flymake-proc.patch (application/octet-stream, attachment)]

[Message part 3 (text/plain, inline)]

Cheers,

Camden


> On Feb 21, 2023, at 5:42 AM, João Távora <joaotavora <at> gmail.com> wrote:
> 
> Hi Camden,
> 
> Flymake-proc is a deprecated part of Flymake.  I haven't used or tested
> it in a long time (there were never automated tests for it).  When I
> redesigned Flymake years ago, in which flymake-proc became another
> backend, I tested some basic use cases, but I didn't change the
> underlying code, including the directory calculating parts and helpers.
> 
> In other words, this is not "my code" and I never understood it.
> 
> So if you think the change makes sense, go for it i.e. provide a patch
> that we can push. I think there are very few users of this backend
> anyway.
> 
> João
> 
> On Tue, Feb 21, 2023 at 12:35 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
>> 
>> Adding João.
>> 
>>> Date: Mon, 20 Feb 2023 11:23:02 -0700
>>> From:  Camden Narzt via "Bug reports for GNU Emacs,
>>> the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>>> 
>>> I’m sorry if this is not the correct place to report a flymake-proc bug, but since flymake is included in the emacs git repo I figured it might be ok.
>>> 
>>> I’m currently seeing incorrect behaviour from the `flymake-proc--delete-temp-directory` function. The path is parsed and then reassembled incorrectly as the following backtrace extract demonstrates:
>>> 
>>> flymake-proc--safe-delete-directory("/private/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T/000gn/T/Users/camdennarzt/Developer/Java/getargv.java/src/main/java/cam/narzt/getargv")
>>> flymake-proc--delete-temp-directory("/private/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T/Users/camdennarzt/Developer/Java/getargv.java/src/main/java/cam/narzt/getargv/“)
>>> 
>>> As you can see the `000gn/T/` segment of the path gets duplicated when `flymake-proc--safe-delete-directory` gets called.
>>> 
>>> This is because in `flymake-proc--delete-temp-directory` when the `suffix` variable is declared it is assumed that `(directory-file-name temporary-file-directory)` is a prefix of the `dir-name` argument, however `(directory-file-name temporary-file-directory)` doesn’t seem to resolve symlinks in the path whereas `dir-name` seems to have symlinks already resolved, so they don’t necessarily match. On my system the difference is:
>>> 
>>> (directory-file-name temporary-file-directory) → "/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T”
>>> dir-name → "/private/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T/Users/camdennarzt/Developer/Java/getargv.java/src/main/java/cam/narzt/getargv/“
>>> 
>>> Note the "/private” prefix on the `dir-name` path. That difference in length causes the suffix to be incorrectly determined and then causes a bunch of errors while any subsequent function call tries to work with a path with the `000gn/T/` segment duplicated which obviously doesn’t exist in the fs.
>>> 
>>> Changing the suffix variable to be computed as follows fixes the bug:
>>> 
>>> (substring dir-name (1+ (length (file-truename (expand-file-name (directory-file-name temp-dir))))))
>>> 
>>> 
>>> If this can be fixed that’s great, if there’s somewhere else I should report this I’d love to know.
>>> 
>>> Cheers,
>>> 
>>> Camden Narzt
>>> 
>>> 
>>> 
> 
> 
> 
> -- 
> João Távora

Message #19 received at 61668 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Camden Narzt <c.narzt <at> me.com>,
 João Távora <joaotavora <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 61668 <at> debbugs.gnu.org
Subject: Re: bug#61668: Bug in flymake-proc with fix
Date: Sun, 8 Oct 2023 16:35:49 +0000

tags 61668 + patch
thanks

Camden Narzt via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs <at> gnu.org> writes:

> Yeah I don’t use flymake-proc directly, I think lsp-mode is using it
> or something. Anyway I’ve attached a patch.

I would like to install this patch.

Could you please send the patch as an attachment instead?  We prefer
that patches are created with a command like `git format-patch -1'.
Please also include the bug number of this bug in the commit message, if
possible, like so: Bug#61668.

That would make it easier for us to review and install this patch.

Thanks in advance.

Message #22 received at 61668 <at> debbugs.gnu.org (full text, mbox):

From: Camden Narzt <c.narzt <at> me.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 João Távora <joaotavora <at> gmail.com>, 61668 <at> debbugs.gnu.org
Subject: Re: bug#61668: Bug in flymake-proc with fix
Date: Sun, 8 Oct 2023 13:54:21 -0600

[Message part 1 (text/plain, inline)]

I’ll try my best.

I’ve attached a patch I made by:
cloning https://git.savannah.gnu.org/git/emacs.git 
making the change
committing the change with a message mentioning the bug number
Running `git format-patch -1` to create the patch

Hopefully it is useful to you.

Cheers,

Camden



> On Oct 8, 2023, at 10:35 AM, Stefan Kangas <stefankangas <at> gmail.com> wrote:
> 
> tags 61668 + patch
> thanks
> 
> Camden Narzt via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs <at> gnu.org> writes:
> 
>> Yeah I don’t use flymake-proc directly, I think lsp-mode is using it
>> or something. Anyway I’ve attached a patch.
> 
> I would like to install this patch.
> 
> Could you please send the patch as an attachment instead?  We prefer
> that patches are created with a command like `git format-patch -1'.
> Please also include the bug number of this bug in the commit message, if
> possible, like so: Bug#61668.
> 
> That would make it easier for us to review and install this patch.
> 
> Thanks in advance.

[Message part 2 (text/html, inline)]

[0001-Fix-for-Bug-61668-resolve-symlinks-in-temp-dir-path.patch (application/octet-stream, attachment)]

[Message part 4 (text/html, inline)]

Message #25 received at 61668 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Camden Narzt <c.narzt <at> me.com>, 61668 <at> debbugs.gnu.org
Subject: Re: bug#61668: Bug in flymake-proc with fix
Date: Mon, 9 Oct 2023 07:45:59 +0000

Camden Narzt via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs <at> gnu.org> writes:

> I’m currently seeing incorrect behaviour from the `flymake-proc--delete-temp-directory` function. The path is parsed and then reassembled incorrectly as the following backtrace extract demonstrates:
>
> flymake-proc--safe-delete-directory("/private/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T/000gn/T/Users/camdennarzt/Developer/Java/getargv.java/src/main/java/cam/narzt/getargv")
> flymake-proc--delete-temp-directory("/private/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T/Users/camdennarzt/Developer/Java/getargv.java/src/main/java/cam/narzt/getargv/“)
>
> As you can see the `000gn/T/` segment of the path gets duplicated when `flymake-proc--safe-delete-directory` gets called.
>
> This is because in `flymake-proc--delete-temp-directory` when the `suffix` variable is declared it is assumed that `(directory-file-name temporary-file-directory)` is a prefix of the `dir-name` argument, however `(directory-file-name temporary-file-directory)` doesn’t seem to resolve symlinks in the path whereas `dir-name` seems to have symlinks already resolved, so they don’t necessarily match. On my system the difference is:
>
> (directory-file-name temporary-file-directory) → "/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T”
> dir-name → "/private/var/folders/p7/03_g5t611499lmjqhwc5tljr0000gn/T/Users/camdennarzt/Developer/Java/getargv.java/src/main/java/cam/narzt/getargv/“
>
> Note the "/private” prefix on the `dir-name` path. That difference in length causes the suffix to be incorrectly determined and then causes a bunch of errors while any subsequent function call tries to work with a path with the `000gn/T/` segment duplicated which obviously doesn’t exist in the fs.
>
> Changing the suffix variable to be computed as follows fixes the bug:
>
> (substring dir-name (1+ (length (file-truename (expand-file-name (directory-file-name temp-dir))))))

Taking a closer look, I'm not yet sure about the proposed fix.  First,
it seems like you're using macOS, which you did not mention in your bug
report.  Which version of macOS are you using?

Reducing the code down to something more minimal, I see this on my
macOS 12.7 machine.  The original code produces something like this:

    (let* ((temp-dir (file-truename temporary-file-directory))
           (dir (directory-file-name temp-dir)))
      (cons dir (length dir)))
    => ("/private/var/folders/pj/rhx0gxy15tv3vx6l3mdy0qvm0000gn/T" . 56)

This is your fix---i.e. (file-truename (expand-file-name ...))---and I
get the exact same result here:

    (let* ((temp-dir (file-truename temporary-file-directory))
           (dir (file-truename (expand-file-name
                                (directory-file-name temp-dir)))))
      (cons dir (length dir)))
    => ("/private/var/folders/pj/rhx0gxy15tv3vx6l3mdy0qvm0000gn/T" . 56)

I'm also seeing the same result if I skip the call to
`expand-file-name':

    (let* ((temp-dir (file-truename temporary-file-directory))
           (dir (file-truename (directory-file-name temp-dir))))
      (cons dir (length dir)))
    => ("/private/var/folders/pj/rhx0gxy15tv3vx6l3mdy0qvm0000gn/T" . 56)

What do the above forms evaluate to on your machine?

Thanks.

Message #28 received at 61668 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Camden Narzt <c.narzt <at> me.com>
Cc: 61668 <at> debbugs.gnu.org
Subject: Re: bug#61668: Bug in flymake-proc with fix
Date: Mon, 9 Oct 2023 14:35:23 +0000

unarchive 46203
forcemerge 46203 61668
thanks

[ Please in the future use "Reply to all" so that the discussion is
 recorded in the bug tracker. ]

Camden Narzt <c.narzt <at> me.com> writes:

> I looked at why you weren’t seeing the bug I reported, and honestly I
> don’t know, it looks like Lars Ingebrigtsen fixed this already in
> 9225599c. I don’t know why I was seeing the behaviour I was seeing.
>
> Cheers,
>
> Camden

Ah, that makes sense.  Thanks.

I'm merging this bug with #46203 then, which should also close this bug.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.