Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] Clear GREP_OPTIONS environment variable.
@ 2011-03-16 10:13 Andrew Burgess
  2011-03-16 15:43 ` Tom Tromey
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Burgess @ 2011-03-16 10:13 UTC (permalink / raw)
  To: gdb-patches

The test gdb.base/maint.exp shells out from gdb to use grep. I have GREP_OPTIONS="--color=auto" in my environment and this causes the test to fail. The patch below clears this environment variable before running gdb. Ok to apply?

Cheers,
Andrew


gdb/testsuite/ChangeLog

2011-03-15  Andrew Burgess  <aburgess@broadcom.com>

	* lib/gdb.exp (default_gdb_start): Clear the GREP_OPTIONS
	environment variable to make grep output more predictable.

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 1b601af..547a232 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1331,6 +1331,11 @@ proc default_gdb_start { } {
     # The same bug doesn't show up if we use ^P / ^N instead.
     set env(TERM) "vt100"
 
+    # Some tests (for example gdb.base/maint.exp) shell out from gdb to use
+    # grep.  Clear GREP_OPTIONS to make the behavoiur predictable, 
+    # especially having color output turned on can cause tests to fail.
+    set env(GREP_OPTIONS) ""
+
     verbose "Spawning $GDB $INTERNAL_GDBFLAGS $GDBFLAGS"
 
     if [info exists gdb_spawn_id] {


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Clear GREP_OPTIONS environment variable.
  2011-03-16 10:13 [PATCH] Clear GREP_OPTIONS environment variable Andrew Burgess
@ 2011-03-16 15:43 ` Tom Tromey
  2011-03-16 16:57   ` Joel Brobecker
  0 siblings, 1 reply; 5+ messages in thread
From: Tom Tromey @ 2011-03-16 15:43 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

>>>>> "Andrew" == Andrew Burgess <aburgess@broadcom.com> writes:

Andrew> The test gdb.base/maint.exp shells out from gdb to use grep. I
Andrew> have GREP_OPTIONS="--color=auto" in my environment and this
Andrew> causes the test to fail. The patch below clears this environment
Andrew> variable before running gdb. Ok to apply?

Andrew> 2011-03-15  Andrew Burgess  <aburgess@broadcom.com>
Andrew> 	* lib/gdb.exp (default_gdb_start): Clear the GREP_OPTIONS
Andrew> 	environment variable to make grep output more predictable.

Ok.

Tom


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Clear GREP_OPTIONS environment variable.
  2011-03-16 15:43 ` Tom Tromey
@ 2011-03-16 16:57   ` Joel Brobecker
  2011-03-17 11:25     ` Andrew Burgess
  0 siblings, 1 reply; 5+ messages in thread
From: Joel Brobecker @ 2011-03-16 16:57 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Andrew Burgess, gdb-patches

> Andrew> The test gdb.base/maint.exp shells out from gdb to use grep. I
> Andrew> have GREP_OPTIONS="--color=auto" in my environment and this
> Andrew> causes the test to fail. The patch below clears this environment
> Andrew> variable before running gdb. Ok to apply?
> 
> Andrew> 2011-03-15  Andrew Burgess  <aburgess@broadcom.com>
> Andrew> 	* lib/gdb.exp (default_gdb_start): Clear the GREP_OPTIONS
> Andrew> 	environment variable to make grep output more predictable.
> 
> Ok.

I think that this is OK too.  However, I'm wondering whether, as
a followup, we should be moving this type of code to the gdb_init
routine, rather than putting it there.  The idea is to run this
once per testcase, rather than once everytime we start a new GDB
process.

