[PATCH] [RFC] Automatically append 'gdb/testsuite/boards' to DejaGnu's boards dir list.

[PATCH] [RFC] Automatically append 'gdb/testsuite/boards' to DejaGnu's boards dir list.

[Pedro Alves]

I wonder what people think of this...

This makes it possible to just do

  'make check RUNTESTFLAGS="native-gdbserver.exp"'

without having to setup a DEJAGNU environment, (set
$DEJAGNU=...site.exp, copy/symlink the boards dir, etc.).

It'll Just Work.

Care is taken to not override any boards dir the user may have set.

Even if DejaGnu is patched to support this without contortions (e.g.,
with a new $extra_boards_dir variable), I'm thinking that it's worth
it to make this work with current DejaGnu's.

Anyone see a simpler way to do this?

DejaGnu (runtest.exp) does, at global scope:

if {[info exists env(DEJAGNU)]} {
    if { [load_file -- $env(DEJAGNU)] == 0 } {
	# It may seem odd to only issue a warning if there isn't a global
	# config file, but issue an error if $DEJAGNU is erroneously defined.
	# Since $DEJAGNU is set there is *supposed* to be a global config file,
	# so the current behaviour seems reasonable.
	send_error "WARNING: global config file $env(DEJAGNU) not found.\n"
    }
    if {![info exists boards_dir]} {
	set boards_dir "[file dirname $env(DEJAGNU)]/boards"
    }
}

if {![info exists boards_dir]} {
    set boards_dir ""
}

So we can't just "lappend boards_dir ..." unconditionally from
testsuite/site.exp, as DejaGnu would skip the '[file dirname
$env(DEJAGNU)]/boards' bit.  Even if we could, that'd make our boards
dir first in the list, while I think it should be last.

testsuite/
2012-03-31  Pedro Alves  <palves@redhat.com>

	* Makefile.in (site.exp): Make site.exp source
	$srcdir/append_gdb_boards_dir.
	* append_gdb_boards_dir: New file.
---
 gdb/testsuite/Makefile.in           |    2 ++
 gdb/testsuite/append_gdb_boards_dir |   45 +++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 0 deletions(-)
 create mode 100644 gdb/testsuite/append_gdb_boards_dir

diff --git a/gdb/testsuite/Makefile.in b/gdb/testsuite/Makefile.in
index b06c382..55fede3 100644
--- a/gdb/testsuite/Makefile.in
+++ b/gdb/testsuite/Makefile.in
@@ -120,6 +120,8 @@ $(abs_builddir)/site.exp site.exp: ./config.status Makefile
 	@echo "set build_triplet ${build_canonical}" >> ./tmp0
 	@echo "set srcdir ${srcdir}" >> ./tmp0
 	@echo "set tool gdb" >> ./tmp0
+	@echo 'source $${srcdir}/append_gdb_boards_dir' >> ./tmp0
+
 	@echo "## All variables above are generated by configure. Do Not Edit ##" >> ./tmp0
 		@cat ./tmp0 > site.exp
 	@cat site.bak | sed \
diff --git a/gdb/testsuite/append_gdb_boards_dir b/gdb/testsuite/append_gdb_boards_dir
new file mode 100644
index 0000000..d595fd6
--- /dev/null
+++ b/gdb/testsuite/append_gdb_boards_dir
@@ -0,0 +1,45 @@
+# Copyright 2012 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Make the boards under testsuite/boards automatically available to
+# DejaGnu.  This is sourced by the site.exp file generated in the
+# build/gdb/testsuite/ directory.
+
+# Append GDB's boards dir to DejaGnu's board dir list, making sure
+# that that is the last boards dir in the list, so that boards found
+# in directories appended to the list by either the global config
+# site.exp, or in `$(dirname $DEJAGNU)/boards' override GDB's boards.
+# Unfortunatelly, there's no standard way to get that behavior.  To
+# make it happen, we trace all writes to the $boards_dirs global, and
+# make sure our dir is always the last in the list.  This relies on
+# the fact that DejaGnu always writes to this variable, even if just
+# to set it to the empty list, which it does.
+#
+proc append_gdb_boards_dir { name1 name2 op } {
+    global srcdir
+    global boards_dir
+
+    # The "boards/../boards" contortion accounts for the odd chance of
+    # someone wanting to point at GDB's boards dir in the global
+    # config before some other board dir, in which case we should not
+    # push it to the end of the list.
+    set gdb_boards_dir "${srcdir}/boards/../boards"
+
+    # Keep our board dir last in the list.
+    set idx [lsearch $boards_dir "$gdb_boards_dir"]
+    set boards_dir [lreplace $boards_dir $idx $idx]
+    lappend boards_dir "${gdb_boards_dir}"
+}
+trace add variable "boards_dir" write append_gdb_boards_dir

