* [PATCH] Make gdb.base/corefile.exp work on terminals with few rows
@ 2019-02-06 3:49 Simon Marchi
2019-02-06 18:42 ` Pedro Alves
0 siblings, 1 reply; 5+ messages in thread
From: Simon Marchi @ 2019-02-06 3:49 UTC (permalink / raw)
To: gdb-patches; +Cc: philippe.waroquiers, Simon Marchi
When creating a pty to spawn a subprocess (such as gdb), Expect
copies the settings of its own controlling terminal, including the
number of rows and columns. If you "make check" on a terminal with just
a few rows (e.g. 4), GDB will paginate before reaching the initial
prompt. In default_gdb_start, used by most tests, this is already
handled: if we see the pagination prompt, we sent \n to continue.
Philippe reported that gdb.base/corefile.exp didn't work in terminals
with just a few rows. This test spawns GDB by hand, because it needs to
check things before the initial prompt, which it couldn't do if it used
default_gdb_start.
In this case I think it's not safe to use the same technique as in
default_gdb_start. Even if we could send a \n if we see a pagination
prompt, we match some multiline regexes in there. So if a pagination
slips in there, it might make the regexes not match and fail the test.
It's also not possible to use -ex "set height 0" or -iex "set height 0",
it is handled after the introduction text is shown.
The simplest way I found to avoid showing the pagination completely is
to set stty_init (documented in expect's man page) to initialize gdb's
pty with a fixed number of rows.
gdb/testsuite/ChangeLog:
* gdb.base/corefile.exp: Set stty_init.
---
gdb/testsuite/gdb.base/corefile.exp | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/gdb/testsuite/gdb.base/corefile.exp b/gdb/testsuite/gdb.base/corefile.exp
index 0053f0594586..c494a875bd31 100644
--- a/gdb/testsuite/gdb.base/corefile.exp
+++ b/gdb/testsuite/gdb.base/corefile.exp
@@ -39,6 +39,10 @@ if {$corefile == ""} {
return 0
}
+# Initialize GDB's pty with a fixed size, to make sure we avoid pagination
+# during startup. See "man expect" for details about stty_init.
+set stty_init "rows 25 cols 80"
+
# Test that we can simply startup with a "-core=$corefile" command line arg
# and recognize that the core file is a valid, usable core file.
# To do this, we must shutdown the currently running gdb and restart
--
2.20.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Make gdb.base/corefile.exp work on terminals with few rows
2019-02-06 3:49 [PATCH] Make gdb.base/corefile.exp work on terminals with few rows Simon Marchi
@ 2019-02-06 18:42 ` Pedro Alves
2019-02-06 23:11 ` Simon Marchi
0 siblings, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2019-02-06 18:42 UTC (permalink / raw)
To: Simon Marchi, gdb-patches; +Cc: philippe.waroquiers
On 02/06/2019 03:48 AM, Simon Marchi wrote:
> When creating a pty to spawn a subprocess (such as gdb), Expect
> copies the settings of its own controlling terminal, including the
> number of rows and columns. If you "make check" on a terminal with just
> a few rows (e.g. 4), GDB will paginate before reaching the initial
> prompt. In default_gdb_start, used by most tests, this is already
> handled: if we see the pagination prompt, we sent \n to continue.
>
> Philippe reported that gdb.base/corefile.exp didn't work in terminals
> with just a few rows. This test spawns GDB by hand, because it needs to
> check things before the initial prompt, which it couldn't do if it used
> default_gdb_start.
>
> In this case I think it's not safe to use the same technique as in
> default_gdb_start. Even if we could send a \n if we see a pagination
> prompt, we match some multiline regexes in there. So if a pagination
> slips in there, it might make the regexes not match and fail the test.
>
> It's also not possible to use -ex "set height 0" or -iex "set height 0",
> it is handled after the introduction text is shown.
>
> The simplest way I found to avoid showing the pagination completely is
> to set stty_init (documented in expect's man page) to initialize gdb's
> pty with a fixed number of rows.
Hmm, good idea. But, if you have a small terminal with just a few
columns (as opposed to rows), then the testsuite all breaks, AFAICT.
E.g., I just tried running gdb.base/break.exp with a small window and
the test hangs starting GDB.
But with:
--- c/gdb/testsuite/lib/gdb.exp
+++ w/gdb/testsuite/lib/gdb.exp
@@ -4752,6 +4752,9 @@ proc gdb_init { test_file_name } {
# tests.
setenv TERM "dumb"
+ global stty_init
+ set stty_init "rows 25 cols 80"
+
# Some tests (for example gdb.base/maint.exp) shell out from gdb to use
# grep. Clear GREP_OPTIONS to make the behavior predictable,
# especially having color output turned on can cause tests to fail.
... it passes. And so does gdb.base/corefile.exp.
Is there any reason we'd ever want GDB's terminal size to match
whatever the user's term size was? I'd think that sanitizing / forcing
the same sizes everywhere would just lead to more stable testing,
and thus be a good thing.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Make gdb.base/corefile.exp work on terminals with few rows
2019-02-06 18:42 ` Pedro Alves
@ 2019-02-06 23:11 ` Simon Marchi
2019-02-07 13:26 ` Pedro Alves
0 siblings, 1 reply; 5+ messages in thread
From: Simon Marchi @ 2019-02-06 23:11 UTC (permalink / raw)
To: Pedro Alves, Simon Marchi, gdb-patches; +Cc: philippe.waroquiers
On 2019-02-06 1:42 p.m., Pedro Alves wrote:
> On 02/06/2019 03:48 AM, Simon Marchi wrote:
>> When creating a pty to spawn a subprocess (such as gdb), Expect
>> copies the settings of its own controlling terminal, including the
>> number of rows and columns. If you "make check" on a terminal with just
>> a few rows (e.g. 4), GDB will paginate before reaching the initial
>> prompt. In default_gdb_start, used by most tests, this is already
>> handled: if we see the pagination prompt, we sent \n to continue.
>>
>> Philippe reported that gdb.base/corefile.exp didn't work in terminals
>> with just a few rows. This test spawns GDB by hand, because it needs to
>> check things before the initial prompt, which it couldn't do if it used
>> default_gdb_start.
>>
>> In this case I think it's not safe to use the same technique as in
>> default_gdb_start. Even if we could send a \n if we see a pagination
>> prompt, we match some multiline regexes in there. So if a pagination
>> slips in there, it might make the regexes not match and fail the test.
>>
>> It's also not possible to use -ex "set height 0" or -iex "set height 0",
>> it is handled after the introduction text is shown.
>>
>> The simplest way I found to avoid showing the pagination completely is
>> to set stty_init (documented in expect's man page) to initialize gdb's
>> pty with a fixed number of rows.
>
> Hmm, good idea. But, if you have a small terminal with just a few
> columns (as opposed to rows), then the testsuite all breaks, AFAICT.
>
> E.g., I just tried running gdb.base/break.exp with a small window and
> the test hangs starting GDB.
>
> But with:
>
> --- c/gdb/testsuite/lib/gdb.exp
> +++ w/gdb/testsuite/lib/gdb.exp
> @@ -4752,6 +4752,9 @@ proc gdb_init { test_file_name } {
> # tests.
> setenv TERM "dumb"
>
> + global stty_init
> + set stty_init "rows 25 cols 80"
> +
> # Some tests (for example gdb.base/maint.exp) shell out from gdb to use
> # grep. Clear GREP_OPTIONS to make the behavior predictable,
> # especially having color output turned on can cause tests to fail.
>
> ... it passes. And so does gdb.base/corefile.exp.
>
> Is there any reason we'd ever want GDB's terminal size to match
> whatever the user's term size was? I'd think that sanitizing / forcing
> the same sizes everywhere would just lead to more stable testing,
> and thus be a good thing.
Agreed. If we do this, we can remove the part where we match the pagination
prompt during startup, like so:
From 67bb1dde957fcd8a0168fb04afa3f8e3a4ccf9bb Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Tue, 5 Feb 2019 22:48:57 -0500
Subject: [PATCH] Make gdb.base/corefile.exp work on terminals with few rows
When creating a pty to spawn a subprocess (such as gdb), Expect
copies the settings of its own controlling terminal, including the
number of rows and columns. If you "make check" on a terminal with just
a few rows (e.g. 4), GDB will paginate before reaching the initial
prompt. In default_gdb_start, used by most tests, this is already
handled: if we see the pagination prompt, we sent \n to continue.
Philippe reported that gdb.base/corefile.exp didn't work in terminals
with just a few rows. This test spawns GDB by hand, because it needs to
check things before the initial prompt, which it couldn't do if it used
default_gdb_start.
In this case I think it's not safe to use the same technique as in
default_gdb_start. Even if we could send a \n if we see a pagination
prompt, we match some multiline regexes in there. So if a pagination
slips in there, it might make the regexes not match and fail the test.
It's also not possible to use -ex "set height 0" or -iex "set height 0",
it is handled after the introduction text is shown.
The simplest way I found to avoid showing the pagination completely is
to set stty_init (documented in expect's man page) to initialize gdb's
pty with a fixed number of rows.
And actually, if we set stty_init in gdb_init, it works nicely as a
general solution applicable to all tests. We can therefore remove the
solution introduced in e882ef3cfc3 ("testsuite: expect possible
pagination when starting gdb") where we matched the pagination prompt
during startup.
gdb/testsuite/ChangeLog:
* lib/gdb.exp (default_gdb_start): Don't match pagination
prompt.
(gdb_init): Set stty_init.
---
gdb/testsuite/lib/gdb.exp | 44 ++++++++++++++++++---------------------
1 file changed, 20 insertions(+), 24 deletions(-)
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index bc7ba12d480..d05854329d8 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1638,7 +1638,7 @@ proc default_gdb_spawn { } {
# Default gdb_start procedure.
proc default_gdb_start { } {
- global gdb_prompt pagination_prompt
+ global gdb_prompt
global gdb_spawn_id
global inferior_spawn_id
@@ -1659,29 +1659,20 @@ proc default_gdb_start { } {
# When running over NFS, particularly if running many simultaneous
# tests on different hosts all using the same server, things can
# get really slow. Give gdb at least 3 minutes to start up.
- set loop_again 1
- while { $loop_again } {
- set loop_again 0
- gdb_expect 360 {
- -re "$pagination_prompt" {
- verbose "Hit pagination during startup. Pressing enter to continue."
- send_gdb "\n"
- set loop_again 1
- }
- -re "\[\r\n\]$gdb_prompt $" {
- verbose "GDB initialized."
- }
- -re "$gdb_prompt $" {
- perror "GDB never initialized."
- unset gdb_spawn_id
- return -1
- }
- timeout {
- perror "(timeout) GDB never initialized after 10 seconds."
- remote_close host
- unset gdb_spawn_id
- return -1
- }
+ gdb_expect 360 {
+ -re "\[\r\n\]$gdb_prompt $" {
+ verbose "GDB initialized."
+ }
+ -re "$gdb_prompt $" {
+ perror "GDB never initialized."
+ unset gdb_spawn_id
+ return -1
+ }
+ timeout {
+ perror "(timeout) GDB never initialized after 10 seconds."
+ remote_close host
+ unset gdb_spawn_id
+ return -1
}
}
@@ -4752,6 +4743,11 @@ proc gdb_init { test_file_name } {
# tests.
setenv TERM "dumb"
+ # Initialize GDB's pty with a fixed size, to make sure we avoid pagination
+ # during startup. See "man expect" for details about stty_init.
+ global stty_init
+ set stty_init "rows 25 cols 80"
+
# Some tests (for example gdb.base/maint.exp) shell out from gdb to use
# grep. Clear GREP_OPTIONS to make the behavior predictable,
# especially having color output turned on can cause tests to fail.
--
2.20.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Make gdb.base/corefile.exp work on terminals with few rows
2019-02-06 23:11 ` Simon Marchi
@ 2019-02-07 13:26 ` Pedro Alves
2019-02-07 14:24 ` Simon Marchi
0 siblings, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2019-02-07 13:26 UTC (permalink / raw)
To: Simon Marchi, Simon Marchi, gdb-patches; +Cc: philippe.waroquiers
On 02/06/2019 11:11 PM, Simon Marchi wrote:
> Agreed. If we do this, we can remove the part where we match the pagination
> prompt during startup, like so:
LGTM.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Make gdb.base/corefile.exp work on terminals with few rows
2019-02-07 13:26 ` Pedro Alves
@ 2019-02-07 14:24 ` Simon Marchi
0 siblings, 0 replies; 5+ messages in thread
From: Simon Marchi @ 2019-02-07 14:24 UTC (permalink / raw)
To: Pedro Alves; +Cc: Simon Marchi, gdb-patches, philippe.waroquiers
On 2019-02-07 08:26, Pedro Alves wrote:
> On 02/06/2019 11:11 PM, Simon Marchi wrote:
>
>> Agreed. If we do this, we can remove the part where we match the
>> pagination
>> prompt during startup, like so:
>
> LGTM.
>
> Thanks,
> Pedro Alves
Thanks, pushed.
Simon
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-07 14:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-06 3:49 [PATCH] Make gdb.base/corefile.exp work on terminals with few rows Simon Marchi
2019-02-06 18:42 ` Pedro Alves
2019-02-06 23:11 ` Simon Marchi
2019-02-07 13:26 ` Pedro Alves
2019-02-07 14:24 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).