bug#43651: 27.1; compile.el should not parse its own header for errors

bug#43651: 27.1; compile.el should not parse its own header for errors

[David Landell]

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

Hi,

I noticed that compilation-mode parses and successfully detect errors
according to set regexp int the compilation buffer header, iow in text
inserted by the mode itself. This is weird since there isn't any error
there to visit.

Reproduce: emacs -Q -l reproduce.el

Expected output:
No error fontification of header line: Compilation started at Sun Sep 27 11:48:24

Actual output:
Error fontified line: Compilation started at Sun Sep 27 11:48:24

I noticed this in my own package rg.el which is similar to grep.el in
that it reuses compilation mode and has similar regexps for parsing
file name and line number that happens to match the "$CMD started at..."
line. The attached test case somewhat emulates what happens in those
packages. The command output is unrelated though.

Looking at the code in compile.el, my impression is that this would be
taken care of by compilation-filter but for some reason that doesn't happen.

Best regards,
David Landell


[-- Attachment #2: reproduce --]
[-- Type: application/emacs-lisp, Size: 232 bytes --]

[-- Attachment #3: Type: text/plain, Size: 4262 bytes --]






In GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.30, cairo version 1.15.10)
 of 2020-09-19 built on lgw01-amd64-054
Windowing system distributor 'The X.Org Foundation', version 11.0.11906000
System Description: Linux Mint 19

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Compilation finished
Making completion list... [2 times]

Configured using:
 'configure --build=x86_64-linux-gnu --prefix=/usr
 '--includedir=${prefix}/include' '--mandir=${prefix}/share/man'
 '--infodir=${prefix}/share/info' --sysconfdir=/etc --localstatedir=/var
 --disable-silent-rules '--libdir=${prefix}/lib/x86_64-linux-gnu'
 '--libexecdir=${prefix}/lib/x86_64-linux-gnu' --disable-maintainer-mode
 --disable-dependency-tracking --prefix=/usr --sharedstatedir=/var/lib
 --libexecdir=/usr/lib --localstatedir=/var/lib
 --infodir=/usr/share/info --mandir=/usr/share/man
 --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/27.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/27.1/site-lisp:/usr/share/emacs/site-lisp
 --program-suffix=27 --with-modules --with-file-notification=inotify
 --with-mailutils --with-harfbuzz --with-json --with-x=yes
 --with-x-toolkit=gtk3 --with-lcms2 --with-cairo --with-xpm=yes
 --with-gif=yes --with-gnutls=yes --with-jpeg=yes --with-png=yes
 --with-tiff=yes --with-xwidgets 'CFLAGS=-g -O2
 -fdebug-prefix-map=/build/emacs27-YwD3ZV/emacs27-27.1~1.git86d8d76aa3=. -fstack-protector-strong
 -Wformat -Werror=format-security -no-pie' 'CPPFLAGS=-Wdate-time
 -D_FORTIFY_SOURCE=2' 'LDFLAGS=-Wl,-Bsymbolic-functions -Wl,-z,relro
 -no-pie''

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF
ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS XWIDGETS
LIBSYSTEMD JSON PDUMPER LCMS2 GMP

Important settings:
  value of $LC_MONETARY: sv_SE.UTF-8
  value of $LC_NUMERIC: sv_SE.UTF-8
  value of $LC_TIME: sv_SE.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Emacs-Lisp

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
compile comint ansi-color ring tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads dbusbind inotify lcms2 dynamic-setting
system-font-setting font-render-setting xwidget-internal cairo
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 49811 10033)
 (symbols 48 6530 1)
 (strings 32 16934 2193)
 (string-bytes 1 560626)
 (vectors 16 10055)
 (vector-slots 8 133282 9060)
 (floats 8 23 180)
 (intervals 56 311 4)
 (buffers 1000 13))

bug#43651: Acknowledgement (27.1; compile.el should not parse its own header for errors)

[David Landell]

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

Sorry, the test case regexp was not crafted for reproducibility so
attaching a new that should be independent of the current time.


