bug#58984: 29.0.50; M-x compile misinterprets libcheck error message format

bug#58984: 29.0.50; M-x compile misinterprets libcheck error message format

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Severity: wishlist

Starting from an Emacs built with the following patches:
https://bugs.gnu.org/58975#11
https://bugs.gnu.org/58976#8

And with Check[0] installed, visit test/manual/noverlay/itree-tests.c,
flip any ck_assert condition to make it fail, and M-x compile RET.

[0]: https://libcheck.github.io/check/

The sanitized error is highlighted correctly, and compile-goto-error
jumps to the correct location:


[-- Attachment #2: 0-san.png --]
[-- Type: image/png, Size: 29303 bytes --]

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


Now remove the call to check-sanitize.sh from the Makefile, and rerun
'make'.  The error is highlighted differently, and compile-goto-error
jumps to the start of the file whose name is read interactively:


[-- Attachment #4: 1-nosan.png --]
[-- Type: image/png, Size: 23990 bytes --]

[-- Attachment #5: Type: text/plain, Size: 1934 bytes --]


(info "(check) Unit Testing in C") claims:

  The test failure messages thrown up by Check use the common idiom of
  'filename:linenumber:message' used by 'gcc' and family to report
  problems in source code.  With (X)Emacs, the output of Check allows
  one to quickly navigate to the location of the unit test that failed;
  presumably that also works in VI and IDEs.

So IWBNI this were indeed the case OOTB, at least for Check's default
CK_NORMAL print_mode verbosity level.  Here are some more message
examples from the Check manual:

check_money.c:9:F:Core:test_money_create:0: Assertion 'money_amount (m)==5' failed: money_amount (m)==0, 5==5
check_money.c:5:E:Core:test_money_create:0: (after this point) Received signal 11 (Segmentation fault)
ex_log_output.c:8:P:Core:test_pass: Test passed
ex_log_output.c:14:F:Core:test_fail: Failure
ex_log_output.c:18:E:Core:test_exit: (after this point) Early exit with return value 1
ex_log_output.c:26:P:Core:test_pass2: Test passed

Thanks,

-- 
Basil

In GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw3d scroll bars) of 2022-11-03 built on tia
Repository revision: d540cb00865368bb9df9299838006dfe09255bc6
Repository branch: blc/itree
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Debian GNU/Linux bookworm/sid

Configured using:
 'configure 'CFLAGS=-Og -ggdb3' -C --prefix=/home/blc/.local
 --enable-checking=structs --with-file-notification=yes
 --with-x-toolkit=lucid --with-x'

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

Important settings:
  value of $LANG: en_IE.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

bug#58984: 29.0.50; M-x compile misinterprets libcheck error message format

[Mattias Engdegård]

Thanks for the report. Is this bug conditional on the patches you mentioned? (Not my field of expertise if so.)

Can the bug be reproduced from a simple text file that, when presented in compilation-mode, results in the incorrect behaviour?
If not, what about a simple script that emits the text piecemeal with the correct timing for the bug to occur and that can be run in M-x compile?

(Not really going to OCR your screenshots to extract the text myself!)

bug#58984: 29.0.50; M-x compile misinterprets libcheck error message format

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Mattias Engdegård [2022-11-04 12:30 +0100] wrote:

> Is this bug conditional on the patches you mentioned?

Yes, but only inasmuch as the tests did not compile previously.  Both
referenced patches are in now, so anyone who has Check installed (in
Debian the package is called... -moments pass- ...'check') should be
able to run the tests and play with their output.

> Can the bug be reproduced from a simple text file that, when presented in
> compilation-mode, results in the incorrect behaviour?

Like this compile log, for instance?


[-- Attachment #2: errors.txt --]
[-- Type: text/plain, Size: 922 bytes --]

-*- mode: compilation; default-directory: "~/.local/src/emacs-check/test/manual/noverlay/" -*-
Compilation started at Fri Nov  4 19:00:15

make
gcc -O0 -g3 -pthread -I ../../../src  -c -o itree-tests.o itree-tests.c
gcc   itree-tests.o  -lcheck_pic -pthread -lrt -lm -lsubunit -lm -o itree-tests
./itree-tests
Running suite(s): basic
../../../src/itree.c:1359:eassert condition failed: g && g->running
92%: Checks: 51, Failures: 3, Errors: 1
itree-tests.c:71:F:insert1:test_insert_1:0: Assertion 'N_50.red' failed
itree-tests.c:91:F:insert1:test_insert_2:0: Assertion 'N_50.right != NULL' failed: N_50.right == 0
itree-tests.c:740:F:remove3:test_remove_10:0: Assertion 'tree.size != 0' failed: tree.size == 0, 0 == 0
itree-tests.c:749:E:generator:test_generator_1:0: (after this point) Early exit with return value 1
make: *** [Makefile:36: check] Error 1

Compilation exited abnormally with code 2 at Fri Nov  4 19:00:16

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


I got it from applying this diff:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: errors.diff --]
[-- Type: text/x-diff, Size: 1874 bytes --]

diff --git a/test/manual/noverlay/Makefile.in b/test/manual/noverlay/Makefile.in
index 3c8dba1ce1..c0fa705174 100644
--- a/test/manual/noverlay/Makefile.in
+++ b/test/manual/noverlay/Makefile.in
@@ -33,7 +33,7 @@ .PHONY:
 all: check
 
 check: $(PROGRAM)
-	./check-sanitize.sh ./$(PROGRAM)
+	./$(PROGRAM)
 
 itree-tests.o: emacs-compat.h $(top_srcdir)/src/itree.c $(top_srcdir)/src/itree.h
 
diff --git a/test/manual/noverlay/itree-tests.c b/test/manual/noverlay/itree-tests.c
index 278e65f9bf..e3cbaffb0f 100644
--- a/test/manual/noverlay/itree-tests.c
+++ b/test/manual/noverlay/itree-tests.c
@@ -68,8 +68,8 @@ START_TEST (test_insert_1)
    */
 
   interval_tree_insert (&tree, &N_50);
-  ck_assert (! N_50.red);
-  ck_assert_ptr_eq (&N_50, tree.root);
+  ck_assert (N_50.red);
+  ck_assert_ptr_ne (&N_50, tree.root);
 }
 END_TEST
 
@@ -88,7 +88,7 @@ START_TEST (test_insert_2)
   ck_assert_ptr_eq (&N_50, tree.root);
   ck_assert_ptr_eq (N_30.parent, &N_50);
   ck_assert_ptr_eq (N_50.left, &N_30);
-  ck_assert_ptr_null (N_50.right);
+  ck_assert_ptr_nonnull (N_50.right);
   ck_assert_ptr_null (N_30.left);
   ck_assert_ptr_null (N_30.right);
 }
@@ -737,7 +737,7 @@ START_TEST (test_remove_10)
       itree_remove (&tree, &nodes[index[i]]);
     }
   ck_assert_ptr_null (tree.root);
-  ck_assert_int_eq (tree.size, 0);
+  ck_assert_int_ne (tree.size, 0);
 }
 END_TEST
 
@@ -749,11 +749,11 @@ START_TEST (test_remove_10)
 START_TEST (test_generator_1)
 {
   struct itree_node node, *n;
-  struct itree_iterator *g;
+  struct itree_iterator *g = NULL;
   interval_tree_init (&tree);
 
   itree_insert (&tree, &node, 10, 20);
-  g = itree_iterator_start (&tree, 0, 30, ITREE_ASCENDING, NULL, 0);
+  /* g = itree_iterator_start (&tree, 0, 30, ITREE_ASCENDING, NULL, 0); */
   n = itree_iterator_next (g);
   ck_assert_ptr_eq (n, &node);
   ck_assert_int_eq (n->begin, 10);

[-- Attachment #5: Type: text/plain, Size: 487 bytes --]


Then building Emacs and finally running 'make' in the
test/manual/noverlay directory.

> If not, what about a simple script that emits the text piecemeal with the
> correct timing for the bug to occur and that can be run in M-x compile?

Let me know if the information above is insufficient and I will try
providing such a script.

> (Not really going to OCR your screenshots to extract the text myself!)

Sorry, that was just an attempt to convey fontification :).

Thanks,

-- 
Basil

bug#58984: 29.0.50; M-x compile misinterprets libcheck error message format

[Mattias Engdegård]

4 nov. 2022 kl. 18.08 skrev Basil L. Contovounesios <contovob@tcd.ie>:

> Like this compile log, for instance?

Thank you, I didn't pay enough attention. Let's look at:

> itree-tests.c:71:F:insert1:test_insert_1:0: Assertion 'N_50.red' failed

This doesn't quite conform to GNU message standards, does it? For it to have the

  [PROGRAM:]FILE:LINE: MESSAGE

form, there should be a space before the message (that is, before the 'F'). Otherwise parsing these things become almost impossible with all the possible variations, file names containing spaces and colons and so on. Currently, the machinery interprets "tree-tests.c" as the program name and "71:F:insert1:test_insert_1" as the file which is of course nonsense.

Any hope the tool can have its output format adjusted?

bug#58984: 29.0.50; M-x compile misinterprets libcheck error message format

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Mattias Engdegård [2022-11-04 18:49 +0100] wrote:

> 4 nov. 2022 kl. 18.08 skrev Basil L. Contovounesios <contovob@tcd.ie>:
>> itree-tests.c:71:F:insert1:test_insert_1:0: Assertion 'N_50.red' failed
>
> This doesn't quite conform to GNU message standards, does it? For it to have the
>
>   [PROGRAM:]FILE:LINE: MESSAGE
>
> form, there should be a space before the message (that is, before the
> 'F'). Otherwise parsing these things become almost impossible with all the
> possible variations, file names containing spaces and colons and so
> on.

I imagined as much, which is why I pinged you for comment from the
outset ;).

> Currently, the machinery interprets "tree-tests.c" as the program name and
> "71:F:insert1:test_insert_1" as the file which is of course nonsense.
>
> Any hope the tool can have its output format adjusted?

The only alternative format I'm aware of is after compiling the test
runner in CK_SUBUNIT mode instead of CK_NORMAL or CK_ENV.  I.e. with the
following patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: errors.diff --]
[-- Type: text/x-diff, Size: 2158 bytes --]

diff --git a/test/manual/noverlay/Makefile.in b/test/manual/noverlay/Makefile.in
index 3c8dba1ce1..c0fa705174 100644
--- a/test/manual/noverlay/Makefile.in
+++ b/test/manual/noverlay/Makefile.in
@@ -33,7 +33,7 @@ .PHONY:
 all: check
 
 check: $(PROGRAM)
-	./check-sanitize.sh ./$(PROGRAM)
+	./$(PROGRAM)
 
 itree-tests.o: emacs-compat.h $(top_srcdir)/src/itree.c $(top_srcdir)/src/itree.h
 
diff --git a/test/manual/noverlay/itree-tests.c b/test/manual/noverlay/itree-tests.c
index 278e65f9bf..a8c69c79be 100644
--- a/test/manual/noverlay/itree-tests.c
+++ b/test/manual/noverlay/itree-tests.c
@@ -68,8 +68,8 @@ START_TEST (test_insert_1)
    */
 
   interval_tree_insert (&tree, &N_50);
-  ck_assert (! N_50.red);
-  ck_assert_ptr_eq (&N_50, tree.root);
+  ck_assert (N_50.red);
+  ck_assert_ptr_ne (&N_50, tree.root);
 }
 END_TEST
 
@@ -88,7 +88,7 @@ START_TEST (test_insert_2)
   ck_assert_ptr_eq (&N_50, tree.root);
   ck_assert_ptr_eq (N_30.parent, &N_50);
   ck_assert_ptr_eq (N_50.left, &N_30);
-  ck_assert_ptr_null (N_50.right);
+  ck_assert_ptr_nonnull (N_50.right);
   ck_assert_ptr_null (N_30.left);
   ck_assert_ptr_null (N_30.right);
 }
@@ -737,7 +737,7 @@ START_TEST (test_remove_10)
       itree_remove (&tree, &nodes[index[i]]);
     }
   ck_assert_ptr_null (tree.root);
-  ck_assert_int_eq (tree.size, 0);
+  ck_assert_int_ne (tree.size, 0);
 }
 END_TEST
 
