bug#3418: Issue with compile.el and compilation-parse-errors-filename-function

[Gary Oberbrunner <garyo@genarts.com>]

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@gnu.org>
> To: "Andrew Hyatt" <ahyatt@gmail.com>
> Cc: "Gary Oberbrunner" <garyo@genarts.com>, 3418@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@gmail.com>
>> Date: Tue, 26 Jan 2016 00:21:51 -0500
>> Cc: 3418@debbugs.gnu.org
>> 
>> Gary Oberbrunner <garyo@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