Re: [PATCH 1/4] ssh.exp: implement ssh_spawn

On 6/17/26 17:07, Joseph Myers wrote:
> This allows tests using remote_spawn to work with ssh.exp (rather than
> calling rsh with standard_spawn otherwise does by default).
>
> ---
>
> On Tue, 2 Jun 2026, Jacob Bachmeyer wrote:
>
>> The big problem I have here is an extra use of sh.  What prevents using Tcl
>> list operations and {eval [list spawn $SSH] $ssh_useropts [list
>> "$ssh_user$hostname" $commandline]} to move the argument splitting into Tcl
>> and avoid an extra shell with all of its potential quoting pitfalls?
>>
>> To use list operations here, initialize ssh_useropts to either [board_info
>> $dest ssh_opts] or an empty list, then use lappend(n) to add to it.  This may
>> also obviate the need for quoting ControlPath.
>>
>> The main reason I do not want an extra shell used here is that it will break
>> if $commandline ever contains single quotes.  It also wastes a process table
>> slot, but that is a minor issue on modern systems and could be fixed by using
>> exec(1) in the shell.
> This version constructs the command to run as a list (including $SSH in
> case that is actually a command plus arguments).
>
> Maybe "spawn {*}$cmd" would be better than "eval spawn $cmd", but I don't
> see any uses of {*} in DejaGnu.

I specifically asked for eval because DejaGnu still keeps compatibility 
with Tcl 8.4 and "{*}" was introduced in Tcl 8.5.  I have yet to see a 
use for "{*}" that would justify raising the minimum Tcl version, 
instead of simply using eval.

(Yes, I know that Tcl 8.4 is ancient, but DejaGnu is a framework:  
raising a version requirement for DejaGnu itself affects every other 
package using DejaGnu, so I take an extremely conservative view on the 
matter.)

>
> ---
>   lib/ssh.exp | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 59 insertions(+), 1 deletion(-)
>
> diff --git a/lib/ssh.exp b/lib/ssh.exp
> index ade5500..b0f0696 100644
> --- a/lib/ssh.exp
> +++ b/lib/ssh.exp
> @@ -195,7 +195,7 @@ proc ssh_exec { boardname program pargs inp outp } {
>   }
>   
>   proc ssh_close { desthost } {
> -    global SSH ssh_initialized
> +    global SSH ssh_initialized board_info
>   
>       verbose "Closing the SSH connection to $desthost"
>   
> @@ -233,6 +233,64 @@ proc ssh_close { desthost } {
>      # Kill the remote server
>       set status [catch "exec ssh $ssh_port -o ControlPath=/tmp/ssh-%r@%h:%p -O exit $args"]
>       set ssh_initialized "no"
> +    if {[board_info $desthost exists fileid]} {
> +	set spawn_id [board_info $desthost fileid]
> +	unset board_info($desthost,fileid)
> +	catch "close -i $spawn_id"
> +	catch "wait -i $spawn_id"
> +    }
>   
>        return ""
>   }
> +
> +proc ssh_spawn { dest commandline } {
> +    global SSH timeout board_info
> +
> +    set ssh_port ""
> +    set scp_port ""
> +    set ssh_user ""
> +    set name ""
> +    set hostname ""
> +
> +    verbose "Spawning on $dest: $commandline"
> +
> +    if {![board_info $dest exists ssh_prog]} {
> +	set SSH ssh
> +    } else {
> +	set SSH [board_info $dest ssh_prog]
> +    }
> +
> +    if {[board_info $dest exists username]} {
> +	set ssh_user "[board_info $dest username]@"
> +    } else {
> +	set ssh_user ""
> +    }
> +
> +    set cmd $SSH
> +
> +    if {[board_info $dest exists ssh_opts]} {
> +	set cmd [concat $cmd [board_info $dest ssh_opts]]
> +    }
> +
> +    if {[board_info $dest exists name]} {
> +	set dest [board_info $dest name]
> +    }
> +
> +    if {[board_info $dest exists hostname]} {
> +	set hostname [board_info $dest hostname]
> +    } else {
> +	set hostname $dest
> +    }
> +
> +    if {[board_info $dest exists port]} {
> +	lappend cmd -p [board_info $dest port]
> +    }
> +
> +    lappend cmd -t -t -o ControlPersist=yes -o ControlMaster=auto -o "ControlPath=/tmp/ssh-%r@%h:%p"
> +    lappend cmd "$ssh_user$hostname" "$commandline"
> +
> +    eval spawn $cmd
> +
> +    set board_info($dest,fileid) $spawn_id
> +    return $spawn_id
> +}

Applied; thanks.


-- Jacob