[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gdb-patches
Subject:    Re: [OB PATCH] gdb/testsuite: Wrap `param_integer_error' in gdb.guile/scm-parameter.exp
From:       Simon Marchi via Gdb-patches <gdb-patches () sourceware ! org>
Date:       2022-10-31 12:57:00
Message-ID: 34e35222-43f8-058c-ead0-6baa4823cee4 () simark ! ca
[Download RAW message or body]



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


[prev in list] [next in list] [prev in thread] [next in thread]