Re: [PATCH] [RFC] Automatically append 'gdb/testsuite/boards' to DejaGnu's boards dir list.

[Jan Kratochvil]

On Sat, 31 Mar 2012 05:02:21 +0200, Pedro Alves wrote:
> +++ b/gdb/testsuite/append_gdb_boards_dir

It should be in testsuite/lib/ , shouldn't it?


> +proc append_gdb_boards_dir { name1 name2 op } {
> +    global srcdir
> +    global boards_dir
> +
> +    # The "boards/../boards" contortion accounts for the odd chance of
> +    # someone wanting to point at GDB's boards dir in the global
> +    # config before some other board dir, in which case we should not
> +    # push it to the end of the list.
> +    set gdb_boards_dir "${srcdir}/boards/../boards"
> +
> +    # Keep our board dir last in the list.
> +    set idx [lsearch $boards_dir "$gdb_boards_dir"]

lsearch -exact


> +    set boards_dir [lreplace $boards_dir $idx $idx]

lreplace X -1 -1 seems not to modify X but according to the doc it seems to be
undefined.  Suggesting one 'if' conditional, ale for the code documentation
purposes.


> +    lappend boards_dir "${gdb_boards_dir}"
> +}
> +trace add variable "boards_dir" write append_gdb_boards_dir

There were some complaints on the list for features not compatible with
tcl-8.3, 'trace add' is not listed in tcl-8.3 doc:
	http://www.tcl.tk/man/tcl8.3/TclCmd/trace.htm


Sure a great feature.


Thanks,
Jan

Re: [PATCH] [RFC] Automatically append 'gdb/testsuite/boards' to DejaGnu's boards dir list.

[Pedro Alves]

On 03/31/2012 08:29 AM, Jan Kratochvil wrote:

> On Sat, 31 Mar 2012 05:02:21 +0200, Pedro Alves wrote:
>> +++ b/gdb/testsuite/append_gdb_boards_dir
> 
> It should be in testsuite/lib/ , shouldn't it?


Yeah, a series of different approaches made me miss that.
Appended an .exp suffix as well.

> 
> 
>> +proc append_gdb_boards_dir { name1 name2 op } {
>> +    global srcdir
>> +    global boards_dir
>> +
>> +    # The "boards/../boards" contortion accounts for the odd chance of
>> +    # someone wanting to point at GDB's boards dir in the global
>> +    # config before some other board dir, in which case we should not
>> +    # push it to the end of the list.
>> +    set gdb_boards_dir "${srcdir}/boards/../boards"
>> +
>> +    # Keep our board dir last in the list.
>> +    set idx [lsearch $boards_dir "$gdb_boards_dir"]
> 
> lsearch -exact


Indeed.

> 
> 
>> +    set boards_dir [lreplace $boards_dir $idx $idx]
> 
> lreplace X -1 -1 seems not to modify X but according to the doc it seems to be
> undefined.  Suggesting one 'if' conditional, ale for the code documentation
> purposes.


Done.

> 
> 
>> +    lappend boards_dir "${gdb_boards_dir}"
>> +}
>> +trace add variable "boards_dir" write append_gdb_boards_dir
> 
> There were some complaints on the list for features not compatible with
> tcl-8.3, 'trace add' is not listed in tcl-8.3 doc:
> 	http://www.tcl.tk/man/tcl8.3/TclCmd/trace.htm
> 


Interesting.  I knew it wasn't supported on older tcls, but since we're
already using "trace add" in gdb.exp I wasn't worrying (it
wasn't a new problem).  But that url points at alternative (older) syntax
we can use to the same effect, so I switched to it.  I don't see any
"trace add execution" there though; the gdb.exp bits should be
perhaps skipped somehow on older tcls.

I also updated the comments a bit.

Below's the updated patch.

Thanks.

testsuite/
2012-03-31  Pedro Alves  <palves@redhat.com>

	* Makefile.in (site.exp): Make site.exp source
	$srcdir/lib/append_gdb_boards_dir.exp.
	* lib/append_gdb_boards_dir.exp: New file.
---

 gdb/testsuite/Makefile.in                   |    1 +
 gdb/testsuite/lib/append_gdb_boards_dir.exp |   48 +++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 0 deletions(-)
 create mode 100644 gdb/testsuite/lib/append_gdb_boards_dir.exp

