[PATCH] gdb/testsuite/README: update default value of INTERNAL_GDBFLAGS

[PATCH] gdb/testsuite/README: update default value of INTERNAL_GDBFLAGS

[Philippe Blain]

The INTERNAL_GDBFLAGS runtest variable was updated in 55c3ad88013
([gdb/testsuite] Prevent pagination in GDB_INTERNALFLAGS, 2020-10-26) to
disable pagination, and in aae1c79a03a (PR python/12227..., 2010-12-07)
to point to the data directory, but its default value mentioned in the
testsuite's README was not kept up to date.

Update the README accordingly, and mention where the default value is
set, so that it's easier for futur new contributors to check if the
default mentioned in the README is still current.
---

Notes:
    I noticed that while testing the fix for PR 24069, since I wanted
    to use my '~/.gdbinit' with 'set startup-with-shell off' for all tests,
    and found out that setting INTERNAL_GDBFLAGS to only '-nw' made some things
    a little worse (as GDB was then missing '-data-directory').
    
    Meta note: when I sent this patch earlier (and messed-up my CCs) in [1], I was
    confused by the fact that my CCs for Simon, Doug Evans and Tom de Vries were
    not showing up on public-inbox, even in the "raw" view [2]. After a little
    web search I think this is due to them setting the mailman "avoid duplicates"
    setting [3].
    
    [1] https://pi.simark.ca/gdb-patches/20220219212442.18136-1-levraiphilippeblain@gmail.com/T/#u
    [2] https://pi.simark.ca/gdb-patches/20220219212442.18136-1-levraiphilippeblain@gmail.com/raw
    [3] https://wiki.list.org/DOC/Mailman%202.1%20Members%20Manual#A7.2_How_can_I_avoid_getting_duplicate_messages.3F_.28duplicates_option.29

 gdb/testsuite/README | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/README b/gdb/testsuite/README
index 7552774c78b..2d3f1c45966 100644
--- a/gdb/testsuite/README
+++ b/gdb/testsuite/README
@@ -168,11 +168,14 @@ INTERNAL_GDBFLAGS
 
 Command line options passed to all GDB invocations.
 
-The default is "-nw -nx".
+The default is set in lib/gdb.exp and is currently
+"-nw -nx -data-directory /path/to/build/directory/gdb/data-directory -iex 'set height 0' -iex 'set width 0'".
 
 `-nw' disables any of the windowed interfaces.
 `-nx' disables ~/.gdbinit, so that it doesn't interfere with
 the tests.
+`-data-directory` points the the data directory in the build directory
+`-iex 'set {height,width} 0'` disables pagination
 
 This is actually considered an internal variable, and you
 won't normally want to change it.  However, in some situations,
@@ -193,7 +196,10 @@ a .gdbinit.  For example:
 	HOME=`pwd` runtest \
 	  GDB=/usr/bin/gdb \
 	  GDBSERVER=/usr/bin/gdbserver \
-	  INTERNAL_GDBFLAGS=-nw
+	  INTERNAL_GDBFLAGS="-nw -iex 'set height 0' -iex 'set width 0'"
+
+Note that we do not need to specify '-data-directory' here
+as we are testing an installed GDB.
 
 GDB_PARALLEL
 

base-commit: 0acf434a23768449cbb4b3732355f3f2febecaee
-- 
2.29.2

Re: [PATCH] gdb/testsuite/README: update default value of INTERNAL_GDBFLAGS

[Simon Marchi]