-- 
Joel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Clear GREP_OPTIONS environment variable.
  2011-03-16 16:57   ` Joel Brobecker
@ 2011-03-17 11:25     ` Andrew Burgess
  2011-03-25 15:46       ` Andrew Burgess
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Burgess @ 2011-03-17 11:25 UTC (permalink / raw)
  To: gdb-patches

On 16/03/2011 16:14, Joel Brobecker wrote:
>> Andrew>  The test gdb.base/maint.exp shells out from gdb to use grep. I
>> Andrew>  have GREP_OPTIONS="--color=auto" in my environment and this
>> Andrew>  causes the test to fail. The patch below clears this environment
>> Andrew>  variable before running gdb. Ok to apply?
>>
>> Andrew>  2011-03-15  Andrew Burgess<aburgess@broadcom.com>
>> Andrew>  	* lib/gdb.exp (default_gdb_start): Clear the GREP_OPTIONS
>> Andrew>  	environment variable to make grep output more predictable.
>>
>> Ok.
> 
> I think that this is OK too.  However, I'm wondering whether, as
> a followup, we should be moving this type of code to the gdb_init
> routine, rather than putting it there.  The idea is to run this
> once per testcase, rather than once everytime we start a new GDB
> process.
> 

So the patch below moves the environment setup into gdb_init. I don't see any regressions here. If this is preferred then I can commit this instead of the original patch.


Cheers,
Andrew


gdb/testsuite/ChangeLog

2011-03-15  Andrew Burgess  <aburgess@broadcom.com>

	* lib/gdb.exp (default_gdb_start,gdb_init): Clear the GREP_OPTIONS
	environment variable to make grep output more predictable. Move
	all the environment setup into gdb_init so it's done once per test
	case rather than each time we start gdb.

 	* gdb.arch/altivec-abi.exp: Skip "generic" tests on 64-bit when
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 1b601af..7be05d1 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1316,21 +1316,6 @@ proc default_gdb_start { } {
 
     gdb_stop_suppressing_tests;
 
-    set env(LC_CTYPE) C
-
-    # Don't let a .inputrc file or an existing setting of INPUTRC mess up
-    # the test results.  Even if /dev/null doesn't exist on the particular
-    # platform, the readline library will use the default setting just by
-    # failing to open the file.  OTOH, opening /dev/null successfully will
-    # also result in the default settings being used since nothing will be
-    # read from this file.
-    set env(INPUTRC) "/dev/null"
-
-    # The gdb.base/readline.exp arrow key test relies on the standard VT100
-    # bindings, so make sure that an appropriate terminal is selected.
-    # The same bug doesn't show up if we use ^P / ^N instead.
-    set env(TERM) "vt100"
-
     verbose "Spawning $GDB $INTERNAL_GDBFLAGS $GDBFLAGS"
 
     if [info exists gdb_spawn_id] {
@@ -2797,11 +2782,30 @@ proc gdb_init { args } {
 	set banned_variables_traced 1
     }
 
-    # We set LC_ALL and LANG to C so that we get the same messages as
-    # expected.
+    # We set LC_ALL, LC_CTYPE, and LANG to C so that we get the same
+    # messages as expected.
     setenv LC_ALL C
+    setenv LC_CTYPE C
     setenv LANG C
 
+    # Don't let a .inputrc file or an existing setting of INPUTRC mess up
+    # the test results.  Even if /dev/null doesn't exist on the particular
+    # platform, the readline library will use the default setting just by
+    # failing to open the file.  OTOH, opening /dev/null successfully will
+    # also result in the default settings being used since nothing will be
+    # read from this file.
+    setenv INPUTRC "/dev/null"
+
+    # The gdb.base/readline.exp arrow key test relies on the standard VT100
+    # bindings, so make sure that an appropriate terminal is selected.
+    # The same bug doesn't show up if we use ^P / ^N instead.
+    setenv TERM "vt100"
+
+    # Some tests (for example gdb.base/maint.exp) shell out from gdb to use
+    # grep.  Clear GREP_OPTIONS to make the behavoiur predictable, 
+    # especially having color output turned on can cause tests to fail.
+    setenv GREP_OPTIONS ""
+
     return [eval default_gdb_init $args];
 }
 




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Clear GREP_OPTIONS environment variable.
  2011-03-17 11:25     ` Andrew Burgess
