[PATCH] gdb/testsuite: fix gdb.guile/scm-parameter.exp "wrong type argument" test pattern

[PATCH] gdb/testsuite: fix gdb.guile/scm-parameter.exp "wrong type argument" test pattern

[Simon Marchi]

Since commit 90319cefe3 ("GDB/Guile: Don't assert that an integer value
is boolean"), I see:

    FAIL: gdb.guile/scm-parameter.exp: kind=PARAM_ZINTEGER: test-PARAM_ZINTEGER-param: guile (set-parameter-value! test-PARAM_ZINTEGER-param #:unlimited)
    FAIL: gdb.guile/scm-parameter.exp: kind=PARAM_ZUINTEGER: test-PARAM_ZUINTEGER-param: guile (set-parameter-value! test-PARAM_ZUINTEGER-param #:unlimited)

This comes from the fact that GDB outputs this:

    ERROR: In procedure set-parameter-value!:
    In procedure gdbscm_set_parameter_value_x: Wrong type argument in position 2 (expecting integer): #:unlimited
    Error while executing Scheme code.

while the test expects an additional "ERROR:" on the second line,
something like this:

    ERROR: In procedure set-parameter-value!:
    ERROR: In procedure gdbscm_set_parameter_value_x: Wrong type argument in position 2 (expecting integer): #:unlimited
    Error while executing Scheme code.

The patch below fixes the test for me.

I believe that the first two lines are output by Guile itself, in the
SCM_ASSERT_TYPE macro.  I tried on different systems, different Guile
versions (2.0, 2.2 and 3.0) and I always get the former output, never
the output the test expects.  I presume the patch below isn't right, as
there is surely some systems that do print the latter output, otherwise
Maciej (the original author) would have noticed it.  I presume we'll
need to accept both outputs.  But I'd like we if could clarify when we
get which.

Change-Id: I9dc45e7492a4f08340cad974610242ed689de959
---
 gdb/testsuite/gdb.guile/scm-parameter.exp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.guile/scm-parameter.exp b/gdb/testsuite/gdb.guile/scm-parameter.exp
index b9f2d825211..5d72da935bc 100644
--- a/gdb/testsuite/gdb.guile/scm-parameter.exp
+++ b/gdb/testsuite/gdb.guile/scm-parameter.exp
@@ -183,10 +183,10 @@ foreach_with_prefix kind {
 	"end"
 
     set param_integer_error \
-	"ERROR: In procedure set-parameter-value!:\r\nERROR: In procedure\
-	 gdbscm_set_parameter_value_x: Wrong type argument in position 2\
-	 \\(expecting integer\\): #:unlimited\r\nError while executing Scheme\
-	 code\\."
+	[multi_line \
+	    "ERROR: In procedure set-parameter-value!:" \
+	    "In procedure gdbscm_set_parameter_value_x: Wrong type argument in position 2 \\(expecting integer\\): #:unlimited" \
+	    "Error while executing Scheme code\\."]
     set param_minus_one_error "integer -1 out of range"
     set param_minus_two_range "integer -2 out of range"
     set param_minus_two_unlimited "only -1 is allowed to set as unlimited"
-- 
2.37.3

Re: [PATCH] gdb/testsuite: fix gdb.guile/scm-parameter.exp "wrong type argument" test pattern

[Maciej W. Rozycki]

On Mon, 24 Oct 2022, Simon Marchi wrote:

> I believe that the first two lines are output by Guile itself, in the
> SCM_ASSERT_TYPE macro.  I tried on different systems, different Guile
> versions (2.0, 2.2 and 3.0) and I always get the former output, never
> the output the test expects.  I presume the patch below isn't right, as
> there is surely some systems that do print the latter output, otherwise
> Maciej (the original author) would have noticed it.  I presume we'll
> need to accept both outputs.  But I'd like we if could clarify when we
> get which.

 FTR I'm still looking into it and like you I have hesitated to just paper 
the issue over by allowing both outputs without first understanding what 
is really going on here.  I cannot rule out a distribution-specific patch 
causing a discrepancy here, but I feel like tracking it down.

 NB guile 2.0.13 here, reporting as:

guile (GNU Guile) 2.0.13
Packaged by Debian (2.0.13-deb+1-5.4)

(and it seems like I have said version consistently throughout my relevant 
development machines except for different "-deb+..." suffixes).  It must 
have had long distribution maintenance history.

  Maciej

Re: [PATCH] gdb/testsuite: fix gdb.guile/scm-parameter.exp "wrong type argument" test pattern

[Simon Marchi]

On 2022-10-24 19:22, Maciej W. Rozycki wrote:
> On Mon, 24 Oct 2022, Simon Marchi wrote:
> 
>> I believe that the first two lines are output by Guile itself, in the
>> SCM_ASSERT_TYPE macro.  I tried on different systems, different Guile
>> versions (2.0, 2.2 and 3.0) and I always get the former output, never
>> the output the test expects.  I presume the patch below isn't right, as
>> there is surely some systems that do print the latter output, otherwise
>> Maciej (the original author) would have noticed it.  I presume we'll
>> need to accept both outputs.  But I'd like we if could clarify when we
>> get which.
> 
>  FTR I'm still looking into it and like you I have hesitated to just paper 
> the issue over by allowing both outputs without first understanding what 
> is really going on here.  I cannot rule out a distribution-specific patch 
> causing a discrepancy here, but I feel like tracking it down.
> 
>  NB guile 2.0.13 here, reporting as:
> 
> guile (GNU Guile) 2.0.13
> Packaged by Debian (2.0.13-deb+1-5.4)

According to that version number, it looks like Ubuntu 20.04?

  https://packages.ubuntu.com/focal/guile-2.0

I tried building on Ubuntu 20.04 against guile-2.0, and I see the same
result as you.  And when I try guile2.0 on Arch Linux (this package
[1]), I also see the same result as you.  So I must have tested it wrong
previously.

You can dig further if you want, but I'd be fine just accepting both
outputs and saying that guile-2.0 outputs the additional ERROR: while
subsequent versions do not.

Simon

[1] https://aur.archlinux.org/packages/guile2.0

Re: [PATCH] gdb/testsuite: fix gdb.guile/scm-parameter.exp "wrong type argument" test pattern

[Tom de Vries]

On 10/25/22 03:08, Simon Marchi via Gdb-patches wrote:
> 
> 
> On 2022-10-24 19:22, Maciej W. Rozycki wrote:
>> On Mon, 24 Oct 2022, Simon Marchi wrote:
>>
>>> I believe that the first two lines are output by Guile itself, in the
>>> SCM_ASSERT_TYPE macro.  I tried on different systems, different Guile
>>> versions (2.0, 2.2 and 3.0) and I always get the former output, never
>>> the output the test expects.  I presume the patch below isn't right, as
>>> there is surely some systems that do print the latter output, otherwise
>>> Maciej (the original author) would have noticed it.  I presume we'll
>>> need to accept both outputs.  But I'd like we if could clarify when we
>>> get which.
>>
>>   FTR I'm still looking into it and like you I have hesitated to just paper
>> the issue over by allowing both outputs without first understanding what
>> is really going on here.  I cannot rule out a distribution-specific patch
>> causing a discrepancy here, but I feel like tracking it down.
>>
>>   NB guile 2.0.13 here, reporting as:
>>
>> guile (GNU Guile) 2.0.13
>> Packaged by Debian (2.0.13-deb+1-5.4)
> 
> According to that version number, it looks like Ubuntu 20.04?
> 
>    https://packages.ubuntu.com/focal/guile-2.0
> 
> I tried building on Ubuntu 20.04 against guile-2.0, and I see the same
> result as you.  And when I try guile2.0 on Arch Linux (this package
> [1]), I also see the same result as you.  So I must have tested it wrong
> previously.
> 
> You can dig further if you want, but I'd be fine just accepting both
> outputs and saying that guile-2.0 outputs the additional ERROR: while
> subsequent versions do not.
> 

FWIW, I did the same here in commit 6bbe1a929c6 ("[gdb/testsuite] Fix 
gdb.guile/scm-breakpoint.exp with guile 3.0").

Thanks,
- Tom

Re: [PATCH] gdb/testsuite: fix gdb.guile/scm-parameter.exp "wrong type argument" test pattern

[Simon Marchi]

On 10/26/22 03:15, Tom de Vries wrote:
> On 10/25/22 03:08, Simon Marchi via Gdb-patches wrote:
>>
>>
>> On 2022-10-24 19:22, Maciej W. Rozycki wrote:
>>> On Mon, 24 Oct 2022, Simon Marchi wrote:
>>>
>>>> I believe that the first two lines are output by Guile itself, in the
>>>> SCM_ASSERT_TYPE macro.  I tried on different systems, different Guile
>>>> versions (2.0, 2.2 and 3.0) and I always get the former output, never
>>>> the output the test expects.  I presume the patch below isn't right, as
>>>> there is surely some systems that do print the latter output, otherwise
>>>> Maciej (the original author) would have noticed it.  I presume we'll
>>>> need to accept both outputs.  But I'd like we if could clarify when we
>>>> get which.
>>>
>>>   FTR I'm still looking into it and like you I have hesitated to just paper
>>> the issue over by allowing both outputs without first understanding what
>>> is really going on here.  I cannot rule out a distribution-specific patch
>>> causing a discrepancy here, but I feel like tracking it down.
>>>
>>>   NB guile 2.0.13 here, reporting as:
>>>
>>> guile (GNU Guile) 2.0.13
>>> Packaged by Debian (2.0.13-deb+1-5.4)
>>
>> According to that version number, it looks like Ubuntu 20.04?
>>
>>    https://packages.ubuntu.com/focal/guile-2.0
>>
>> I tried building on Ubuntu 20.04 against guile-2.0, and I see the same
>> result as you.  And when I try guile2.0 on Arch Linux (this package
>> [1]), I also see the same result as you.  So I must have tested it wrong
>> previously.
>>
>> You can dig further if you want, but I'd be fine just accepting both
>> outputs and saying that guile-2.0 outputs the additional ERROR: while
>> subsequent versions do not.
>>
> 
> FWIW, I did the same here in commit 6bbe1a929c6 ("[gdb/testsuite] Fix gdb.guile/scm-breakpoint.exp with guile 3.0").

Thanks, then I just went ahead and pushed this:


From ee7f721ea2f51cd6cda301ce6a68e84f61c31e0c Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Mon, 24 Oct 2022 12:43:38 -0400
Subject: [PATCH] gdb/testsuite: fix gdb.guile/scm-parameter.exp "wrong type
 argument" test pattern for Guile >= 2.2

Since commit 90319cefe3 ("GDB/Guile: Don't assert that an integer value
is boolean"), I see:

    FAIL: gdb.guile/scm-parameter.exp: kind=PARAM_ZINTEGER: test-PARAM_ZINTEGER-param: guile (set-parameter-value! test-PARAM_ZINTEGER-param #:unlimited)
    FAIL: gdb.guile/scm-parameter.exp: kind=PARAM_ZUINTEGER: test-PARAM_ZUINTEGER-param: guile (set-parameter-value! test-PARAM_ZUINTEGER-param #:unlimited)

This comes from the fact that GDB outputs this:

    ERROR: In procedure set-parameter-value!:
    In procedure gdbscm_set_parameter_value_x: Wrong type argument in position 2 (expecting integer): #:unlimited
    Error while executing Scheme code.

while the test expects an additional "ERROR:" on the second line,
something like this:

    ERROR: In procedure set-parameter-value!:
    ERROR: In procedure gdbscm_set_parameter_value_x: Wrong type argument in position 2 (expecting integer): #:unlimited
    Error while executing Scheme code.

Guile 2.0 outputs the `ERROR:` on the second line, while later versions
do not.  Change the pattern to accept both outputs.  This is similar to
commit 6bbe1a929c6 ("[gdb/testsuite] Fix gdb.guile/scm-breakpoint.exp
with guile 3.0").

Change-Id: I9dc45e7492a4f08340cad974610242ed689de959
---
 gdb/testsuite/gdb.guile/scm-parameter.exp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.guile/scm-parameter.exp b/gdb/testsuite/gdb.guile/scm-parameter.exp
index b9f2d8252117..0b2076c40576 100644
--- a/gdb/testsuite/gdb.guile/scm-parameter.exp
+++ b/gdb/testsuite/gdb.guile/scm-parameter.exp
@@ -183,10 +183,10 @@ foreach_with_prefix kind {
 	"end"

     set param_integer_error \
-	"ERROR: In procedure set-parameter-value!:\r\nERROR: In procedure\
-	 gdbscm_set_parameter_value_x: Wrong type argument in position 2\
-	 \\(expecting integer\\): #:unlimited\r\nError while executing Scheme\
-	 code\\."
+	[multi_line \
+	    "ERROR: In procedure set-parameter-value!:" \
+	    "(ERROR: )?In procedure gdbscm_set_parameter_value_x: Wrong type argument in position 2 \\(expecting integer\\): #:unlimited" \
+	    "Error while executing Scheme code\\."]
     set param_minus_one_error "integer -1 out of range"
     set param_minus_two_range "integer -2 out of range"
     set param_minus_two_unlimited "only -1 is allowed to set as unlimited"
-- 
2.38.1

[OB PATCH] gdb/testsuite: Wrap `param_integer_error' in gdb.guile/scm-parameter.exp

[Maciej W. Rozycki]

Wrap an overlong line in the definition of `param_integer_error' in 
gdb.guile/scm-parameter.exp.  No functional change.
---
On Wed, 26 Oct 2022, Simon Marchi wrote:

> >>>   FTR I'm still looking into it and like you I have hesitated to just paper
> >>> the issue over by allowing both outputs without first understanding what
> >>> is really going on here.  I cannot rule out a distribution-specific patch
> >>> causing a discrepancy here, but I feel like tracking it down.
> >>>
> >>>   NB guile 2.0.13 here, reporting as:
> >>>
> >>> guile (GNU Guile) 2.0.13
> >>> Packaged by Debian (2.0.13-deb+1-5.4)
> >>
> >> According to that version number, it looks like Ubuntu 20.04?
> >>
> >>    https://packages.ubuntu.com/focal/guile-2.0
> >>
> >> I tried building on Ubuntu 20.04 against guile-2.0, and I see the same
> >> result as you.  And when I try guile2.0 on Arch Linux (this package
> >> [1]), I also see the same result as you.  So I must have tested it wrong
> >> previously.
> >>
> >> You can dig further if you want, but I'd be fine just accepting both
> >> outputs and saying that guile-2.0 outputs the additional ERROR: while
> >> subsequent versions do not.
> >>
> > 
> > FWIW, I did the same here in commit 6bbe1a929c6 ("[gdb/testsuite] Fix gdb.guile/scm-breakpoint.exp with guile 3.0").
> 
> Thanks, then I just went ahead and pushed this:

 Thanks, though why such a rush to fix a benign test failure while the 
review took months in the first place?

 FWIW I have been struggling to get multiple versions of Guile compiled 
and installed locally (easier) and then GDB built against each of them 
(tougher).  As it turns out our documentation is misleading.  We have:

`--with-guile[=GUILE]'
     Build GDB with GNU Guile scripting support.  (Done by default if
     libguile is present and found at configure time.)  If your host
     does not have Guile installed, you can find it at
     `https://www.gnu.org/software/guile/'.  The optional argument
     GUILE can be a version number, which will cause `configure' to
     try to use that version of Guile; or the file name of a
     `pkg-config' executable, which will be queried to find the
     information needed to compile and link against Guile.

(and similarly in `./configure --help'), so one could have thought 
`--with-guile=2.0' will work.  Well, not.  You need to specify both the 
name and the version, e.g.: `--with-guile=guile-2.0', which I find kind of 
awkward: why would one want to have a Guile package under a name that is 
not "guile"?  I think documentation is sensible and it's implementation 
that ought to be fixed.

 But that is not enough.  Still I got:

checking whether to use guile... guile-2.0
checking for pkg-config... /usr/bin/pkg-config
checking for usable guile from /usr/bin/pkg-config... checking for scm_set_automatic_finalization_enabled... no
configure: error: in `.../gdb':
configure: error: linking guile version guile-2.0 test program failed
See `config.log' for more details
make[1]: *** [Makefile:12496: configure-gdb] Error 1

This is because I have built local Guile as static libraries only and 
dependencies are not pulled with the (incorrect) `pkg-config' invocation 
we have in our configury.

 I got these issues sorted now and will be posting fixes soon.  With these 
in place I have figured out that the message changed between Guile 2.0 and 
2.2.

 Last but not least here's a fix I committed as obvious to correct an 
overlong line you have introduced with your change.  While we do have an 
exemption for TCL code, there's no need to make use of it here and these 
long lines hit clarity of code badly.  I have verified this test case to 
still pass after my change.

  Maciej
---
 gdb/testsuite/gdb.guile/scm-parameter.exp |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

gdb-test-scm-parameter-error-wrap.diff
Index: src/gdb/testsuite/gdb.guile/scm-parameter.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.guile/scm-parameter.exp
+++ src/gdb/testsuite/gdb.guile/scm-parameter.exp
@@ -185,7 +185,9 @@ foreach_with_prefix kind {
     set param_integer_error \
 	[multi_line \
 	    "ERROR: In procedure set-parameter-value!:" \
-	    "(ERROR: )?In procedure gdbscm_set_parameter_value_x: Wrong type argument in position 2 \\(expecting integer\\): #:unlimited" \
+	    "(ERROR: )?In procedure gdbscm_set_parameter_value_x:\
+	     Wrong type argument in position 2 \\(expecting integer\\):\
+	     #:unlimited" \
 	    "Error while executing Scheme code\\."]
     set param_minus_one_error "integer -1 out of range"
     set param_minus_two_range "integer -2 out of range"

Re: [OB PATCH] gdb/testsuite: Wrap `param_integer_error' in gdb.guile/scm-parameter.exp

[Simon Marchi]

On 10/29/22 09:56, Maciej W. Rozycki wrote:
> Wrap an overlong line in the definition of `param_integer_error' in 
> gdb.guile/scm-parameter.exp.  No functional change.
> ---
> On Wed, 26 Oct 2022, Simon Marchi wrote:
> 
>>>>>   FTR I'm still looking into it and like you I have hesitated to just paper
>>>>> the issue over by allowing both outputs without first understanding what
>>>>> is really going on here.  I cannot rule out a distribution-specific patch
>>>>> causing a discrepancy here, but I feel like tracking it down.
>>>>>
>>>>>   NB guile 2.0.13 here, reporting as:
>>>>>
>>>>> guile (GNU Guile) 2.0.13
>>>>> Packaged by Debian (2.0.13-deb+1-5.4)
>>>>
>>>> According to that version number, it looks like Ubuntu 20.04?
>>>>
>>>>    https://packages.ubuntu.com/focal/guile-2.0
>>>>
>>>> I tried building on Ubuntu 20.04 against guile-2.0, and I see the same
>>>> result as you.  And when I try guile2.0 on Arch Linux (this package
>>>> [1]), I also see the same result as you.  So I must have tested it wrong
>>>> previously.
>>>>
>>>> You can dig further if you want, but I'd be fine just accepting both
>>>> outputs and saying that guile-2.0 outputs the additional ERROR: while
>>>> subsequent versions do not.
>>>>
>>>
>>> FWIW, I did the same here in commit 6bbe1a929c6 ("[gdb/testsuite] Fix gdb.guile/scm-breakpoint.exp with guile 3.0").
>>
>> Thanks, then I just went ahead and pushed this:
> 
>  Thanks, though why such a rush to fix a benign test failure while the 
> review took months in the first place?

The fix seemed relatively obvious, given we already had one instance of
this.

>  FWIW I have been struggling to get multiple versions of Guile compiled 
> and installed locally (easier) and then GDB built against each of them 
> (tougher).  As it turns out our documentation is misleading.  We have:
> 
> `--with-guile[=GUILE]'
>      Build GDB with GNU Guile scripting support.  (Done by default if
>      libguile is present and found at configure time.)  If your host
>      does not have Guile installed, you can find it at
>      `https://www.gnu.org/software/guile/'.  The optional argument
>      GUILE can be a version number, which will cause `configure' to
>      try to use that version of Guile; or the file name of a
>      `pkg-config' executable, which will be queried to find the
>      information needed to compile and link against Guile.
> 
> (and similarly in `./configure --help'), so one could have thought 
> `--with-guile=2.0' will work.  Well, not.  You need to specify both the 
> name and the version, e.g.: `--with-guile=guile-2.0', which I find kind of 
> awkward: why would one want to have a Guile package under a name that is 
> not "guile"?  I think documentation is sensible and it's implementation 
> that ought to be fixed.

Ack.

> 
>  But that is not enough.  Still I got:
> 
> checking whether to use guile... guile-2.0
> checking for pkg-config... /usr/bin/pkg-config
> checking for usable guile from /usr/bin/pkg-config... checking for scm_set_automatic_finalization_enabled... no
> configure: error: in `.../gdb':
> configure: error: linking guile version guile-2.0 test program failed
> See `config.log' for more details
> make[1]: *** [Makefile:12496: configure-gdb] Error 1
> 
> This is because I have built local Guile as static libraries only and 
> dependencies are not pulled with the (incorrect) `pkg-config' invocation 
> we have in our configury.
> 
>  I got these issues sorted now and will be posting fixes soon.  With these 
> in place I have figured out that the message changed between Guile 2.0 and 
> 2.2.

Thanks.

>  Last but not least here's a fix I committed as obvious to correct an 
> overlong line you have introduced with your change.  While we do have an 
> exemption for TCL code, there's no need to make use of it here and these 
> long lines hit clarity of code badly.  I have verified this test case to 
> still pass after my change.

This is subjective.  For expected output patterns, I prefer to format it
as one expected line per source line.  But how you formatted it is fine
with me as well.

Simon