[PATCH v2] Fix gdb.base/gdbinit-history.exp when HISTSIZE is set in the environment (Re: [PATCH] [COMMITTED] Fix PR gdb/17820)

Pedro Alves
2015-05-15T18:33:56Z

On 05/15/2015 06:03 PM, Patrick Palka wrote:
> On Fri, May 15, 2015 at 12:05 PM, Pedro Alves <palves@redhat.com> wrote:
>> + if {$have_old_histsize} {
>> + set env(HISTSIZE) $old_histsize
>> + }
>
> Why not change this predicate to
>
> if [info exists old_histsize]
>
> to obviate the need for $have_old_histsize altogether?

Yeah, "info exists" is ok since this is local scope; I was worrying
that the code ends up copied elsewhere to global context, and
then the "info exists" would be the wrong thing to use, considering
e.g., [1] and [2]. Maybe I'm worrying too much. But how about instead
simply saving/restoring the whole env array, like in the updated
patch below, which sidesteps that issue?

[1] https://sourceware.org/ml/gdb-patches/2015-04/msg00261.html
[2] https://sourceware.org/ml/gdb-patches/2015-04/msg00537.html

--------
From b5c09f8bdd4f780d176d110ce61f4eb153ecc9ef Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 15 May 2015 19:27:07 +0100
Subject: [PATCH] Fix gdb.base/gdbinit-history.exp when HISTSIZE is set in the
environment

Some buildslaves are showing that this test is failing. E.g.,:

https://sourceware.org/ml/gdb-testers/2015-q2/msg04164.html

The issue is that HISTSIZE is set to 1000 in the environment that runs
the tests (that's the default in Fedora, set in /etc/profile).

We can trivially reproduce it with:

$ HISTSIZE=1000 make check RUNTESTFLAGS="gdbinit-history.exp"
(...)
Running /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.base/gdbinit-history.exp ...
FAIL: gdb.base/gdbinit-history.exp: show history size
FAIL: gdb.base/gdbinit-history.exp: show history size
FAIL: gdb.base/gdbinit-history.exp: show commands

gdb.log shows:
...
(gdb) set height 0
(gdb) set width 0
(gdb) show history size
The size of the command history is 1000.
(gdb) FAIL: gdb.base/gdbinit-history.exp: show history size

gdb/testsuite/ChangeLog:
2015-05-15 Pedro Alves <palves@redhat.com>

* gdb.base/gdbinit-history.exp (test_gdbinit_history_setting):
Save the whole env array instead of just HOME. Unset HISTSIZE in
the environment while testing. Restore whole environment
afterwards.
---
gdb/testsuite/gdb.base/gdbinit-history.exp | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/gdbinit-history.exp b/gdb/testsuite/gdb.base/gdbinit-history.exp
index 474680a..aba15b4 100644
--- a/gdb/testsuite/gdb.base/gdbinit-history.exp
+++ b/gdb/testsuite/gdb.base/gdbinit-history.exp
@@ -27,8 +27,15 @@ proc test_gdbinit_history_setting { home size } {
global srcdir
global subdir

- set old_home $env(HOME)
+ array set old_env [array get env]
+
set env(HOME) "$srcdir/$subdir/$home"
+
+ # The HISTSIZE environment variable takes precedence over whatever
+ # history size is set in .gdbinit. Make sure the former is not
+ # set.
+ unset -nocomplain env(HISTSIZE)
+
set saved_internal_gdbflags $INTERNAL_GDBFLAGS
set INTERNAL_GDBFLAGS [string map {"-nx" ""} $INTERNAL_GDBFLAGS]

@@ -44,7 +51,8 @@ proc test_gdbinit_history_setting { home size } {
}

set INTERNAL_GDBFLAGS $saved_internal_gdbflags
- set $env(HOME) $old_home
+
+ array set env [array get old_env]
}

test_gdbinit_history_setting "gdbinit-history/unlimited" "unlimited"
--
1.9.3