GNU bug report logs - #3418
Issue with compile.el and compilation-parse-errors-filename-function

Previous Next

To reply to this bug, email your comments to 3418 AT debbugs.gnu.org.

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

Message #5 received at submit <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Gary Oberbrunner <garyo <at> genarts.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Issue with compile.el and compilation-parse-errors-filename-function
Date: Fri, 29 May 2009 10:46:51 -0400

Hi emacs folks.  I submitted a patch to compilation-get-file-structure 
in compile.el in 2001, introducing this stanza:

	;; If compilation-parse-errors-filename-function is
	;; defined, use it to process the filename.
	(when compilation-parse-errors-filename-function
	  (setq filename
		(funcall
			 filename)))

At some point since then, the filename was changed to not always be 
absolute; there's now a variable spec-directory in that function.  This 
means that implementations of compilation-parse-errors-filename-function 
can't always work correctly since it doesn't know the full path of the file.

I'm happy to work on a fix, but I see a few issues.

Solution 1: add 2nd arg SPEC-DIRECTORY to 
compilation-parse-errors-filename-function.
Problem: existing implementations will get an incorrect number of args 
error and will have to change.

Solution 2: make filename absolute before passing to 
compilation-parse-errors-filename-function.
Problem: the rest of the code is pretty careful not to absolutize the 
filename; this would change the behavior in ways I don't completely 
understand.

Of course I am personally happy with solution 1, but since it affects 
compatibility I thought I should bring it up on this list.  I am not on 
the list, so please cc me with any replies, thanks!

-- 
. . . . . . . . . . . . . . . . . . . . . . . . .
Gary Oberbrunner                garyo <at> genarts.com
GenArts, Inc.                   Tel: 617-492-2888
955 Mass. Ave                   Fax: 617-492-2852
Cambridge, MA 02139 USA         www.genarts.com

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

From: Andrew Hyatt <ahyatt <at> gmail.com>
To: Gary Oberbrunner <garyo <at> genarts.com>
Cc: 3418 <at> debbugs.gnu.org
Subject: Re: bug#3418: Issue with compile.el and
 compilation-parse-errors-filename-function
Date: Tue, 26 Jan 2016 00:21:51 -0500

Gary Oberbrunner <garyo <at> genarts.com> writes:

> Hi emacs folks.  I submitted a patch to compilation-get-file-structure in
> compile.el in 2001, introducing this stanza:
>
> 	;; If compilation-parse-errors-filename-function is
> 	;; defined, use it to process the filename.
> 	(when compilation-parse-errors-filename-function
> 	  (setq filename
> 		(funcall
> 			 filename)))
>
> At some point since then, the filename was changed to not always be absolute;
> there's now a variable spec-directory in that function.  This means that
> implementations of compilation-parse-errors-filename-function can't always work
> correctly since it doesn't know the full path of the file.
>
> I'm happy to work on a fix, but I see a few issues.
>
> Solution 1: add 2nd arg SPEC-DIRECTORY to
> compilation-parse-errors-filename-function.
> Problem: existing implementations will get an incorrect number of args error and
> will have to change.
>
> Solution 2: make filename absolute before passing to
> compilation-parse-errors-filename-function.
> Problem: the rest of the code is pretty careful not to absolutize the filename;
> this would change the behavior in ways I don't completely understand.
>
> Of course I am personally happy with solution 1, but since it affects
> compatibility I thought I should bring it up on this list.  I am not on the
> list, so please cc me with any replies, thanks!

Sadly, this bug hasn't been responded to.  Your description is pretty
code-intensive, for those of us not familiar with the internals, can you
give instructions on how to reproduce a user-visible issue?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andrew Hyatt <ahyatt <at> gmail.com>
Cc: 3418 <at> debbugs.gnu.org, garyo <at> genarts.com
Subject: Re: bug#3418: Issue with compile.el and
 compilation-parse-errors-filename-function
Date: Tue, 26 Jan 2016 16:42:58 +0200