@ 2011-03-25 15:46       ` Andrew Burgess
  0 siblings, 0 replies; 5+ messages in thread
From: Andrew Burgess @ 2011-03-25 15:46 UTC (permalink / raw)
  To: gdb-patches

On 17/03/2011 09:11, Andrew Burgess wrote:
> 
> gdb/testsuite/ChangeLog
> 
> 2011-03-15  Andrew Burgess<aburgess@broadcom.com>
> 
> 	* lib/gdb.exp (default_gdb_start,gdb_init): Clear the GREP_OPTIONS
> 	environment variable to make grep output more predictable. Move
> 	all the environment setup into gdb_init so it's done once per test
> 	case rather than each time we start gdb.

I committed the second version of this patch with a tiny modification, removing the "global env" line from default_gdb_start as it's no longer needed. No test regressions here.

Cheers,
Andrew


Patch as committed:


Index: ./gdb/testsuite/lib/gdb.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
retrieving revision 1.168
retrieving revision 1.169
diff -u -p -r1.168 -r1.169
--- ./gdb/testsuite/lib/gdb.exp	15 Mar 2011 16:41:42 -0000	1.168
+++ ./gdb/testsuite/lib/gdb.exp	25 Mar 2011 13:08:11 -0000	1.169
@@ -1311,25 +1311,9 @@ proc default_gdb_start { } {
     global gdb_prompt
     global timeout
     global gdb_spawn_id;
-    global env
 
     gdb_stop_suppressing_tests;
 
-    set env(LC_CTYPE) C
-
-    # Don't let a .inputrc file or an existing setting of INPUTRC mess up
-    # the test results.  Even if /dev/null doesn't exist on the particular
-    # platform, the readline library will use the default setting just by
-    # failing to open the file.  OTOH, opening /dev/null successfully will
-    # also result in the default settings being used since nothing will be
-    # read from this file.
-    set env(INPUTRC) "/dev/null"
-
-    # The gdb.base/readline.exp arrow key test relies on the standard VT100
-    # bindings, so make sure that an appropriate terminal is selected.
-    # The same bug doesn't show up if we use ^P / ^N instead.
-    set env(TERM) "vt100"
-
     verbose "Spawning $GDB $INTERNAL_GDBFLAGS $GDBFLAGS"
 
     if [info exists gdb_spawn_id] {
@@ -2796,11 +2780,30 @@ proc gdb_init { args } {
 	set banned_variables_traced 1
     }
 
-    # We set LC_ALL and LANG to C so that we get the same messages as
-    # expected.
+    # We set LC_ALL, LC_CTYPE, and LANG to C so that we get the same
+    # messages as expected.
     setenv LC_ALL C
+    setenv LC_CTYPE C
     setenv LANG C
 
+    # Don't let a .inputrc file or an existing setting of INPUTRC mess up
+    # the test results.  Even if /dev/null doesn't exist on the particular
+    # platform, the readline library will use the default setting just by
+    # failing to open the file.  OTOH, opening /dev/null successfully will
+    # also result in the default settings being used since nothing will be
+    # read from this file.
+    setenv INPUTRC "/dev/null"
+
+    # The gdb.base/readline.exp arrow key test relies on the standard VT100
+    # bindings, so make sure that an appropriate terminal is selected.
+    # The same bug doesn't show up if we use ^P / ^N instead.
+    setenv TERM "vt100"
+
+    # Some tests (for example gdb.base/maint.exp) shell out from gdb to use
+    # grep.  Clear GREP_OPTIONS to make the behavoiur predictable, 
+    # especially having color output turned on can cause tests to fail.
+    setenv GREP_OPTIONS ""
+
     return [eval default_gdb_init $args];
 }
 



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-03-25 13:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-16 10:13 [PATCH] Clear GREP_OPTIONS environment variable Andrew Burgess
2011-03-16 15:43 ` Tom Tromey
2011-03-16 16:57   ` Joel Brobecker
2011-03-17 11:25     ` Andrew Burgess
2011-03-25 15:46       ` Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox