bug#10239: compilation-auto-jump-to-first-error vs. giant binary executables

bug#10239: compilation-auto-jump-to-first-error vs. giant binary executables

[jidanni]

$ cat Makefile
a:;set -Y
$ emacs -Q -eval '(setq compilation-auto-jump-to-first-error t)' -f compile
RET
/bin/sh: line 0: set: -Y: invalid option
Wham pow! We are looking at a whole megabyte of binary ELF /bin/sh executable.
I didn't try it on lager files or with ones with strings the colorizer
would try to color etc.

bug#10239: compilation-auto-jump-to-first-error vs. giant binary executables

[Lars Ingebrigtsen]

jidanni@jidanni.org writes:

> $ cat Makefile
> a:;set -Y
> $ emacs -Q -eval '(setq compilation-auto-jump-to-first-error t)' -f compile
> RET
> /bin/sh: line 0: set: -Y: invalid option
> Wham pow! We are looking at a whole megabyte of binary ELF /bin/sh executable.
> I didn't try it on lager files or with ones with strings the colorizer
> would try to color etc.

The problem is that compilation mode interprets this

/bin/sh: 1: test: -gt: unexpected operator

as an error (which is right), but believes that this error originates in
/bin/sh, so when you jump to the first error in the buffer, you're
presented with /bin/sh in a buffer, which isn't very helpful.

In addition, it identifies this line:

make: *** [Makefile:1: a] Error 2

as another error, which is even more right, but it thinks that the file
name is "*** [Makefile", which is wrong.

So I took a whack at fixing both these things with a new mechanism that
allows transformation of detected file names (and ignoring them in the
case of /bin/sh).

Patch included below.

Does this seems like a good mechanism for doing this stuff?

diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index d80fef3103..a29dc5cc47 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -58,6 +58,16 @@ compilation-window-height
   :type '(choice (const :tag "Default" nil)
 		 integer))
 
+(defcustom compilation-transform-file-match-alist
+  '(("/bin/[a-z]*sh\\'" nil)
+    ("\\*+ \\[\\(Makefile\\)" "\\1"))
+  "Alist of regexp/replacements to alter file names in compilation errors.
+If the replacement is nil, the file will not be considered an
+error after all.  If not nil, it should be a regexp replacement
+string."
+  :type '(repeat (list regexp string))
+  :version "27.1")
+
 (defvar compilation-filter-hook nil
   "Hook run after `compilation-filter' has inserted a string into the buffer.
 It is called with the variable `compilation-filter-start' bound
@@ -1155,19 +1165,36 @@ compilation-error-properties
                      (setq end-col (match-string-no-properties end-col))
                      (- (string-to-number end-col) -1)))
               (and end-line -1)))
-    (if (consp type)			; not a static type, check what it is.
+    (if (consp type)            ; not a static type, check what it is.
 	(setq type (or (and (car type) (match-end (car type)) 1)
 		       (and (cdr type) (match-end (cdr type)) 0)
 		       2)))
-
-    (when (and compilation-auto-jump-to-next
-               (>= type compilation-skip-threshold))
-      (kill-local-variable 'compilation-auto-jump-to-next)
-      (run-with-timer 0 nil 'compilation-auto-jump
-                      (current-buffer) (match-beginning 0)))
-
-    (compilation-internal-error-properties
-     file line end-line col end-col type fmt)))
+    ;; Remove matches like /bin/sh and do other file name transforms.
+    (save-match-data
+      (let ((transformed nil))
+        (dolist (f file)
+          (let ((match
+                 (cl-loop for (regexp replacement)
+                          in compilation-transform-file-match-alist
+                          when (string-match regexp f)
+                          return (or replacement t))))
+            (cond ((not match)
+                   (push f transformed))
+                  ((stringp match)
+                   (push (replace-match match nil nil f) transformed)))))
+        (setq file (nreverse transformed))))
+    (if (not file)
+        ;; If we ignored all the files with errors on this line, then
+        ;; return nil.
+        nil
+      (when (and compilation-auto-jump-to-next
+                 (>= type compilation-skip-threshold))
+        (kill-local-variable 'compilation-auto-jump-to-next)
+        (run-with-timer 0 nil 'compilation-auto-jump
+                        (current-buffer) (match-beginning 0)))
+
+      (compilation-internal-error-properties
+       file line end-line col end-col type fmt))))
 
 (defun compilation-beginning-of-line (&optional n)
   "Like `beginning-of-line', but accounts for lines hidden by `selective-display'."


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#10239: compilation-auto-jump-to-first-error vs. giant binary executables

[積丹尼 Dan Jacobson]

>>>>> "LI" == Lars Ingebrigtsen <larsi@gnus.org> writes:
LI> Patch included below.

LI> Does this seems like a good mechanism for doing this stuff?
I bet it will. Thanks for fixing it!

bug#10239: compilation-auto-jump-to-first-error vs. giant binary executables

[Andreas Schwab]

On Sep 29 2019, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
> index d80fef3103..a29dc5cc47 100644
> --- a/lisp/progmodes/compile.el
> +++ b/lisp/progmodes/compile.el
> @@ -58,6 +58,16 @@ compilation-window-height
>    :type '(choice (const :tag "Default" nil)
>  		 integer))
>  
> +(defcustom compilation-transform-file-match-alist
> +  '(("/bin/[a-z]*sh\\'" nil)
> +    ("\\*+ \\[\\(Makefile\\)" "\\1"))

That doesn't make sense.  "Makefile" can be anything.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#25329: bug#10239: compilation-auto-jump-to-first-error vs. giant binary executables

[Lars Ingebrigtsen]

Andreas Schwab <schwab@linux-m68k.org> writes:

>> +(defcustom compilation-transform-file-match-alist
>> +  '(("/bin/[a-z]*sh\\'" nil)
>> +    ("\\*+ \\[\\(Makefile\\)" "\\1"))
>
> That doesn't make sense.  "Makefile" can be anything.

I wanted to be conservative when introducing the feature.  If we decide
to add this feature, the regexps will evolve over the years.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#10239: bug#25329: bug#10239: compilation-auto-jump-to-first-error vs. giant binary executables

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>>> +(defcustom compilation-transform-file-match-alist
>>> +  '(("/bin/[a-z]*sh\\'" nil)
>>> +    ("\\*+ \\[\\(Makefile\\)" "\\1"))
>>
>> That doesn't make sense.  "Makefile" can be anything.
>
> I wanted to be conservative when introducing the feature.  If we decide
> to add this feature, the regexps will evolve over the years.

There weren't any other comments, so I've now applied the patch and I'm
closing this bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#25329: bug#10239: bug#25329: bug#10239: compilation-auto-jump-to-first-error vs. giant binary executables

[Andreas Schwab]

On Okt 10 2019, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> Andreas Schwab <schwab@linux-m68k.org> writes:
>>
>>>> +(defcustom compilation-transform-file-match-alist
>>>> +  '(("/bin/[a-z]*sh\\'" nil)
>>>> +    ("\\*+ \\[\\(Makefile\\)" "\\1"))
>>>
>>> That doesn't make sense.  "Makefile" can be anything.
>>
>> I wanted to be conservative when introducing the feature.  If we decide
>> to add this feature, the regexps will evolve over the years.
>
> There weren't any other comments, so I've now applied the patch and I'm
> closing this bug report.

This has been obsoleted by Pauls patch.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."