Re: [PATCH 3/5] ssh: 'auto' variant to select between 'ssh' and 'simple'

From: Stefan Beller <sbeller@google.com>
Date: Oct 23, 2017, 22:33 UTC
Message-ID: <CAGZ79kZTjUvcq_hKHCqTDoaBxt2x+9XcqYc6ao1bhcET2SM-PQ@mail.gmail.com>
In-Reply-To: <20171023213159.eitrjrqrh277advm@aiede.mtv.corp.google.com>

On Mon, Oct 23, 2017 at 2:31 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 13 quoted lines

>  1. First, check whether $GIT_SSH supports OpenSSH options by running
>
>         $GIT_SSH -G <options> <host>
>
>     This returns status 0 and prints configuration in OpenSSH if it
>     recognizes all <options> and returns status 255 if it encounters
>     an unrecognized option.  A wrapper script like
>
>         exec ssh -- "$@"
>
>     would fail with
>
>         ssh: Could not resolve hostname -g: Name or service not known

capital -G?

> -       if (variant == VARIANT_TORTOISEPLINK)
> -               argv_array_push(&conn->args, "-batch");
> +               detect.use_shell = conn->use_shell;

Why do we have to use a shell for evaluation of this test balloon?

> +               detect.no_stdin = detect.no_stdout = detect.no_stderr = 1;

okay.

...

> +               argv_array_push(&detect.args, "-G");

...

> +               variant = run_command(&detect) ? VARIANT_SIMPLE : VARIANT_SSH;

What if we have a VARIANT_SIMPLE, that doesn't care about -G but just connects to the remote host (keeping the session open), do we need to kill it after some time to have run_command return eventually?

Or can we give a command to be executed remotely (e.g. 'false') that we know returns quickly?

Show 5 quoted lines

>  '
>
> +test_expect_success 'OpenSSH-like uplink is treated as ssh' '
> +       write_script "$TRASH_DIRECTORY/uplink" <<-EOF &&
> +       if test "\$1" = "-G"

Reading this test (and the commit message) I realize, we do care about the order of options, so this is fine.