On 2022-02-19 18:40, Philippe Blain via Gdb-patches wrote:
> The INTERNAL_GDBFLAGS runtest variable was updated in 55c3ad88013
> ([gdb/testsuite] Prevent pagination in GDB_INTERNALFLAGS, 2020-10-26) to
> disable pagination, and in aae1c79a03a (PR python/12227..., 2010-12-07)
> to point to the data directory, but its default value mentioned in the
> testsuite's README was not kept up to date.
> 
> Update the README accordingly, and mention where the default value is
> set, so that it's easier for futur new contributors to check if the
> default mentioned in the README is still current.
> ---
> 
> Notes:
>     I noticed that while testing the fix for PR 24069, since I wanted
>     to use my '~/.gdbinit' with 'set startup-with-shell off' for all tests,
>     and found out that setting INTERNAL_GDBFLAGS to only '-nw' made some things
>     a little worse (as GDB was then missing '-data-directory').
>     
>     Meta note: when I sent this patch earlier (and messed-up my CCs) in [1], I was
>     confused by the fact that my CCs for Simon, Doug Evans and Tom de Vries were
>     not showing up on public-inbox, even in the "raw" view [2]. After a little
>     web search I think this is due to them setting the mailman "avoid duplicates"
>     setting [3].
>     
>     [1] https://pi.simark.ca/gdb-patches/20220219212442.18136-1-levraiphilippeblain@gmail.com/T/#u
>     [2] https://pi.simark.ca/gdb-patches/20220219212442.18136-1-levraiphilippeblain@gmail.com/raw
>     [3] https://wiki.list.org/DOC/Mailman%202.1%20Members%20Manual#A7.2_How_can_I_avoid_getting_duplicate_messages.3F_.28duplicates_option.29
> 
>  gdb/testsuite/README | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/testsuite/README b/gdb/testsuite/README
> index 7552774c78b..2d3f1c45966 100644
> --- a/gdb/testsuite/README
> +++ b/gdb/testsuite/README
> @@ -168,11 +168,14 @@ INTERNAL_GDBFLAGS
>  
>  Command line options passed to all GDB invocations.
>  
> -The default is "-nw -nx".
> +The default is set in lib/gdb.exp and is currently
> +"-nw -nx -data-directory /path/to/build/directory/gdb/data-directory -iex 'set height 0' -iex 'set width 0'".
>  
>  `-nw' disables any of the windowed interfaces.
>  `-nx' disables ~/.gdbinit, so that it doesn't interfere with
>  the tests.
> +`-data-directory` points the the data directory in the build directory
> +`-iex 'set {height,width} 0'` disables pagination

I think this should just say "see the default in lib/gdb.exp", and the
flags can be explained there.  That would avoid things getting out of
sync.

Still, your patch makes things better than they are now, so I think we
can push it in any case.

Simon

Re: [PATCH] gdb/testsuite/README: update default value of INTERNAL_GDBFLAGS

[Philippe Blain]

Hi Simon,

> Le 19 févr. 2022 à 20:41, Simon Marchi <simon.marchi@polymtl.ca> a écrit :
> 
>> On 2022-02-19 18:40, Philippe Blain via Gdb-patches wrote:
>> The INTERNAL_GDBFLAGS runtest variable was updated in 55c3ad88013
>> ([gdb/testsuite] Prevent pagination in GDB_INTERNALFLAGS, 2020-10-26) to
>> disable pagination, and in aae1c79a03a (PR python/12227..., 2010-12-07)
>> to point to the data directory, but its default value mentioned in the
>> testsuite's README was not kept up to date.
>> 
>> Update the README accordingly, and mention where the default value is
>> set, so that it's easier for futur new contributors to check if the
>> default mentioned in the README is still current.
>> ---
>> 
>> Notes:
>>    I noticed that while testing the fix for PR 24069, since I wanted
>>    to use my '~/.gdbinit' with 'set startup-with-shell off' for all tests,
>>    and found out that setting INTERNAL_GDBFLAGS to only '-nw' made some things
>>    a little worse (as GDB was then missing '-data-directory').
>> 
>>    Meta note: when I sent this patch earlier (and messed-up my CCs) in [1], I was
>>    confused by the fact that my CCs for Simon, Doug Evans and Tom de Vries were
>>    not showing up on public-inbox, even in the "raw" view [2]. After a little
>>    web search I think this is due to them setting the mailman "avoid duplicates"
>>    setting [3].
>> 
>>    [1] https://pi.simark.ca/gdb-patches/20220219212442.18136-1-levraiphilippeblain@gmail.com/T/#u
>>    [2] https://pi.simark.ca/gdb-patches/20220219212442.18136-1-levraiphilippeblain@gmail.com/raw
>>    [3] https://wiki.list.org/DOC/Mailman%202.1%20Members%20Manual#A7.2_How_can_I_avoid_getting_duplicate_messages.3F_.28duplicates_option.29
>> 
>> gdb/testsuite/README | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>> 
>> diff --git a/gdb/testsuite/README b/gdb/testsuite/README
>> index 7552774c78b..2d3f1c45966 100644
>> --- a/gdb/testsuite/README
>> +++ b/gdb/testsuite/README
>> @@ -168,11 +168,14 @@ INTERNAL_GDBFLAGS
>> 
>> Command line options passed to all GDB invocations.
>> 
>> -The default is "-nw -nx".
>> +The default is set in lib/gdb.exp and is currently
>> +"-nw -nx -data-directory /path/to/build/directory/gdb/data-directory -iex 'set height 0' -iex 'set width 0'".
>> 
>> `-nw' disables any of the windowed interfaces.
>> `-nx' disables ~/.gdbinit, so that it doesn't interfere with
>> the tests.
>> +`-data-directory` points the the data directory in the build directory
>> +`-iex 'set {height,width} 0'` disables pagination
> 
> I think this should just say "see the default in lib/gdb.exp", and the
> flags can be explained there.  That would avoid things getting out of
> sync.

