* [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).