Re: [RFC] new board file 'remote-host-native.exp'

[Pedro Alves <palves at redhat dot com>]

On 07/24/2012 02:10 PM, Yao Qi wrote:
> Hi, when fixing other PR, I need to make sure test case running well on
> 'remote host' as well.  I write a new board file remote-host-native.exp
> so that we can run gdb testsuite in 'remote-host' locally.
> 
> When run testsuite for 'remote-host', I get following error,
> 
> ERROR: tcl error sourcing ../../../../git/gdb/testsuite/gdb.base/break.exp.
> ERROR: wrong # args: should be "gdb_download filename"
>     while executing
> "gdb_download "remote-host" ../../../../git/gdb/testsuite/gdb.base/break.c break.c"
>     ("eval" body line 1)
>     invoked from within
> "eval ${try}_${proc} \"$dest\" $args"
>     (procedure "call_remote" line 66)
>     invoked from within
> "call_remote "" download $dest $file $destfile"
>     (procedure "remote_download" line 30)
>     invoked from within
> "remote_download host $x"
>     (procedure "default_target_compile" line 323)
>     invoked from within
> "default_target_compile $source $destfile $type $options"
>     (procedure "target_compile" line 6)
>     invoked from within
> "target_compile $source $dest $type $options"
>     (procedure "gdb_compile" line 142)
>     invoked from within
> "gdb_compile "${srcdir}/${subdir}/${s}" "${binfile}${i}.o" object $options"
> 
> As we can see, dejagnu looks proc ${board}_download, and matches gdb_download.  However,

I'm confused at why ${board} expanded to gdb, the tool.

> we can't simply change the parameters of gdb_download to meet the required parameters.
> This will cause an endless recurisive call "remote_download->call_remote->gdb_download
> ->remote_download".  This patch simply renames this proc to gdb_download_1.
> 
> As this patch applied, there are a lot of fails when running testsuite with
> board file remote-host-native.exp.  Some of them are intermittent, and
> some of them reflect the flaws in test cases.
> 
> gdb/testsuite:
> 
> 2012-07-24  Yao Qi  <yao@codesourcery.com>
> 
> 	* boards/remote-host-native.exp: New board file.
> 	* lib/gdb.exp: Rename gdb_downlad to gdb_download_1.
> 	(gdb_load_shlibs): Caller update.
> 	* lib/gdbserver-support.exp (gdbserver_download_current_prog):
> 	Likewise.
> 	* lib/mi-support.exp (mi_load_shlibs): Likewise.
> 	* gdb.base/jit-so.exp, gdb.base/jit.exp: Likewise.
> ---
>  gdb/testsuite/boards/remote-host-native.exp |   60 +++++++++++++++++++++++++++
>  gdb/testsuite/gdb.base/jit-so.exp           |    2 +-
>  gdb/testsuite/gdb.base/jit.exp              |    2 +-
>  gdb/testsuite/lib/gdb.exp                   |    6 +-
>  gdb/testsuite/lib/gdbserver-support.exp     |    2 +-
>  gdb/testsuite/lib/mi-support.exp            |    2 +-
>  6 files changed, 67 insertions(+), 7 deletions(-)
>  create mode 100644 gdb/testsuite/boards/remote-host-native.exp
> 
> diff --git a/gdb/testsuite/boards/remote-host-native.exp b/gdb/testsuite/boards/remote-host-native.exp
> new file mode 100644
> index 0000000..cdb0684
> --- /dev/null
> +++ b/gdb/testsuite/boards/remote-host-native.exp
> @@ -0,0 +1,60 @@
> +# 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/>.
> +
> +# This file is a dejagnu "board file" and is used to run the testsuite
> +# natively in remote host.

Why natively?  I'd think you could use both this host board with e.g.,
a gdbserver target board file.  I suggest:

+# This file is a dejagnu "board file" and is used to run the testsuite
+# against local host, in remote host mode.

I'd even rename the file to local-remote-host.exp.

> +#
> +# To use this file:
> +# bash$ touch ${my_dejagnu_dir}/my-dejagnu.exp
> +# bash$ export DEJAGNU=${my_dejagnu_dir}/my-dejagnu.exp
> +# bash$ mkdir ${my_dejagnu_dir}/boards
> +# bash$ cp ${src_dir}/gdb/testsuite/boards/remote-host-native.exp \
> +#   ${my_dejagnu_dir}/boards
> +
> +# Modify the location of GDB and the username to access your box
> +# below.
> +
> +# bash$ cd ${build_dir}/gdb
> +# bash$ make check RUNTESTFLAGS="--host_board=remote-host-native"
> +
> +load_generic_config "gdb"

Why is this necessary?  Why "gdb", and not "unix" ?  What is "gdb"
actually loading?  Isn't this "gdb" the reason remote_download maps
to gdb_download ?  Otherwise, that gdb_download issue should have tripped
up other configurations doing remote host testing, I'd think.

> +
> +set_board_info gdb_protocol "standard"

Why is this necessary?  A host board file should only specify the bits
for downloading and executing bits on the host, I think.

> +
> +global GDB
> +# Specify the location of GDB binary.
> +set GDB "/your/built/gdb"

How about we make this pick up the just built GDB ?

 set GDB [file join [pwd] "../gdb"]

> +
> +set_board_info compiler "[find_gcc]"

Is this necessary in a host board file?

> +set_board_info hostname 127.0.0.1
> +
> +set_board_info username YOUR.USER.NAME

We can make this pick up the current user by default on at least GNU/Linux,
and probably other Unixen:

 set_board_info username $::env(USER)


> +
> +# The ssh key should be correctly set up that you ssh to 127.0.0.1
> +# without having to type password.
> +set_board_info rsh_prog /usr/bin/ssh
> +set_board_info rcp_prog /usr/bin/scp
> +set_board_info file_transfer "rsh"

> +
> +proc ${board}_download { board src dest } {
> +
> +    # If file name is a relative, convert it to absolute, otherwise file can't
> +    # be found on host, because the current directory usually is /home/$USER.
> +    if { [file pathtype $src] == "relative" } {
> +	return [file join [pwd] $src]
> +    } else {
> +	return $src
> +    }
> +}

Hmm, so the intention is to bypass the download to the host.  I'd like a small
comment above the function that says something to the effect.


> diff --git a/gdb/testsuite/gdb.base/jit-so.exp b/gdb/testsuite/gdb.base/jit-so.exp
> index 36f18ad..95c2eac 100644
> --- a/gdb/testsuite/gdb.base/jit-so.exp
> +++ b/gdb/testsuite/gdb.base/jit-so.exp
> @@ -61,7 +61,7 @@ if { [gdb_compile_shlib ${solib_srcfile} ${solib_binfile} {}] != "" } {
>      return -1
>  }
>  
> -set solib_binfile_target [gdb_download ${solib_binfile}]
> +set solib_binfile_target [gdb_download_1 ${solib_binfile}]

Someone might well in the future notice there's a "gdb_download_1" overload
but no "gdb_download", and decide to "clean up" things by renaming "gdb_download_1"
back to "gdb_download"...  I'd prefer a better name.  But, in any case, I'm confused
at why this board trips on this, while other remote host testing CS does, does not.

-- 
Pedro Alves