[PATCH] Another board file for remote host

[PATCH] Another board file for remote host

[Yao Qi]

In the recent review to my patch about copying files to remote host,
we find that we need a board file which is more closely mapped real
remote host testing to improve coverage.  With the board file
local-remote-host-native.exp, DejaGNU copies files to /tmp/gdb/ to
emulate the effect of remote host.  Is it OK?

gdb/testsuite:

2014-08-28  Yao Qi  <yao@codesourcery.com>

	* boards/local-remote-host-native.exp: New file.
---
 gdb/testsuite/boards/local-remote-host-native.exp | 79 +++++++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100644 gdb/testsuite/boards/local-remote-host-native.exp

diff --git a/gdb/testsuite/boards/local-remote-host-native.exp b/gdb/testsuite/boards/local-remote-host-native.exp
new file mode 100644
index 0000000..a3fd2dc
--- /dev/null
+++ b/gdb/testsuite/boards/local-remote-host-native.exp
@@ -0,0 +1,79 @@
+# Copyright 2014 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
+# against local host, in remote host mode.
+#
+# To use this file:
+# bash$ cd ${build_dir}/gdb
+# bash$ make check RUNTESTFLAGS="--host_board=local-remote-host-native --target_board=local-remote-host-native"
+
+if { $board_type == "target" } {
+    set_board_info compiler gcc
+}
+
+global GDB
+set GDB [file join [pwd] "../gdb"]
+
+set_board_info hostname 127.0.0.1
+
+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"
+
+# The directory to copy files to.  In default, we choose /tmp/gdb, to
+# avoiding messing up your HOME.  You can choose other directory as
+# you like.
+set tmp_dir "/tmp/gdb"
+
+if { $board_type == "host" } {
+    set_board_info gdb_opts "-ex \"dir ${tmp_dir}\""
+}
+
+proc ${board}_spawn { board cmd } {
+    global board_info
+
+    set remote [board_info $board hostname]
+    set username [board_info $board username]
+    set RSH [board_info $board rsh_prog]
+
+    spawn $RSH -t -l $username $remote $cmd
+    set board_info($board,fileid) $spawn_id
+    return $spawn_id
+}
+
+proc ${board}_download { board src dest } {
+    global env board_type
+    global tmp_dir
+
+    if { [llength $dest] > 0 } {
+	set destfile [lindex $dest 0]
+    } else {
+	set destfile [file tail $src]
+    }
+
+    if { ![file exists $tmp_dir] } {
+	file mkdir $tmp_dir
+    }
+
+    set destfile [file join $tmp_dir $destfile]
+    file copy -force $src $destfile
+
+    return $destfile
+}
-- 
1.9.3

Re: [PATCH] Another board file for remote host

[Pedro Alves]

On 08/28/2014 02:31 PM, Yao Qi wrote:
> In the recent review to my patch about copying files to remote host,
> we find that we need a board file which is more closely mapped real
> remote host testing to improve coverage.  With the board file
> local-remote-host-native.exp, DejaGNU copies files to /tmp/gdb/ to
> emulate the effect of remote host.  Is it OK?
> 
> gdb/testsuite:
> 
> 2014-08-28  Yao Qi  <yao@codesourcery.com>
> 
> 	* boards/local-remote-host-native.exp: New file.
> ---
>  gdb/testsuite/boards/local-remote-host-native.exp | 79 +++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
>  create mode 100644 gdb/testsuite/boards/local-remote-host-native.exp
> 
> diff --git a/gdb/testsuite/boards/local-remote-host-native.exp b/gdb/testsuite/boards/local-remote-host-native.exp
> new file mode 100644
> index 0000000..a3fd2dc
> --- /dev/null
> +++ b/gdb/testsuite/boards/local-remote-host-native.exp

I'm confused on the choice of "native" for name.
local-remote-host.exp/local-remote-host-notty.exp seem just or more
more native to me, as they assume a shared filesystem, while
the whole point of this one is emulate the case of _not_ sharing
the filesystem?

> +# To use this file:
> +# bash$ cd ${build_dir}/gdb
> +# bash$ make check RUNTESTFLAGS="--host_board=local-remote-host-native --target_board=local-remote-host-native"
> +
> +if { $board_type == "target" } {
> +    set_board_info compiler gcc

Why do we need to also use the board as a target board?

> +}

> +if { $board_type == "host" } {
> +    set_board_info gdb_opts "-ex \"dir ${tmp_dir}\""

No biggie, but this:

  set_board_info gdb_opts "-d \"${tmp_dir}\""

looks a little simpler to me, just for the fact that it runs
through less code / fewer indirections in GDB.

> +}

-- 
Thanks,
Pedro Alves

Re: [PATCH] Another board file for remote host

[Jan Kratochvil]

