Mention --with/without-system-readline for --configuration
Commit Message
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(+)
base-commit: 796029320e75a141570220224731c8151311f8d9
Comments
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
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
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
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
@@ -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) } {
@@ -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
@@ -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\