diff --git a/gdb/testsuite/Makefile.in b/gdb/testsuite/Makefile.in
index b06c382..6711930 100644
--- a/gdb/testsuite/Makefile.in
+++ b/gdb/testsuite/Makefile.in
@@ -120,6 +120,7 @@ $(abs_builddir)/site.exp site.exp: ./config.status Makefile
 	@echo "set build_triplet ${build_canonical}" >> ./tmp0
 	@echo "set srcdir ${srcdir}" >> ./tmp0
 	@echo "set tool gdb" >> ./tmp0
+	@echo 'source $${srcdir}/lib/append_gdb_boards_dir.exp' >> ./tmp0
 	@echo "## All variables above are generated by configure. Do Not Edit ##" >> ./tmp0
 		@cat ./tmp0 > site.exp
 	@cat site.bak | sed \
diff --git a/gdb/testsuite/lib/append_gdb_boards_dir.exp b/gdb/testsuite/lib/append_gdb_boards_dir.exp
new file mode 100644
index 0000000..4b01f2b
--- /dev/null
+++ b/gdb/testsuite/lib/append_gdb_boards_dir.exp
@@ -0,0 +1,48 @@
+# Copyright 2012 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Make the boards under testsuite/boards automatically available to
+# DejaGnu.  This is sourced by the `site.exp' file generated in the
+# root of the testsuite's build directory.
+
+# Append GDB's boards dir to DejaGnu's board dir list, making sure
+# that that is the last boards dir in the list, so that boards found
+# in directories appended to the list by either the global config
+# site.exp, or in `$(dirname $DEJAGNU)/boards' override GDB's boards.
+# Unfortunately, there's no standard way to get that behavior.  To
+# make it happen, we trace all writes to the $boards_dirs global, and
+# make sure our dir is always the last in the list.  This relies on
+# the fact that DejaGnu always writes to this variable, even if just
+# to set it to the empty list, which it does.
+#
+proc append_gdb_boards_dir { name1 name2 op } {
+    global srcdir
+    global boards_dir
+
+    # In case someone wants to point at GDB's boards dir in the global
+    # config before some other board dir, in which case we should not
+    # push it to the end of the list, use an unlikely path to GDB's
+    # boards, so it compares different to other simpler but equivalent
+    # paths.
+    set gdb_boards_dir "${srcdir}/boards/../boards"
+
+    # Keep our board dir last in the list.
+    set idx [lsearch -exact $boards_dir "$gdb_boards_dir"]
+    if { $idx >= 0 } {
+	set boards_dir [lreplace $boards_dir $idx $idx]
+    }
+    lappend boards_dir "${gdb_boards_dir}"
+}
+trace variable "boards_dir" w append_gdb_boards_dir

Re: [PATCH] [RFC] Automatically append 'gdb/testsuite/boards' to DejaGnu's boards dir list.

[Pedro Alves]

On 04/02/2012 05:05 AM, Doug Evans wrote:
> On Mar 30, 2012 8:02 PM, "Pedro Alves" <palves@redhat.com <mailto:palves@redhat.com>> wrote:>
>> Even if DejaGnu is patched to support this without contortions (e.g.,
>> with a new $extra_boards_dir variable), I'm thinking that it's worth
>> it to make this work with current DejaGnu's.
> TBH, this feels like a hacky workaround when the right fix is to fix/enhance Dejagnu.

Sure.  See quoted above.

-- 
Pedro Alves

Re: [PATCH] [RFC] Automatically append 'gdb/testsuite/boards' to DejaGnu's boards dir list.

[Joel Brobecker]

Hi Pedro,

> I wonder what people think of this...
> 
> This makes it possible to just do
> 
>   'make check RUNTESTFLAGS="native-gdbserver.exp"'
> 
> without having to setup a DEJAGNU environment, (set
> $DEJAGNU=...site.exp, copy/symlink the boards dir, etc.).

I was hoping that I would have some time to look at the patch,
but I don't think it's going to happen this week. But I just LOVE
the idea of it. In fact, I have been doing my own testing by
setting DEJAGNU=... to the board file in the source directory
itself, rather than make a copy. The only thing I needed to do
was create a boards directory inside the sources which was less
than ideal. So, I'm all for your enhancement.

Thanks for doing this!

-- 
Joel

[commit] Re: [PATCH] [RFC] Automatically append 'gdb/testsuite/boards' to DejaGnu's boards dir list.

[Pedro Alves]

On 03/31/2012 11:30 AM, Pedro Alves wrote:

> 
> testsuite/
> 2012-03-31  Pedro Alves  <palves@redhat.com>
> 
> 	* Makefile.in (site.exp): Make site.exp source
> 	$srcdir/lib/append_gdb_boards_dir.exp.
> 	* lib/append_gdb_boards_dir.exp: New file.


I've checked this in now.

Thanks,
-- 
Pedro Alves