On Thu, 28 Aug 2014 15:31:30 +0200, Yao Qi wrote:
> With the board file
> local-remote-host-native.exp, DejaGNU copies files to /tmp/gdb/ to
> emulate the effect of remote host.  Is it OK?

Could it use some more unique directory name so that parallel runs of
testsuites from multiple independent GDB directories work?
And also:
	cd gdb; make -j20 -k check//unix/-m64 check//unix/-m32
	etc.

IIUC instead of [file tail $src] it may be better to use $src's absolute
directory.

Sorry I did not test this patch if I miss anything.


Thanks,
Jan

Re: [PATCH] Another board file for remote host

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> I'm confused on the choice of "native" for name.
> local-remote-host.exp/local-remote-host-notty.exp seem just or more
> more native to me, as they assume a shared filesystem, while
> the whole point of this one is emulate the case of _not_ sharing
> the filesystem?

This board file is used to emulate the real remote host testing, in
which file system of host and build are not shared.  To be concrete,
I'd like this board file to emulate the testing we do for native
mingw32 GDB.  GDB is built on x86_64-linux, is run on Windows, and
does debugging on Windows.  I find such testing mode exposes some
problems in test cases, such as files not copied to host or referencing
build file path on host, etc.

Existing board files local-remote-host-*.exp share the file system, so
the problem of copying needed files to host is not exposed.

This board file is to emulate the testing to a native gdb (host ==
target) on a remote host (host != build), so I name it
local-remote-host-native.  At first, I name it local-remote-host-target,
but I discard it as it is confusing.