Yes, you are probably right. I can send an updated patch with that change. 

Philippe. 

[PATCH v2] gdb/testsuite/README: point to default value of INTERNAL_GDBFLAGS

[Philippe Blain]

The INTERNAL_GDBFLAGS runtest variable was updated in 55c3ad88013
([gdb/testsuite] Prevent pagination in GDB_INTERNALFLAGS, 2020-10-26) to
disable pagination, and in aae1c79a03a (PR python/12227..., 2010-12-07)
to point to the data directory, but its default value mentioned in the
testsuite's README was not kept up to date.

To avoid it getting out of sync even more, point the reader to the
definition of the variable in lib/gdb.exp, and move the explanation of
the different flags there. Also adjust the example in the README
so it follows the flags added in 55c3ad88013.
---

Changes since v1:
Following Simon's comment, move the explanation of the flags to gdb.exp
and simply point the reader to there.

 gdb/testsuite/README      | 12 +++++-------
 gdb/testsuite/lib/gdb.exp |  4 ++++
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/README b/gdb/testsuite/README
index 7552774c78b..c2f659a7188 100644
--- a/gdb/testsuite/README
+++ b/gdb/testsuite/README
@@ -167,12 +167,7 @@ checks both the installed GDB and GDBserver.
 INTERNAL_GDBFLAGS
 
 Command line options passed to all GDB invocations.
-
-The default is "-nw -nx".
-
-`-nw' disables any of the windowed interfaces.
-`-nx' disables ~/.gdbinit, so that it doesn't interfere with
-the tests.
+The default is set in lib/gdb.exp.
 
 This is actually considered an internal variable, and you
 won't normally want to change it.  However, in some situations,
@@ -193,7 +188,10 @@ a .gdbinit.  For example:
 	HOME=`pwd` runtest \
 	  GDB=/usr/bin/gdb \
 	  GDBSERVER=/usr/bin/gdbserver \
-	  INTERNAL_GDBFLAGS=-nw
+	  INTERNAL_GDBFLAGS="-nw -iex 'set height 0' -iex 'set width 0'"
+
+Note that we do not need to specify '-data-directory' here
+as we are testing an installed GDB.
 
 GDB_PARALLEL
 
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index a3717a40229..0cec46731bb 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -129,6 +129,10 @@ verbose "using GDBFLAGS = $GDBFLAGS" 2
 set BUILD_DATA_DIRECTORY "[pwd]/../data-directory"
 
 # INTERNAL_GDBFLAGS contains flags that the testsuite requires.
