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

[Simon Marchi <simark@simark.ca>]

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