@@ -749,11 +749,11 @@ START_TEST (test_remove_10)
 START_TEST (test_generator_1)
 {
   struct itree_node node, *n;
-  struct itree_iterator *g;
+  struct itree_iterator *g = NULL;
   interval_tree_init (&tree);
 
   itree_insert (&tree, &node, 10, 20);
-  g = itree_iterator_start (&tree, 0, 30, ITREE_ASCENDING, NULL, 0);
+  /* g = itree_iterator_start (&tree, 0, 30, ITREE_ASCENDING, NULL, 0); */
   n = itree_iterator_next (g);
   ck_assert_ptr_eq (n, &node);
   ck_assert_int_eq (n->begin, 10);
@@ -1282,7 +1282,7 @@ main (void)
   SRunner *sr = srunner_create (s);
 
   init_itree ();
-  srunner_run_all (sr, CK_ENV);
+  srunner_run_all (sr, CK_SUBUNIT);
   int nfailed = srunner_ntests_failed (sr);
   srunner_free (sr);
   return (nfailed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;

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


I get the following output:


[-- Attachment #4: errors.txt --]
[-- Type: text/plain, Size: 4128 bytes --]

-*- mode: compilation; default-directory: "~/.local/src/emacs/test/manual/noverlay/" -*-
Compilation started at Sun Nov  6 14:08:38

make
gcc -O0 -g3 -pthread -I ../../../src  -c -o itree-tests.o itree-tests.c
gcc   itree-tests.o  -lcheck_pic -pthread -lrt -lm -lsubunit -lm -o itree-tests
./itree-tests
test: insert1:test_insert_1
failure: insert1:test_insert_1 [
itree-tests.c:71: Assertion 'N_50.red' failed
]
test: insert1:test_insert_2
failure: insert1:test_insert_2 [
itree-tests.c:91: Assertion 'N_50.right != NULL' failed: N_50.right == 0
]
test: insert1:test_insert_3
success: insert1:test_insert_3
test: insert1:test_insert_4
success: insert1:test_insert_4
test: insert1:test_insert_5
success: insert1:test_insert_5
test: insert1:test_insert_6
success: insert1:test_insert_6
test: insert2:test_insert_7
success: insert2:test_insert_7
test: insert2:test_insert_8
success: insert2:test_insert_8
test: insert2:test_insert_9
success: insert2:test_insert_9
test: insert2:test_insert_10
success: insert2:test_insert_10
test: insert2:test_insert_11
success: insert2:test_insert_11
test: insert2:test_insert_12
success: insert2:test_insert_12
test: insert3:test_insert_13
success: insert3:test_insert_13
test: insert3:test_insert_14
success: insert3:test_insert_14
test: remove1:test_remove_1
success: remove1:test_remove_1
test: remove1:test_remove_2
success: remove1:test_remove_2
test: remove1:test_remove_3
success: remove1:test_remove_3
test: remove1:test_remove_4
success: remove1:test_remove_4
test: remove2:test_remove_5
success: remove2:test_remove_5
test: remove2:test_remove_6
success: remove2:test_remove_6
test: remove2:test_remove_7
success: remove2:test_remove_7
test: remove2:test_remove_8
success: remove2:test_remove_8
test: remove3:test_remove_9
success: remove3:test_remove_9
test: remove3:test_remove_10
failure: remove3:test_remove_10 [
itree-tests.c:740: Assertion 'tree.size != 0' failed: tree.size == 0, 0 == 0
]
test: generator:test_generator_1
../../../src/itree.c:1370:eassert condition failed: g && g->running
error: generator:test_generator_1 [
itree-tests.c:749: (after this point) Early exit with return value 1
]
test: generator:test_generator_2
success: generator:test_generator_2
test: generator:test_generator_3
success: generator:test_generator_3
test: generator:test_generator_5
success: generator:test_generator_5
test: generator:test_generator_6
success: generator:test_generator_6
test: generator:test_generator_7
success: generator:test_generator_7
test: generator:test_generator_8
success: generator:test_generator_8
test: generator:test_generator_9
success: generator:test_generator_9
test: insert_gap:test_gap_insert_1
success: insert_gap:test_gap_insert_1
test: insert_gap:test_gap_insert_2
success: insert_gap:test_gap_insert_2
test: insert_gap:test_gap_insert_3
success: insert_gap:test_gap_insert_3
test: insert_gap:test_gap_insert_4
success: insert_gap:test_gap_insert_4
test: insert_gap:test_gap_insert_5
success: insert_gap:test_gap_insert_5
test: insert_gap:test_gap_insert_6
success: insert_gap:test_gap_insert_6
test: insert_gap:test_gap_insert_7
success: insert_gap:test_gap_insert_7
test: insert_gap:test_gap_insert_8
success: insert_gap:test_gap_insert_8
test: insert_gap:test_gap_insert_9
success: insert_gap:test_gap_insert_9
test: insert_gap:test_gap_insert_10
success: insert_gap:test_gap_insert_10
test: insert_gap:test_gap_insert_11
success: insert_gap:test_gap_insert_11
test: delete_gap:test_gap_delete_1
success: delete_gap:test_gap_delete_1
test: delete_gap:test_gap_delete_2
success: delete_gap:test_gap_delete_2
test: delete_gap:test_gap_delete_3
success: delete_gap:test_gap_delete_3
test: delete_gap:test_gap_delete_4
success: delete_gap:test_gap_delete_4
test: delete_gap:test_gap_delete_5
success: delete_gap:test_gap_delete_5
test: delete_gap:test_gap_delete_6
success: delete_gap:test_gap_delete_6
test: delete_gap:test_gap_delete_7
success: delete_gap:test_gap_delete_7
test: delete_gap:test_gap_delete_8
success: delete_gap:test_gap_delete_8

make: *** [Makefile:36: check] Error 1

Compilation exited abnormally with code 2 at Sun Nov  6 14:08:39

[-- Attachment #5: Type: text/plain, Size: 439 bytes --]


This is much more verbose, but the error messages are in a more readily
grokked format.  Not sure which is preferable between that and the
status quo (filtering through check-sanitize.sh).

In any case the feature request was to support Check's default output
(what we do with our small manual test suite is largely
inconsequential), so if that's too difficult and brittle for few gains,
we can close this as wontfix.

Thanks,

-- 
Basil

bug#58984: 29.0.50; M-x compile misinterprets libcheck error message format

[Mattias Engdegård]

6 nov. 2022 kl. 13.10 skrev Basil L. Contovounesios <contovob@tcd.ie>:

> The only alternative format I'm aware of is after compiling the test
> runner in CK_SUBUNIT mode instead of CK_NORMAL or CK_ENV.

If you like but I rather thought about fixing libcheck. Would something break if we added a space to the format string in `tr_str`? Ask the maintainers?

> In any case the feature request was to support Check's default output
> (what we do with our small manual test suite is largely
> inconsequential), so if that's too difficult and brittle for few gains,
> we can close this as wontfix.

You decide -- I don't think it's worth the trouble to add more contortions to the already much too complex `gnu` compilation-mode rule, if that is even possible without breaking something else or introducing major regexp backtracking points...

bug#58984: 29.0.50; M-x compile misinterprets libcheck error message format

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Mattias Engdegård [2022-11-06 15:26 +0100] wrote:

> 6 nov. 2022 kl. 13.10 skrev Basil L. Contovounesios <contovob@tcd.ie>:
>
>> The only alternative format I'm aware of is after compiling the test
>> runner in CK_SUBUNIT mode instead of CK_NORMAL or CK_ENV.
>
> If you like but I rather thought about fixing libcheck. Would
> something break if we added a space to the format string in `tr_str`?
> Ask the maintainers?

I'd have to look at the source and search their past discussions; it's
possible this has come up before.  Don't hold your breath though,
because I won't get around to this soon.  Either way I wouldn't be
particularly comfortable asking to change a default format that's
presumably been around for a while, but perhaps they can advise us
indeed.

>> In any case the feature request was to support Check's default output
>> (what we do with our small manual test suite is largely
>> inconsequential), so if that's too difficult and brittle for few gains,
>> we can close this as wontfix.
>
> You decide -- I don't think it's worth the trouble to add more contortions to
> the already much too complex `gnu` compilation-mode rule, if that is even
> possible without breaking something else or introducing major regexp
> backtracking points...

Does it have to go in the 'gnu' rule?  Is it not feasible to have it as
a separate rule?

Thanks,

-- 
Basil

bug#58984: 29.0.50; M-x compile misinterprets libcheck error message format

[Mattias Engdegård]

8 nov. 2022 kl. 19.24 skrev Basil L. Contovounesios <contovob@tcd.ie>:

> I'd have to look at the source and search their past discussions; it's
> possible this has come up before.  Don't hold your breath though,
> because I won't get around to this soon.  Either way I wouldn't be
> particularly comfortable asking to change a default format that's
> presumably been around for a while, but perhaps they can advise us
> indeed.

Yes, we usually prefer working with upstream tool developers whenever possible, rather than adapting to yet another quirky message format.
Let's not assume their format is that way by design; it could just be a simple oversight.

> Does it have to go in the 'gnu' rule?  Is it not feasible to have it as
> a separate rule?

That's a good point and it clearly can be a separate rule, but should that rule be added to the way-too-long list of patterns in Emacs or something that libcheck users need to add? Or should we include it but disabled by default?

(And would the new rule have to cope with spaces and colons in file names, duplicating most of the messiness of the gnu rule?)

Each time another rule is added to compilation-error-regexp-alist-alist I hold my breath, watching the now slightly taller tower sway uneasily...

An alternative is to post-process the output of the command that emits these messages -- a simple sed command might do.