> From: Andrew Hyatt <ahyatt <at> gmail.com>
> Date: Tue, 26 Jan 2016 00:21:51 -0500
> Cc: 3418 <at> debbugs.gnu.org
> 
> Gary Oberbrunner <garyo <at> genarts.com> writes:
> 
> > Hi emacs folks.  I submitted a patch to compilation-get-file-structure in
> > compile.el in 2001, introducing this stanza:
> >
> > 	;; If compilation-parse-errors-filename-function is
> > 	;; defined, use it to process the filename.
> > 	(when compilation-parse-errors-filename-function
> > 	  (setq filename
> > 		(funcall
> > 			 filename)))
> >
> > At some point since then, the filename was changed to not always be absolute;
> > there's now a variable spec-directory in that function.  This means that
> > implementations of compilation-parse-errors-filename-function can't always work
> > correctly since it doesn't know the full path of the file.
> >
> > I'm happy to work on a fix, but I see a few issues.
> >
> > Solution 1: add 2nd arg SPEC-DIRECTORY to
> > compilation-parse-errors-filename-function.
> > Problem: existing implementations will get an incorrect number of args error and
> > will have to change.
> >
> > Solution 2: make filename absolute before passing to
> > compilation-parse-errors-filename-function.
> > Problem: the rest of the code is pretty careful not to absolutize the filename;
> > this would change the behavior in ways I don't completely understand.
> >
> > Of course I am personally happy with solution 1, but since it affects
> > compatibility I thought I should bring it up on this list.  I am not on the
> > list, so please cc me with any replies, thanks!
> 
> Sadly, this bug hasn't been responded to.  Your description is pretty
> code-intensive, for those of us not familiar with the internals, can you
> give instructions on how to reproduce a user-visible issue?

FWIW, I don't see why not adopt Soution 1, just make the second
argument optional.  That would be backward-compatible, IIUC.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Gary Oberbrunner <garyo <at> genarts.com>
Cc: ahyatt <at> gmail.com, 3418 <at> debbugs.gnu.org
Subject: Re: bug#3418: Issue with compile.el and
 compilation-parse-errors-filename-function
Date: Tue, 26 Jan 2016 18:08:50 +0200

> Date: Tue, 26 Jan 2016 10:15:27 -0500 (EST)
> From: Gary Oberbrunner <garyo <at> genarts.com>
> Cc: Andrew Hyatt <ahyatt <at> gmail.com>, 3418 <at> debbugs.gnu.org
> 
> Wow, a blast from the past! 

Better late than never, right?

> I am totally happy with soln 1. For all I know, since I added that hook I might be the only one using it. :-) But I'm also usually a stickler for backward compatibility, so that's why I brought it up.

But if we make the additional argument optional, the backward
compatibility is preserved, right?  Or did I miss something?

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

From: Gary Oberbrunner <garyo <at> genarts.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Andrew Hyatt <ahyatt <at> gmail.com>, 3418 <at> debbugs.gnu.org
Subject: Re: bug#3418: Issue with compile.el and
 compilation-parse-errors-filename-function
Date: Tue, 26 Jan 2016 10:15:27 -0500 (EST)

Wow, a blast from the past! 

I am totally happy with soln 1. For all I know, since I added that hook I might be the only one using it. :-) But I'm also usually a stickler for backward compatibility, so that's why I brought it up.

As for how to make it happen, I'm not sure what triggers the absolute vs. relative names getting passed around in compilation-parse-errors; it probably depends on what it finds in the *compilation* buffer. But Andrew, in order for this to matter at all, the emacs user has to have added their own compilation-parse-errors-filename-function, so "normal" users wouldn't be affected by this at all.

The use case is a build system that copies/symlinks the sources to a build dir and compiles them there rather than in their original location. Compilation errors will be given relative to that build dir, not the original source. A user with such a build system would make a compilation-parse-errors-filename-function that would string-replace the build dir to the source dir, so next-error would jump to the proper source file (not the build dir copy).

----- Original Message -----
> From: "Eli Zaretskii" <eliz <at> gnu.org>
> To: "Andrew Hyatt" <ahyatt <at> gmail.com>
> Cc: "Gary Oberbrunner" <garyo <at> genarts.com>, 3418 <at> debbugs.gnu.org
> Sent: Tuesday, January 26, 2016 9:42:58 AM
> Subject: Re: bug#3418: Issue with compile.el and	compilation-parse-errors-filename-function