[-- Attachment #2: reproduce --]
[-- Type: application/emacs-lisp, Size: 233 bytes --]

bug#43651: 27.1; compile.el should not parse its own header for errors

[Lars Ingebrigtsen]

David Landell <david.landell@sunnyhill.email> writes:

> Expected output:
> No error fontification of header line: Compilation started at Sun Sep
> 27 11:48:24
>
> Actual output:
> Error fontified line: Compilation started at Sun Sep 27 11:48:24

I can reproduce this in Emacs 28, too, and I'm not sure how to fix it.
You'd think there'd be a way to tell the font-locking mechanism (which I
assume is used here?) to stay away from certain regions of the buffer,
but reading the documentation, I couldn't find anything.

But there surely must be.  Anybody know?

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

bug#43651: 27.1; compile.el should not parse its own header for errors

[David Landell]

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

Hi,
This is actually not restricted to font locking as demonstrated by the
test case. My initial report was a bit unclear about that. It also
affects settings like:

(setq compilation-scroll-output 'first-error)

where the jump is done to the header instead of first error. My
impression from reading the code is that we would want to avoid running
(compilation--parse-region) on the header to avoid problems like this.
I am attaching a patch that superficially seems to fix these issues.
I have a hard time judging if this fix has any bad side effects or
similar though.

I am definitely missing a bit of context how the related functions are
invoked. In testing of my own package I have seen what seems to be
multiple rounds of font locking that erase previous rounds, probably
coming from buffer changes in my downstream filter-function. In that
setup, the jump to first error has been a more stable way of reproducing.

/David


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Don-t-parse-compilation-buffer-header-for-errors.patch --]
[-- Type: text/x-diff, Size: 2816 bytes --]

From 4f30436f99537925f4ec83ed759e3ae098f63d3c Mon Sep 17 00:00:00 2001
From: David Landell <david.landell@sunnyhill.email>
Date: Sun, 27 Sep 2020 16:52:44 +0200
Subject: [PATCH] Don't parse compilation buffer header for errors

 * lisp/progmodes/compile.el (compilation-header-size): New var.
 (compilation--ensure-parse): Init compilation--parsed with start of
 process output instead of (point-min).
 (compilation-start): Calculate header size.
---
 lisp/progmodes/compile.el | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index a408d16e37..4fd2ce88bc 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -1573,7 +1573,7 @@ compilation--ensure-parse
       ;; grep.el) don't need to flush-parse when they modify the buffer
       ;; in a way that impacts buffer positions but does not require
       ;; re-parsing.
-      (setq compilation--parsed (point-min-marker)))
+      (setq compilation--parsed (copy-marker (+ (point-min) compilation-header-size))))
     (when (< compilation--parsed limit)
       (let ((start (max compilation--parsed (point-min))))
         (move-marker compilation--parsed limit)
@@ -1710,6 +1710,9 @@ compilation--update-in-progress-mode-line
   ;; buffers when it changes from nil to non-nil or vice-versa.
   (unless compilation-in-progress (force-mode-line-update t)))
 
+(defvar compilation-header-size 0
+  "Size of the inserted compilation header.")
+
 ;;;###autoload
 (defun compilation-start (command &optional mode name-function highlight-regexp)
   "Run compilation command COMMAND (low level interface).
@@ -1810,14 +1813,16 @@ compilation-start
 		(eq compilation-scroll-output 'first-error))
             (set (make-local-variable 'compilation-auto-jump-to-next) t))
 	;; Output a mode setter, for saving and later reloading this buffer.
-	(insert "-*- mode: " name-of-mode
-		"; default-directory: "
-                (prin1-to-string (abbreviate-file-name default-directory))
-		" -*-\n"
-		(format "%s started at %s\n\n"
-			mode-name
-			(substring (current-time-string) 0 19))
-		command "\n")
+        (let ((header (concat "-*- mode: " name-of-mode
+                              "; default-directory: "
+                              (prin1-to-string (abbreviate-file-name default-directory))
+                              " -*-\n"
+                              (format "%s started at %s\n\n"
+                                      mode-name
+			              (substring (current-time-string) 0 19))
+		              command "\n")))
+          (insert header)
+          (setq compilation-header-size (length header)))
 	(setq thisdir default-directory))
       (set-buffer-modified-p nil))
     ;; Pop up the compilation buffer.
-- 
2.17.1

bug#43651: 27.1; compile.el should not parse its own header for errors

[Lars Ingebrigtsen]

David Landell <david.landell@sunnyhill.email> writes:

> I am attaching a patch that superficially seems to fix these issues.
> I have a hard time judging if this fix has any bad side effects or
> similar though.

Thanks for the analysis -- your patch was basically correct, I think,
but a bit brittle.  Instead of doing the marking with a variable, I've
now made compile mark the end of the header with a text property, and
that's then used to find the end of the header when pasing.

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