+# `-nw' disables any of the windowed interfaces.
+# `-nx' disables ~/.gdbinit, so that it doesn't interfere with the tests.
+# `-data-directory' points to the data directory in the build directory.
+# `-iex "set {height,width} 0"' disables pagination.
 global INTERNAL_GDBFLAGS
 if ![info exists INTERNAL_GDBFLAGS] {
     set INTERNAL_GDBFLAGS \

base-commit: 0acf434a23768449cbb4b3732355f3f2febecaee
-- 
2.29.2

Re: [PATCH v2] gdb/testsuite/README: point to default value of INTERNAL_GDBFLAGS

[Simon Marchi]

On 2022-02-20 11:49, Philippe Blain via Gdb-patches wrote:
> The INTERNAL_GDBFLAGS runtest variable was updated in 55c3ad88013
> ([gdb/testsuite] Prevent pagination in GDB_INTERNALFLAGS, 2020-10-26) to
> disable pagination, and in aae1c79a03a (PR python/12227..., 2010-12-07)
> to point to the data directory, but its default value mentioned in the
> testsuite's README was not kept up to date.
> 
> To avoid it getting out of sync even more, point the reader to the
> definition of the variable in lib/gdb.exp, and move the explanation of
> the different flags there. Also adjust the example in the README
> so it follows the flags added in 55c3ad88013.

Thanks, this is ok.

If you plan on contributing more patches, it would be useful for you to
get push access so you can push patches once they are approved.  Would
you prefer that?

Simon

Re: [PATCH v2] gdb/testsuite/README: point to default value of INTERNAL_GDBFLAGS

[Philippe Blain]

Hi Simon,

Le 2022-02-22 à 08:46, Simon Marchi a écrit :
> On 2022-02-20 11:49, Philippe Blain via Gdb-patches wrote:
>> The INTERNAL_GDBFLAGS runtest variable was updated in 55c3ad88013
>> ([gdb/testsuite] Prevent pagination in GDB_INTERNALFLAGS, 2020-10-26) to
>> disable pagination, and in aae1c79a03a (PR python/12227..., 2010-12-07)
>> to point to the data directory, but its default value mentioned in the
>> testsuite's README was not kept up to date.
>>
>> To avoid it getting out of sync even more, point the reader to the
>> definition of the variable in lib/gdb.exp, and move the explanation of
>> the different flags there. Also adjust the example in the README
>> so it follows the flags added in 55c3ad88013.
> 
> Thanks, this is ok.
> 
> If you plan on contributing more patches, it would be useful for you to
> get push access so you can push patches once they are approved.  Would
> you prefer that?

I think I would be more comfortable if someone else pushes, at least for
now, if you don't mind. I'm not completely clear on all the mechanics of the
contribution flow for this project. If it becomes too much burden than I could 
get push access. How does that sound ?

By the way, I did not yet sign the FSF copyright assigment. I don't know if
this kind of patch would be OK without it, but anyway I'm happy to sign if needed.

Cheers,

Philippe.

Re: [PATCH v2] gdb/testsuite/README: point to default value of INTERNAL_GDBFLAGS

[Simon Marchi]

On 2022-02-22 09:01, Philippe Blain wrote:
> Hi Simon,
> 
> Le 2022-02-22 à 08:46, Simon Marchi a écrit :
>> On 2022-02-20 11:49, Philippe Blain via Gdb-patches wrote:
>>> The INTERNAL_GDBFLAGS runtest variable was updated in 55c3ad88013
>>> ([gdb/testsuite] Prevent pagination in GDB_INTERNALFLAGS, 2020-10-26) to
>>> disable pagination, and in aae1c79a03a (PR python/12227..., 2010-12-07)
>>> to point to the data directory, but its default value mentioned in the
>>> testsuite's README was not kept up to date.
>>>
>>> To avoid it getting out of sync even more, point the reader to the
>>> definition of the variable in lib/gdb.exp, and move the explanation of
>>> the different flags there. Also adjust the example in the README
>>> so it follows the flags added in 55c3ad88013.
>>
>> Thanks, this is ok.
>>
>> If you plan on contributing more patches, it would be useful for you to
>> get push access so you can push patches once they are approved.  Would
>> you prefer that?
> 
> I think I would be more comfortable if someone else pushes, at least for
> now, if you don't mind. I'm not completely clear on all the mechanics of the
> contribution flow for this project. If it becomes too much burden than I could 
> get push access. How does that sound ?

No problem, you just need to make it clear when someone approves your
patch that you need them to push the patch, otherwise it will fall
through the cracks.

I will push this one now.

> By the way, I did not yet sign the FSF copyright assigment. I don't know if
> this kind of patch would be OK without it, but anyway I'm happy to sign if needed.

Good point.  This patch is small enough, especially since you mostly
move existing text.  But if you plan on sending slightly more
significative patches, it would be good to have the copyright assignment
on file.  And it can take a bit of time, so better get the process
started early.

Simon

Re: [PATCH v2] gdb/testsuite/README: point to default value of INTERNAL_GDBFLAGS

[Philippe Blain]

Le 2022-02-22 à 09:31, Simon Marchi a écrit :
> On 2022-02-22 09:01, Philippe Blain wrote:
>> Hi Simon,
>>
>> Le 2022-02-22 à 08:46, Simon Marchi a écrit :
>>> On 2022-02-20 11:49, Philippe Blain via Gdb-patches wrote:
> 
> No problem, you just need to make it clear when someone approves your
> patch that you need them to push the patch, otherwise it will fall
> through the cracks.
> 

Understood, I'll make sure to mention it next time.

> I will push this one now.
> 
>> By the way, I did not yet sign the FSF copyright assigment. I don't know if
>> this kind of patch would be OK without it, but anyway I'm happy to sign if needed.
> 
> Good point.  This patch is small enough, especially since you mostly
> move existing text.  But if you plan on sending slightly more
> significative patches, it would be good to have the copyright assignment
> on file.  And it can take a bit of time, so better get the process
> started early.

OK, let's get the process started then. How should we proceed ? Last time
I checked I think a handwritten signature was still required, at least for
non-US citizens ?

Philippe.

Re: [PATCH v2] gdb/testsuite/README: point to default value of INTERNAL_GDBFLAGS

[Simon Marchi]

> OK, let's get the process started then. How should we proceed ? Last time
> I checked I think a handwritten signature was still required, at least for
> non-US citizens ?

I don't think that's needed nowadays, luckily.

Follow the instructions here:

https://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/request-assign.future

Simon