>> From: Andrew Hyatt <ahyatt <at> gmail.com>
>> Date: Tue, 26 Jan 2016 00:21:51 -0500
>> Cc: 3418 <at> debbugs.gnu.org
>> 
>> Gary Oberbrunner <garyo <at> genarts.com> writes:
>> 
>> > Hi emacs folks.  I submitted a patch to compilation-get-file-structure in
>> > compile.el in 2001, introducing this stanza:
>> >
>> > 	;; If compilation-parse-errors-filename-function is
>> > 	;; defined, use it to process the filename.
>> > 	(when compilation-parse-errors-filename-function
>> > 	  (setq filename
>> > 		(funcall
>> > 			 filename)))
>> >
>> > At some point since then, the filename was changed to not always be absolute;
>> > there's now a variable spec-directory in that function.  This means that
>> > implementations of compilation-parse-errors-filename-function can't always work
>> > correctly since it doesn't know the full path of the file.
>> >
>> > I'm happy to work on a fix, but I see a few issues.
>> >
>> > Solution 1: add 2nd arg SPEC-DIRECTORY to
>> > compilation-parse-errors-filename-function.
>> > Problem: existing implementations will get an incorrect number of args error and
>> > will have to change.
>> >
>> > Solution 2: make filename absolute before passing to
>> > compilation-parse-errors-filename-function.
>> > Problem: the rest of the code is pretty careful not to absolutize the filename;
>> > this would change the behavior in ways I don't completely understand.
>> >
>> > Of course I am personally happy with solution 1, but since it affects
>> > compatibility I thought I should bring it up on this list.  I am not on the
>> > list, so please cc me with any replies, thanks!
>> 
>> Sadly, this bug hasn't been responded to.  Your description is pretty
>> code-intensive, for those of us not familiar with the internals, can you
>> give instructions on how to reproduce a user-visible issue?
> 
> FWIW, I don't see why not adopt Soution 1, just make the second
> argument optional.  That would be backward-compatible, IIUC.

-- 
Gary Oberbrunner

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

From: Gary Oberbrunner <garyo <at> genarts.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Andrew Hyatt <ahyatt <at> gmail.com>, 3418 <3418 <at> debbugs.gnu.org>
Subject: Re: bug#3418: Issue with compile.el and
 compilation-parse-errors-filename-function
Date: Tue, 26 Jan 2016 11:19:41 -0500 (EST)

Hi, Eli. 
If a user (such as myself) has an implementation of this function in his .emacs today, like so:

(defun process-error-filename (filename)
  ;;; do stuff with filename
  filename)
(setq compilation-parse-errors-filename-function 'process-error-filename)

and we add a new argument that gets passed to that function, it'll throw an error. *Users* will have to add 
  &optional spec-dir 
to their implementations of it to avoid the error.

(And btw, I've already done that in mine, so I'm future-proof. :-))

-- Gary

----- Original Message -----
> From: "Eli Zaretskii" <eliz <at> gnu.org>
> To: "Gary Oberbrunner" <garyo <at> genarts.com>
> Cc: "Andrew Hyatt" <ahyatt <at> gmail.com>, "3418" <3418 <at> debbugs.gnu.org>
> Sent: Tuesday, January 26, 2016 11:08:50 AM
> Subject: Re: bug#3418: Issue with compile.el and	compilation-parse-errors-filename-function

>> Date: Tue, 26 Jan 2016 10:15:27 -0500 (EST)
>> From: Gary Oberbrunner <garyo <at> genarts.com>
>> Cc: Andrew Hyatt <ahyatt <at> gmail.com>, 3418 <at> debbugs.gnu.org
>> 
>> Wow, a blast from the past!
> 
> Better late than never, right?
> 
>> I am totally happy with soln 1. For all I know, since I added that hook I might
>> be the only one using it. :-) But I'm also usually a stickler for backward
>> compatibility, so that's why I brought it up.
> 
> But if we make the additional argument optional, the backward
> compatibility is preserved, right?  Or did I miss something?

-- 
Gary Oberbrunner

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

From: npostavs <at> users.sourceforge.net
To: Gary Oberbrunner <garyo <at> genarts.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 3418 <at> debbugs.gnu.org,
 Andrew Hyatt <ahyatt <at> gmail.com>
Subject: Re: bug#3418: Issue with compile.el and
 compilation-parse-errors-filename-function
Date: Thu, 10 Aug 2017 20:50:29 -0400

Gary Oberbrunner <garyo <at> genarts.com> writes:

> If a user (such as myself) has an implementation of this function in his .emacs today, like so:
>
> (defun process-error-filename (filename)
>   ;;; do stuff with filename
>   filename)
> (setq compilation-parse-errors-filename-function 'process-error-filename)
>
> and we add a new argument that gets passed to that function, it'll throw an error. *Users* will have to add 
>   &optional spec-dir 
> to their implementations of it to avoid the error.

We could do something like

    (condition-case err
        (funcall compilation-parse-errors-filename-function filename spec-dir)
      (wrong-number-of-arguments
       ;; Try again with single arg for backwards compatibility.
       (funcall compilation-parse-errors-filename-function filename)))

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

From: Gary Oberbrunner <garyo <at> genarts.com>
To: npostavs <at> users.sourceforge.net
Cc: Eli Zaretskii <eliz <at> gnu.org>, 3418 <at> debbugs.gnu.org,
 Andrew Hyatt <ahyatt <at> gmail.com>
Subject: Re: bug#3418: Issue with compile.el and
 compilation-parse-errors-filename-function
Date: Thu, 10 Aug 2017 21:04:47 -0400

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

This sounds perfect to me.

On Thu, Aug 10, 2017 at 8:50 PM, <npostavs <at> users.sourceforge.net> wrote:

> Gary Oberbrunner <garyo <at> genarts.com> writes:
>
> > If a user (such as myself) has an implementation of this function in his
> .emacs today, like so:
> >
> > (defun process-error-filename (filename)
> >   ;;; do stuff with filename
> >   filename)
> > (setq compilation-parse-errors-filename-function
> 'process-error-filename)
> >
> > and we add a new argument that gets passed to that function, it'll throw
> an error. *Users* will have to add
> >   &optional spec-dir
> > to their implementations of it to avoid the error.
>
> We could do something like
>
>     (condition-case err
>         (funcall compilation-parse-errors-filename-function filename
> spec-dir)
>       (wrong-number-of-arguments
>        ;; Try again with single arg for backwards compatibility.
>        (funcall compilation-parse-errors-filename-function filename)))
>



-- 
Gary Oberbrunner *--* CTO *--* Boris FX

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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: npostavs <at> users.sourceforge.net
Cc: ahyatt <at> gmail.com, 3418 <at> debbugs.gnu.org, garyo <at> genarts.com
Subject: Re: bug#3418: Issue with compile.el and
 compilation-parse-errors-filename-function
Date: Fri, 11 Aug 2017 09:34:59 +0300

> From: npostavs <at> users.sourceforge.net
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  3418 <at> debbugs.gnu.org,  Andrew Hyatt <ahyatt <at> gmail.com>
> Date: Thu, 10 Aug 2017 20:50:29 -0400
> 
> Gary Oberbrunner <garyo <at> genarts.com> writes:
> 
> > If a user (such as myself) has an implementation of this function in his .emacs today, like so:
> >
> > (defun process-error-filename (filename)
> >   ;;; do stuff with filename
> >   filename)
> > (setq compilation-parse-errors-filename-function 'process-error-filename)
> >
> > and we add a new argument that gets passed to that function, it'll throw an error. *Users* will have to add 
> >   &optional spec-dir 
> > to their implementations of it to avoid the error.
> 
> We could do something like
> 
>     (condition-case err
>         (funcall compilation-parse-errors-filename-function filename spec-dir)
>       (wrong-number-of-arguments
>        ;; Try again with single arg for backwards compatibility.
>        (funcall compilation-parse-errors-filename-function filename)))

Or use func-arity?

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

From: npostavs <at> users.sourceforge.net
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: ahyatt <at> gmail.com, 3418 <at> debbugs.gnu.org, garyo <at> genarts.com
Subject: Re: bug#3418: Issue with compile.el and
 compilation-parse-errors-filename-function
Date: Sat, 12 Aug 2017 11:42:05 -0400

Eli Zaretskii <eliz <at> gnu.org> writes:

>> We could do something like
>> 
>>     (condition-case err
>>         (funcall compilation-parse-errors-filename-function filename spec-dir)
>>       (wrong-number-of-arguments
>>        ;; Try again with single arg for backwards compatibility.
>>        (funcall compilation-parse-errors-filename-function filename)))
>
> Or use func-arity?

I think func-arity could fail in case the function is advised or created
with `apply-partially'.  On the other hand the condition-case trick can
cause problems if there is an unrelated wrong arguments error inside the
function (perhaps this can be migitated by checking the error
information).  Doesn't really matter too much either way I guess, it's
all minor corner cases.

Previous Next

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