[PATCH] [gdb] Mention --with/without-system-readline for --configuration

[PATCH] [gdb] Mention --with/without-system-readline for --configuration

[Tom de Vries]

Simon reported that the new test-case gdb.tui/pr30056.exp fails with system
readline.

This is because the test-case requires a fix in readline that's present in our
in-repo copy of readline, but most likely not in any system readline yet.

Fix this by:
- mentioning --with-system-readline or --without-system-readline in the
  configuration string.
- adding a new proc with_system_readline that makes this information available
  in the testsuite.
- using this in test-case gdb.tui/pr30056.exp to declare it unsupported for
  --with-system-readline.

Tested on x86_64-linux.

Reported-By: Simon Marchi <simon.marchi@efficios.com>
---
 gdb/testsuite/gdb.tui/pr30056.exp |  4 ++++
 gdb/testsuite/lib/gdb.exp         |  8 ++++++++
 gdb/top.c                         | 10 ++++++++++
 3 files changed, 22 insertions(+)

diff --git a/gdb/testsuite/gdb.tui/pr30056.exp b/gdb/testsuite/gdb.tui/pr30056.exp
index 7a57a5627a8..4ca7a8b56a8 100644
--- a/gdb/testsuite/gdb.tui/pr30056.exp
+++ b/gdb/testsuite/gdb.tui/pr30056.exp
@@ -15,6 +15,10 @@
 
 # Regression test for PR30056.
 