>
>> +# To use this file:
>> +# bash$ cd ${build_dir}/gdb
>> +# bash$ make check
>> RUNTESTFLAGS="--host_board=local-remote-host-native
>> --target_board=local-remote-host-native"
>> +
>> +if { $board_type == "target" } {
>> +    set_board_info compiler gcc
>
> Why do we need to also use the board as a target board?
>

Because [is_remote target] is true when I pass
--host_board=local-remote-host-native
--target_board=local-remote-host-native to DejaGNU.  If I do
"--host_board=local-remote-host-native (--target_board=unix)",
[is_remote target] is false.  Since I am doing test under target == host
!= build, so [is_remote target] is expected to be true, isn't?

>> +}
>
>> +if { $board_type == "host" } {
>> +    set_board_info gdb_opts "-ex \"dir ${tmp_dir}\""
>
> No biggie, but this:
>
>   set_board_info gdb_opts "-d \"${tmp_dir}\""
>
> looks a little simpler to me, just for the fact that it runs
> through less code / fewer indirections in GDB.

OK, I'll fix it.

I realize that the rationale and explanations are missing for this board
file, and I'll add more comments about them if the board file is OK.

-- 
Yao (齐尧)

Re: [PATCH] Another board file for remote host

[Yao Qi]

Jan Kratochvil <jan.kratochvil@redhat.com> writes:

> Could it use some more unique directory name so that parallel runs of
> testsuites from multiple independent GDB directories work?
> And also:
> 	cd gdb; make -j20 -k check//unix/-m64 check//unix/-m32
> 	etc.

Do we want to run testing in remote host in parallel?  If RUNTESTFLAGS
is not empty, the tests will be serialized unless FORCE_PARALLEL is set.
IIRC, when Tom worked on parallel testing, we think it should be fine to
leave remote host testing serialized.

>
> IIUC instead of [file tail $src] it may be better to use $src's absolute
> directory.

Do you mean, supposing $src is /home/yao/src/gdb/testsuite/gdb.base/foo.c,
copy it to /tmp/gdb/home/yao/src/gdb/testsuite/gdb.base/foo.c?

-- 
Yao (齐尧)

Re: [PATCH] Another board file for remote host

[Doug Evans]

On Thu, Aug 28, 2014 at 6:31 AM, Yao Qi <yao@codesourcery.com> wrote:
> In the recent review to my patch about copying files to remote host,
> we find that we need a board file which is more closely mapped real
> remote host testing to improve coverage.  With the board file
> local-remote-host-native.exp, DejaGNU copies files to /tmp/gdb/ to
> emulate the effect of remote host.  Is it OK?
>
> gdb/testsuite:
>
> 2014-08-28  Yao Qi  <yao@codesourcery.com>
>
>         * boards/local-remote-host-native.exp: New file.

I haven't given this a try yet, which I want to do, but thanks for
taking this on!

Re: [PATCH] Another board file for remote host

[Jan Kratochvil]

On Fri, 29 Aug 2014 03:51:51 +0200, Yao Qi wrote:
> Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> > Could it use some more unique directory name so that parallel runs of
> > testsuites from multiple independent GDB directories work?
> > And also:
> > 	cd gdb; make -j20 -k check//unix/-m64 check//unix/-m32
> > 	etc.
> 
> Do we want to run testing in remote host in parallel?  If RUNTESTFLAGS
> is not empty, the tests will be serialized unless FORCE_PARALLEL is set.

This is unrelated as I can run multiple testsuites at once on the same host
from unrelated GDB source-trees/build/trees.

Besides that I do use FORCE_PARALLEL:
	http://git.jankratochvil.net/?p=nethome.git;a=blob;f=bin/hammock

I even run all testsuites twice at the same time - for i686 and x86_64
although I run them in different chroots so this issue would not matter.
But I could run the i686 build even on x86_64 just compiling everything
with -m32 (which I also do).


> > IIUC instead of [file tail $src] it may be better to use $src's absolute
> > directory.
> 
> Do you mean, supposing $src is /home/yao/src/gdb/testsuite/gdb.base/foo.c,
> copy it to /tmp/gdb/home/yao/src/gdb/testsuite/gdb.base/foo.c?

Yes.  There may be other options (such as using something PID-based etc.) but
I find the absolute pathname probably the best one.


Thanks,
Jan

Re: [PATCH] Another board file for remote host

[Yao Qi]

Jan Kratochvil <jan.kratochvil@redhat.com> writes:

>> > 	cd gdb; make -j20 -k check//unix/-m64 check//unix/-m32
>> > 	etc.
>> 
>> Do we want to run testing in remote host in parallel?  If RUNTESTFLAGS
>> is not empty, the tests will be serialized unless FORCE_PARALLEL is set.
>
> This is unrelated as I can run multiple testsuites at once on the same host
> from unrelated GDB source-trees/build/trees.
>
> Besides that I do use FORCE_PARALLEL:
> 	http://git.jankratochvil.net/?p=nethome.git;a=blob;f=bin/hammock

Do you plan to use this board file local-remote-host-native.exp to run
tests in parallel (set FORCE_PARALLEL to 1)?  If yes, I am not sure it
is correct, because I don't think current gdb testsuite support parallel
testing for remote host.  If no, why do we have to worry about parallel
testing?  We can mention that parallel testing with this board file is
not supported.

>
> I even run all testsuites twice at the same time - for i686 and x86_64
> although I run them in different chroots so this issue would not matter.
> But I could run the i686 build even on x86_64 just compiling everything
> with -m32 (which I also do).

If you think it is unsafe to copy files to a global place (/tmp/gdb/),
how about copy files to ${build_testsuite}/remote-host?
${build_testsuite} is the directory created by GDB having the
"testsuite." prefix.  For example, if we do:

  make -k check //unix/{-m32,-m64}

the ${build_testsuite} will be ${build_dir}/gdb/testsuite.unix.{-m32,-m64},
and source files are copied to
${build_dir}/gdb/testsuite.unix.{-m32,-m64}/remote-host.  Is it good to you?

-- 
Yao (齐尧)

Re: [PATCH] Another board file for remote host

[Jan Kratochvil]

On Mon, 01 Sep 2014 10:10:32 +0200, Yao Qi wrote:
> Do you plan to use this board file local-remote-host-native.exp to run
> tests in parallel (set FORCE_PARALLEL to 1)?

I do not think I will myself but I guess someone should setup a nightly (or
even per-commit) regression testing even in this mode.


> If yes, I am not sure it is correct, because I don't think current gdb
> testsuite support parallel testing for remote host.

I have never been dealing with remote hosts testing so I do not know.
But I find it a bug in such case.


> If no, why do we have to worry about parallel testing?

I find it a bug that something does not work during parallel testing while it
could.  Testsuite takes IIRC half an hour or more in non-parallel mode.


> We can mention that parallel testing with this board file is
> not supported.

In such case it should IMNSHO print an error so that people do not have to
chase random testsuite failures if they do so and they miss the documentation
mentioning it.


> If you think it is unsafe to copy files to a global place (/tmp/gdb/),
> how about copy files to ${build_testsuite}/remote-host?

I find that really the best.  Although it still should use more than
a basename as there are duplicate basenames in the gdb.* directions:
	for i in gdb/testsuite/gdb.*;do find $i -type f -printf "%f\n";done|sort|uniq -d


Thanks,
Jan

Re: [PATCH] Another board file for remote host

[Yao Qi]

Hi, here is the V2 to address comments I've got from Jan and Pedro:

 - Add more comments in the board file.
 - Rename variable tmp_gdb to host_dir, and set it to
   $build/gdb/testsuite/remote-host.
 - I also tried to copy file to $build/gdb/testsuite/remote-host with
   the directory, say copy file $src/gdb/testsuite/gdb.dwarf2/file1.txt
   to $build/gdb/testsuite/remote-host/gdb.dwarf2/file1.txt.  This
   is suggested by Jan but GDB is unable to find file1.txt, because GDB
   test harness doesn't set search directories for remote host, see
   gdb_reinitialize_dir in lib/gdb.exp:

proc gdb_reinitialize_dir { subdir } {
    global gdb_prompt

    if [is_remote host] {
	return ""
    }

   so we have to set the search directory in the board file like

      set_board_info gdb_opts "-d \"${host_dir}\""

   and copy file there without directory.
-- 
Yao (齐尧)

Subject: [PATCH] Another board file for remote host

In the recent review to my patch about copying files to remote host,
we find that we need a board file which is more closely mapped real
remote host testing to improve coverage.  With the board file
local-remote-host-native.exp, DejaGNU copies files to
$build/gdb/testsuite/remote-host to emulate the effect of remote host.
Is it OK?

gdb/testsuite:

2014-09-02  Yao Qi  <yao@codesourcery.com>

	* boards/local-remote-host-native.exp: New file.

diff --git a/gdb/testsuite/boards/local-remote-host-native.exp b/gdb/testsuite/boards/local-remote-host-native.exp
new file mode 100644
index 0000000..b2a0b31
--- /dev/null
+++ b/gdb/testsuite/boards/local-remote-host-native.exp
@@ -0,0 +1,86 @@
+# Copyright 2014 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
+# against local host, in remote host mode.
+#
+# This board file is used to emulate the real remote host testing, in
+# which file system of host and build are not shared.  This is achieved
+# by copying files from source directory to ${host_dir}.
+#
+# To use this file:
+# bash$ cd ${build_dir}/gdb
+# bash$ make check RUNTESTFLAGS="--host_board=local-remote-host-native --target_board=local-remote-host-native"
+#
+# We set both target board and host board together to test a native gdb
+# (host == target) on a remote host (host != build).
+
+if { $board_type == "target" } {
+    set_board_info compiler gcc
+}
+
+global GDB
+set GDB [file join [pwd] "../gdb"]
+
+set_board_info hostname 127.0.0.1
+
+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"
+
+# The directory to copy files to.  In default, we choose ./remote-host, to
+# avoiding messing up your HOME.  You can choose other directory as
+# you like.
+set host_dir [file join [pwd] "remote-host"]
+
+if { $board_type == "host" } {
+    set_board_info gdb_opts "-d \"${host_dir}\""
+}
+
+proc ${board}_spawn { board cmd } {
+    global board_info
+
+    set remote [board_info $board hostname]
+    set username [board_info $board username]
+    set RSH [board_info $board rsh_prog]
+
+    spawn $RSH -t -l $username $remote $cmd
+    set board_info($board,fileid) $spawn_id
+    return $spawn_id
+}
+
+proc ${board}_download { board src dest } {
+    global env board_type
+    global host_dir
+
+    if { [llength $dest] > 0 } {
+	set destfile [lindex $dest 0]
+    } else {
+	set destfile [file tail $src]
+    }
+
+    if { ![file exists $host_dir] } {
+	file mkdir $host_dir
+    }
+
+    set destfile [file join $host_dir $destfile]
+    file copy -force $src $destfile
+
+    return $destfile
+}

Re: [PATCH] Another board file for remote host

[Doug Evans]

Yao Qi <yao@codesourcery.com> writes:
> Hi, here is the V2 to address comments I've got from Jan and Pedro:
>
>  - Add more comments in the board file.
>  - Rename variable tmp_gdb to host_dir, and set it to
>    $build/gdb/testsuite/remote-host.
>  - I also tried to copy file to $build/gdb/testsuite/remote-host with
>    the directory, say copy file $src/gdb/testsuite/gdb.dwarf2/file1.txt
>    to $build/gdb/testsuite/remote-host/gdb.dwarf2/file1.txt.  This
>    is suggested by Jan but GDB is unable to find file1.txt, because GDB
>    test harness doesn't set search directories for remote host, see
>    gdb_reinitialize_dir in lib/gdb.exp:
>
> proc gdb_reinitialize_dir { subdir } {
>     global gdb_prompt
>
>     if [is_remote host] {
> 	return ""
>     }
>
>    so we have to set the search directory in the board file like
>
>       set_board_info gdb_opts "-d \"${host_dir}\""
>
>    and copy file there without directory.
> [...]

Hi.  Some comments inline.

2014-09-02  Yao Qi  <yao@codesourcery.com>

	* boards/local-remote-host-native.exp: New file.

diff --git a/gdb/testsuite/boards/local-remote-host-native.exp b/gdb/testsuite/boards/local-remote-host-native.exp
new file mode 100644
index 0000000..b2a0b31
--- /dev/null
+++ b/gdb/testsuite/boards/local-remote-host-native.exp
@@ -0,0 +1,86 @@
+# Copyright 2014 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
+# against local host, in remote host mode.
+#
+# This board file is used to emulate the real remote host testing, in
+# which file system of host and build are not shared.  This is achieved
+# by copying files from source directory to ${host_dir}.
+#
+# To use this file:
+# bash$ cd ${build_dir}/gdb
+# bash$ make check RUNTESTFLAGS="--host_board=local-remote-host-native --target_board=local-remote-host-native"
+#
+# We set both target board and host board together to test a native gdb
+# (host == target) on a remote host (host != build).
+
+if { $board_type == "target" } {
+    set_board_info compiler gcc
+}
+
+global GDB
+set GDB [file join [pwd] "../gdb"]

Check if GDB has been set first, allow the user to
pass this in from the command line.

+
+set_board_info hostname 127.0.0.1
+
+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"
+
+# The directory to copy files to.  In default, we choose ./remote-host, to
+# avoiding messing up your HOME.  You can choose other directory as
+# you like.
+set host_dir [file join [pwd] "remote-host"]

How about allowing the user to pass this in from the command line?
By convention such variables are then uppercase.

+
+if { $board_type == "host" } {
+    set_board_info gdb_opts "-d \"${host_dir}\""
+}
+
+proc ${board}_spawn { board cmd } {
+    global board_info
+
+    set remote [board_info $board hostname]
+    set username [board_info $board username]
+    set RSH [board_info $board rsh_prog]
+
+    spawn $RSH -t -l $username $remote $cmd
+    set board_info($board,fileid) $spawn_id
+    return $spawn_id
+}
+
+proc ${board}_download { board src dest } {
+    global env board_type
+    global host_dir
+
+    if { [llength $dest] > 0 } {
+	set destfile [lindex $dest 0]
+    } else {
+	set destfile [file tail $src]
+    }

This doesn't feel right.
I realize /usr/share/dejagnu/remote.exp:remote_download
checks for whether the third parameter can be a list, but
remote_download is a varargs function (the name of the last
variable is "args") and is not a "board" function.
Other ${foo}_download functions that are actual "board" functions,
e.g., /usr/share/dejagnu/remote.exp:standard_download,
or gdb/testsuite/boards/remote-stdio-gdbserver.exp,
don't treat the third parameter as if it can be a list.

+
+    if { ![file exists $host_dir] } {
+	file mkdir $host_dir
+    }
+
+    set destfile [file join $host_dir $destfile]
+    file copy -force $src $destfile
+
+    return $destfile
+}

Re: [PATCH] Another board file for remote host

[Yao Qi]

Doug Evans <xdje42@gmail.com> writes:

> +global GDB
> +set GDB [file join [pwd] "../gdb"]
>
> Check if GDB has been set first, allow the user to
> pass this in from the command line.

GDB is always set in lib/gdb.exp and it is loaded earlier than the
board file.  In lib/gdb.exp:

if ![info exists GDB] {
    if ![is_remote host] {
	set GDB [findfile $base_dir/../../gdb/gdb "$base_dir/../../gdb/gdb" [transform gdb]]
    } else {
	set GDB [transform gdb]
    }
}

so we can't do the check here.

> +# The directory to copy files to.  In default, we choose ./remote-host, to
> +# avoiding messing up your HOME.  You can choose other directory as
> +# you like.
> +set host_dir [file join [pwd] "remote-host"]
>
> How about allowing the user to pass this in from the command line?
> By convention such variables are then uppercase.
>

That is fine to me.  In the updated patch, user can specify the
directory in command line via variable HOST_DIR.

> +
> +proc ${board}_download { board src dest } {
> +    global env board_type
> +    global host_dir
> +
> +    if { [llength $dest] > 0 } {
> +	set destfile [lindex $dest 0]
> +    } else {
> +	set destfile [file tail $src]
> +    }
>
> This doesn't feel right.
> I realize /usr/share/dejagnu/remote.exp:remote_download
> checks for whether the third parameter can be a list, but
> remote_download is a varargs function (the name of the last
> variable is "args") and is not a "board" function.
> Other ${foo}_download functions that are actual "board" functions,
> e.g., /usr/share/dejagnu/remote.exp:standard_download,
> or gdb/testsuite/boards/remote-stdio-gdbserver.exp,
> don't treat the third parameter as if it can be a list.

Yes, remote_download acts as a wrapper or an adapter to "board"
functions which are defined in board files.  In tcl, simple string is
also list, see,

% llength "foo"
1
% lindex "foo" 0     
foo

The code above is right to me but unnecessary.  Use dest as a string in
the updated patch.

-- 
Yao (齐尧)

Subject: [PATCH] Another board file for remote host

In the recent review to my patch about copying files to remote host,
we find that we need a board file which is more closely mapped real
remote host testing to improve coverage.  With the board file
local-remote-host-native.exp, DejaGNU copies files to
$build/gdb/testsuite/remote-host to emulate the effect of remote host.
Is it OK?

gdb/testsuite:

2014-09-04  Yao Qi  <yao@codesourcery.com>

	* boards/local-remote-host-native.exp: New file.

diff --git a/gdb/testsuite/boards/local-remote-host-native.exp b/gdb/testsuite/boards/local-remote-host-native.exp
new file mode 100644
index 0000000..1119c2d
--- /dev/null
+++ b/gdb/testsuite/boards/local-remote-host-native.exp
@@ -0,0 +1,80 @@
+# Copyright 2014 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
+# against local host, in remote host mode.
+#
+# This board file is used to emulate the real remote host testing, in
+# which file system of host and build are not shared.  This is achieved
+# by copying files from source directory to ${host_dir}.
+#
+# To use this file:
+# bash$ cd ${build_dir}/gdb
+# bash$ make check RUNTESTFLAGS="--host_board=local-remote-host-native --target_board=local-remote-host-native HOST_DIR=/tmp/foo/"
+#
+# We set both target board and host board together to test a native gdb
+# (host == target) on a remote host (host != build).  $HOST_DIR is the
+# directory for copying files to, to avoid messing up your HOME.  When
+# it is absent, files are copied to ./remote-host.
+
+if { $board_type == "target" } {
+    set_board_info compiler gcc
+}
+
+global GDB
+set GDB [file join [pwd] "../gdb"]
+
+set_board_info hostname 127.0.0.1
+
+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"
+
+if { ![info exists HOST_DIR] } {
+    set HOST_DIR [file join [pwd] "remote-host"]
+}
+
+if { $board_type == "host" } {
+    set_board_info gdb_opts "-d \"${HOST_DIR}\""
+}
+
+proc ${board}_spawn { board cmd } {
+    global board_info
+
+    set remote [board_info $board hostname]
+    set username [board_info $board username]
+    set RSH [board_info $board rsh_prog]
+
+    spawn $RSH -t -l $username $remote $cmd
+    set board_info($board,fileid) $spawn_id
+    return $spawn_id
+}
+
+proc ${board}_download { board src dest } {
+    global HOST_DIR
+
+    if { ![file exists $HOST_DIR] } {
+	file mkdir $HOST_DIR
+    }
+
+    set destfile [file join $HOST_DIR $dest]
+    file copy -force $src $destfile
+
+    return $destfile
+}

Re: [PATCH] Another board file for remote host

[Yao Qi]

Yao Qi <yao@codesourcery.com> writes:

>> +global GDB
>> +set GDB [file join [pwd] "../gdb"]
>>
>> Check if GDB has been set first, allow the user to
>> pass this in from the command line.
>
> GDB is always set in lib/gdb.exp and it is loaded earlier than the
> board file.  In lib/gdb.exp:
>
> if ![info exists GDB] {
>     if ![is_remote host] {
> 	set GDB [findfile $base_dir/../../gdb/gdb "$base_dir/../../gdb/gdb" [transform gdb]]
>     } else {
> 	set GDB [transform gdb]
>     }
> }
>
> so we can't do the check here.
>
>> +# The directory to copy files to.  In default, we choose ./remote-host, to
>> +# avoiding messing up your HOME.  You can choose other directory as
>> +# you like.
>> +set host_dir [file join [pwd] "remote-host"]
>>
>> How about allowing the user to pass this in from the command line?
>> By convention such variables are then uppercase.
>>
>
> That is fine to me.  In the updated patch, user can specify the
> directory in command line via variable HOST_DIR.
>
>> +
>> +proc ${board}_download { board src dest } {
>> +    global env board_type
>> +    global host_dir
>> +
>> +    if { [llength $dest] > 0 } {
>> +	set destfile [lindex $dest 0]
>> +    } else {
>> +	set destfile [file tail $src]
>> +    }
>>
>> This doesn't feel right.
>> I realize /usr/share/dejagnu/remote.exp:remote_download
>> checks for whether the third parameter can be a list, but
>> remote_download is a varargs function (the name of the last
>> variable is "args") and is not a "board" function.
>> Other ${foo}_download functions that are actual "board" functions,
>> e.g., /usr/share/dejagnu/remote.exp:standard_download,
>> or gdb/testsuite/boards/remote-stdio-gdbserver.exp,
>> don't treat the third parameter as if it can be a list.
>
> Yes, remote_download acts as a wrapper or an adapter to "board"
> functions which are defined in board files.  In tcl, simple string is
> also list, see,
>
> % llength "foo"
> 1
> % lindex "foo" 0     
> foo
>
> The code above is right to me but unnecessary.  Use dest as a string in
> the updated patch.

Hi, Doug,
How do you think of the updated patch?
https://sourceware.org/ml/gdb-patches/2014-09/msg00117.html

-- 
Yao (齐尧)

Re: [PATCH] Another board file for remote host

[Doug Evans]

Actually I've been spending more time with this today, I know I owe
you a response.
I have an emacs buffer with a reply in progress.
Good timing ... :-)

On Sun, Sep 14, 2014 at 9:41 PM, Yao Qi <yao@codesourcery.com> wrote:
> Yao Qi <yao@codesourcery.com> writes:
>
>>> +global GDB
>>> +set GDB [file join [pwd] "../gdb"]
>>>
>>> Check if GDB has been set first, allow the user to
>>> pass this in from the command line.
>>
>> GDB is always set in lib/gdb.exp and it is loaded earlier than the
>> board file.  In lib/gdb.exp:
>>
>> if ![info exists GDB] {
>>     if ![is_remote host] {
>>       set GDB [findfile $base_dir/../../gdb/gdb "$base_dir/../../gdb/gdb" [transform gdb]]
>>     } else {
>>       set GDB [transform gdb]
>>     }
>> }
>>
>> so we can't do the check here.
>>
>>> +# The directory to copy files to.  In default, we choose ./remote-host, to
>>> +# avoiding messing up your HOME.  You can choose other directory as
>>> +# you like.
>>> +set host_dir [file join [pwd] "remote-host"]
>>>
>>> How about allowing the user to pass this in from the command line?
>>> By convention such variables are then uppercase.
>>>
>>
>> That is fine to me.  In the updated patch, user can specify the
>> directory in command line via variable HOST_DIR.
>>
>>> +
>>> +proc ${board}_download { board src dest } {
>>> +    global env board_type
>>> +    global host_dir
>>> +
>>> +    if { [llength $dest] > 0 } {
>>> +    set destfile [lindex $dest 0]
>>> +    } else {
>>> +    set destfile [file tail $src]
>>> +    }
>>>
>>> This doesn't feel right.
>>> I realize /usr/share/dejagnu/remote.exp:remote_download
>>> checks for whether the third parameter can be a list, but
>>> remote_download is a varargs function (the name of the last
>>> variable is "args") and is not a "board" function.
>>> Other ${foo}_download functions that are actual "board" functions,
>>> e.g., /usr/share/dejagnu/remote.exp:standard_download,
>>> or gdb/testsuite/boards/remote-stdio-gdbserver.exp,
>>> don't treat the third parameter as if it can be a list.
>>
>> Yes, remote_download acts as a wrapper or an adapter to "board"
>> functions which are defined in board files.  In tcl, simple string is
>> also list, see,
>>
>> % llength "foo"
>> 1
>> % lindex "foo" 0
>> foo
>>
>> The code above is right to me but unnecessary.  Use dest as a string in
>> the updated patch.
>
> Hi, Doug,
> How do you think of the updated patch?
> https://sourceware.org/ml/gdb-patches/2014-09/msg00117.html
>
> --
> Yao (齐尧)

Re: [PATCH] Another board file for remote host

[Doug Evans]

Yao Qi <yao@codesourcery.com> writes:
> Doug Evans <xdje42@gmail.com> writes:
>
>> +global GDB
>> +set GDB [file join [pwd] "../gdb"]
>>
>> Check if GDB has been set first, allow the user to
>> pass this in from the command line.
>
> GDB is always set in lib/gdb.exp and it is loaded earlier than the
> board file.  In lib/gdb.exp:
>
> if ![info exists GDB] {
>     if ![is_remote host] {
> 	set GDB [findfile $base_dir/../../gdb/gdb "$base_dir/../../gdb/gdb" [transform gdb]]
>     } else {
> 	set GDB [transform gdb]
>     }
> }
>
> so we can't do the check here.

I deleted the setting of GDB in this file and was able to
run tests by passing GDB=/usr/bin/gdb on the command line.

I then tried not passing GDB= on the command line and I see /usr/bin/gdb
still being run (it's invoked as "gdb" without any path).  Bleah.

It would really be nice to make this work, but it's not critical
for this first checkin.
OTOH, I think this is way too subtle an issue and requires a comment.
Something like:

+# We have to explicitly specify GDB with the path to the copy in
+# in the build directory because otherwise it will be set to the
+# result of "transform GDB" since the harness thinks we're using
+# a remote host.  See lib/gdb.exp.
+set GDB [file join [pwd] "../gdb"]
+verbose -log "Overriding setting of GDB to $GDB"

There's no need for "global GDB" here.

>> +# The directory to copy files to.  In default, we choose ./remote-host, to
>> +# avoiding messing up your HOME.  You can choose other directory as
>> +# you like.
>> +set host_dir [file join [pwd] "remote-host"]
>>
>> How about allowing the user to pass this in from the command line?
>> By convention such variables are then uppercase.
>>
>
> That is fine to me.  In the updated patch, user can specify the
> directory in command line via variable HOST_DIR.
>
>> +
>> +proc ${board}_download { board src dest } {
>> +    global env board_type
>> +    global host_dir
>> +
>> +    if { [llength $dest] > 0 } {
>> +	set destfile [lindex $dest 0]
>> +    } else {
>> +	set destfile [file tail $src]
>> +    }
>>
>> This doesn't feel right.
>> I realize /usr/share/dejagnu/remote.exp:remote_download
>> checks for whether the third parameter can be a list, but
>> remote_download is a varargs function (the name of the last
>> variable is "args") and is not a "board" function.
>> Other ${foo}_download functions that are actual "board" functions,
>> e.g., /usr/share/dejagnu/remote.exp:standard_download,
>> or gdb/testsuite/boards/remote-stdio-gdbserver.exp,
>> don't treat the third parameter as if it can be a list.
>
> Yes, remote_download acts as a wrapper or an adapter to "board"
> functions which are defined in board files.  In tcl, simple string is
> also list, see,
>
> % llength "foo"
> 1
> % lindex "foo" 0     
> foo
>
> The code above is right to me but unnecessary.  Use dest as a string in
> the updated patch.

Besides the issue of setting GDB I have one more nit.
I found this useful while diagnosing failures:

 proc ${board}_download { board src dest } {
     global HOST_DIR
 
     if { ![file exists $HOST_DIR] } {
 	file mkdir $HOST_DIR
     }
 
     set destfile [file join $HOST_DIR $dest]
+    verbose -log "${board}_download: file copy -force $src $destfile"
     file copy -force $src $destfile
 
     return $destfile
 }

With those two issues addressed I'm happy.
[I'm sure more issues will arise as I'm seeing failures
I wouldn't have expected.  I didn't spend much time on them
and at any rate I think we can get this checked in so that
we can start addressing them collectively.]

Re: [PATCH] Another board file for remote host

[Yao Qi]

Doug Evans <xdje42@gmail.com> writes:

> With those two issues addressed I'm happy.

They are fixed in the updated patch below.  I'll commit it tomorrow if
no other objections.

> [I'm sure more issues will arise as I'm seeing failures
> I wouldn't have expected.  I didn't spend much time on them
> and at any rate I think we can get this checked in so that
> we can start addressing them collectively.]

That sounds good to me too.

-- 
Yao (齐尧)

From: Yao Qi <yao@codesourcery.com>
Date: Thu, 28 Aug 2014 21:27:40 +0800
Subject: [PATCH] Another board file for remote host

In the recent review to my patch about copying files to remote host,
we find that we need a board file which is more closely mapped real
remote host testing to improve coverage.  With the board file
local-remote-host-native.exp, DejaGNU copies files to
$build/gdb/testsuite/remote-host to emulate the effect of remote host.
Is it OK?

gdb/testsuite:

2014-09-15  Yao Qi  <yao@codesourcery.com>

	* boards/local-remote-host-native.exp: New file.

diff --git a/gdb/testsuite/boards/local-remote-host-native.exp b/gdb/testsuite/boards/local-remote-host-native.exp
new file mode 100644
index 0000000..69d40d9
--- /dev/null
+++ b/gdb/testsuite/boards/local-remote-host-native.exp
@@ -0,0 +1,85 @@
+# Copyright 2014 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
+# against local host, in remote host mode.
+#
+# This board file is used to emulate the real remote host testing, in
+# which file system of host and build are not shared.  This is achieved
+# by copying files from source directory to ${host_dir}.
+#
+# To use this file:
+# bash$ cd ${build_dir}/gdb
+# bash$ make check RUNTESTFLAGS="--host_board=local-remote-host-native --target_board=local-remote-host-native HOST_DIR=/tmp/foo/"
+#
+# We set both target board and host board together to test a native gdb
+# (host == target) on a remote host (host != build).  $HOST_DIR is the
+# directory for copying files to, to avoid messing up your HOME.  When
+# it is absent, files are copied to ./remote-host.
+
+if { $board_type == "target" } {
+    set_board_info compiler gcc
+}
+
+# We have to explicitly specify GDB with the path to the copy in
+# the build directory because otherwise it will be set to the
+# result of "transform GDB" since the harness thinks we're using
+# a remote host.  See lib/gdb.exp.
+set GDB [file join [pwd] "../gdb"]
+verbose -log "Overriding setting of GDB to $GDB"
+
+set_board_info hostname 127.0.0.1
+
+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"
+
+if { ![info exists HOST_DIR] } {
+    set HOST_DIR [file join [pwd] "remote-host"]
+}
+
+if { $board_type == "host" } {
+    set_board_info gdb_opts "-d \"${HOST_DIR}\""
+}
+
+proc ${board}_spawn { board cmd } {
+    global board_info
+
+    set remote [board_info $board hostname]
+    set username [board_info $board username]
+    set RSH [board_info $board rsh_prog]
+
+    spawn $RSH -t -l $username $remote $cmd
+    set board_info($board,fileid) $spawn_id
+    return $spawn_id
+}
+
+proc ${board}_download { board src dest } {
+    global HOST_DIR
+
+    if { ![file exists $HOST_DIR] } {
+	file mkdir $HOST_DIR
+    }
+
+    set destfile [file join $HOST_DIR $dest]
+    verbose -log "${board}_download: file copy -force $src $destfile"
+    file copy -force $src $destfile
+
+    return $destfile
+}