+# This PR is fixed in the in-repo copy of readline.  System readline may or
+# may not be fixed, so skip this test-case.
+require !with_system_readline
+
 tuiterm_env
 
 save_vars { env(LC_ALL) } {
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 133d914aff8..294d136a547 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2565,6 +2565,14 @@ gdb_caching_proc allow_python_tests {} {
     return [expr {[string first "--with-python" $output] != -1}]
 }
 
+# Return a 1 for configurations that use system readline rather than the
+# in-repo copy.
+
+gdb_caching_proc with_system_readline {} {
+    set output [remote_exec host $::GDB "$::INTERNAL_GDBFLAGS --configuration"]
+    return [expr {[string first "--with-system-readline" $output] != -1}]
+}
+
 gdb_caching_proc allow_dap_tests {} {
     if { ![allow_python_tests] } {
 	return 0
diff --git a/gdb/top.c b/gdb/top.c
index 92de30a1472..90ddc5f5ea7 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1560,6 +1560,16 @@ This GDB was configured as follows:\n\
 "));
 #endif
 
+#ifdef HAVE_READLINE_READLINE_H
+  gdb_printf (stream, _("\
+	     --with-system-readline\n\
+"));
+#else
+  gdb_printf (stream, _("\
+	     --without-system-readline\n\
+"));
+#endif
+
 #ifdef RELOC_SRCDIR
   gdb_printf (stream, _("\
 	     --with-relocated-sources=%s\n\

base-commit: 796029320e75a141570220224731c8151311f8d9
-- 
2.35.3

Re: [PATCH] [gdb] Mention --with/without-system-readline for --configuration

[Simon Marchi]

On 5/30/23 05:33, Tom de Vries wrote:
> Simon reported that the new test-case gdb.tui/pr30056.exp fails with system
> readline.
> 
> This is because the test-case requires a fix in readline that's present in our
> in-repo copy of readline, but most likely not in any system readline yet.
> 
> Fix this by:
> - mentioning --with-system-readline or --without-system-readline in the
>   configuration string.
> - adding a new proc with_system_readline that makes this information available
>   in the testsuite.
> - using this in test-case gdb.tui/pr30056.exp to declare it unsupported for
>   --with-system-readline.
> 
> Tested on x86_64-linux.
> 
> Reported-By: Simon Marchi <simon.marchi@efficios.com>

I confirm this makes the test skipped on my machine.

Once the fix is merged and in a readline release, could we make the skip
conditional on the readline version?  We could have a maintenance
command or something like that that outputs RL_READLINE_VERSION, and
skip only for old readline versions.

Simon

Re: [PATCH] [gdb] Mention --with/without-system-readline for --configuration

[Tom de Vries]

On 5/30/23 15:36, Simon Marchi wrote:
> On 5/30/23 05:33, Tom de Vries wrote:
>> Simon reported that the new test-case gdb.tui/pr30056.exp fails with system
>> readline.
>>
>> This is because the test-case requires a fix in readline that's present in our
>> in-repo copy of readline, but most likely not in any system readline yet.
>>
>> Fix this by:
>> - mentioning --with-system-readline or --without-system-readline in the
>>    configuration string.
>> - adding a new proc with_system_readline that makes this information available
>>    in the testsuite.
>> - using this in test-case gdb.tui/pr30056.exp to declare it unsupported for
>>    --with-system-readline.
>>
>> Tested on x86_64-linux.
>>
>> Reported-By: Simon Marchi <simon.marchi@efficios.com>
> 
> I confirm this makes the test skipped on my machine.
> 
> Once the fix is merged and in a readline release, could we make the skip
> conditional on the readline version?  We could have a maintenance
> command or something like that that outputs RL_READLINE_VERSION, and
> skip only for old readline versions.

We could do that, that sounds useful.

I also considered printing the readline version string after 
--with/without-system-readline, but it looked a bit too different to all 
the other lines.

Do you want have this implemented before, or are ok with the fix as is?

Thanks,
- Tom

Re: [PATCH] [gdb] Mention --with/without-system-readline for --configuration

[Simon Marchi]

On 5/30/23 11:41, Tom de Vries wrote:
> On 5/30/23 15:36, Simon Marchi wrote:
>> On 5/30/23 05:33, Tom de Vries wrote:
>>> Simon reported that the new test-case gdb.tui/pr30056.exp fails with system
>>> readline.
>>>
>>> This is because the test-case requires a fix in readline that's present in our
>>> in-repo copy of readline, but most likely not in any system readline yet.
>>>
>>> Fix this by:
>>> - mentioning --with-system-readline or --without-system-readline in the
>>>    configuration string.
>>> - adding a new proc with_system_readline that makes this information available
>>>    in the testsuite.
>>> - using this in test-case gdb.tui/pr30056.exp to declare it unsupported for
>>>    --with-system-readline.
>>>
>>> Tested on x86_64-linux.
>>>
>>> Reported-By: Simon Marchi <simon.marchi@efficios.com>
>>
>> I confirm this makes the test skipped on my machine.
>>
>> Once the fix is merged and in a readline release, could we make the skip
>> conditional on the readline version?  We could have a maintenance
>> command or something like that that outputs RL_READLINE_VERSION, and
>> skip only for old readline versions.
> 
> We could do that, that sounds useful.
> 
> I also considered printing the readline version string after --with/without-system-readline, but it looked a bit too different to all the other lines.
> 
> Do you want have this implemented before, or are ok with the fix as is?

I'm fine with the fix as is:

Approved-By: Simon Marchi <simon.marchi@efficios.com>

We just need to remember to do this when the next readline comes out :)

Simon

Re: [PATCH] [gdb] Mention --with/without-system-readline for --configuration

[Tom de Vries]

On 5/30/23 17:43, Simon Marchi wrote:
> On 5/30/23 11:41, Tom de Vries wrote:
>> On 5/30/23 15:36, Simon Marchi wrote:
>>> On 5/30/23 05:33, Tom de Vries wrote:
>>>> Simon reported that the new test-case gdb.tui/pr30056.exp fails with system
>>>> readline.
>>>>
>>>> This is because the test-case requires a fix in readline that's present in our
>>>> in-repo copy of readline, but most likely not in any system readline yet.
>>>>
>>>> Fix this by:
>>>> - mentioning --with-system-readline or --without-system-readline in the
>>>>     configuration string.
>>>> - adding a new proc with_system_readline that makes this information available
>>>>     in the testsuite.
>>>> - using this in test-case gdb.tui/pr30056.exp to declare it unsupported for
>>>>     --with-system-readline.
>>>>
>>>> Tested on x86_64-linux.
>>>>
>>>> Reported-By: Simon Marchi <simon.marchi@efficios.com>
>>>
>>> I confirm this makes the test skipped on my machine.
>>>
>>> Once the fix is merged and in a readline release, could we make the skip
>>> conditional on the readline version?  We could have a maintenance
>>> command or something like that that outputs RL_READLINE_VERSION, and
>>> skip only for old readline versions.
>>
>> We could do that, that sounds useful.
>>
>> I also considered printing the readline version string after --with/without-system-readline, but it looked a bit too different to all the other lines.
>>
>> Do you want have this implemented before, or are ok with the fix as is?
> 
> I'm fine with the fix as is:
> 
> Approved-By: Simon Marchi <simon.marchi@efficios.com>
> 
> We just need to remember to do this when the next readline comes out :)

Indeed, filed a PR ( 
https://sourceware.org/bugzilla/show_bug.cgi?id=30500 ) about it.

Thanks,
- Tom