Re: [RFC PATCH 4/8] Support remote helpers implementing smart transports

["Shawn O. Pearce" <spearce@spearce.org>]

Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:
> diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
> index 5cfdc0c..adf815c 100644
> --- a/Documentation/git-remote-helpers.txt
> +++ b/Documentation/git-remote-helpers.txt
> @@ -90,6 +90,28 @@ Supported if the helper has the "push" capability.
>  +
>  Supported if the helper has the "import" capability.
>  
> +'connect-r' <service>::
> +	Connects to given service. Stdin and stdout of helper are
> +	connected to specified service (no git or git- prefixes are used,
> +	so e.g. fetching uses 'upload-pack' as service) on remote side.

This flies against every other convention we have.  git:// uses the
string 'git-upload-pack' and 'git-receive-pack', and so does the
smart-http code.  We should continue to use the git- prefix here,
to be consistent, even though by context its clearly implied.

> +	Valid replies to this command are 'OK' (connection established),

Why 'OK'?  Currently remote-helpers return an empty blank line
to any successful command, not 'OK'.

> +	'FALLBACK' (no smart transport support, fall back to dumb
> +	transports) and 'ERROR' (can't connect, don't bother trying to
> +	fall back).

FALLBACK almost makes sense, but ERROR we don't do in the
the existing helper protocol.  Instead the helper simply
prints its error message(s) to stderr and does exit(128).
aka what die() does.

> +Supported if the helper has the "connect-r" capability. Not used if
> +helper has the "invoke-r" capability, as invoke is preferred to connect.
> +
> +'invoke-r' <cmdlength> <cmd>::
> +	Like connect-r command, but instead of service name, command
> +	line is given. The length of command field is given in command
> +	length field.
> ++
> +Supported if the helper has the "invoke-r" capability.

Why both connect-r and invoke-r?  Why isn't connect-r sufficient
here?  Isn't it sufficient for any service that runs over git:// ?

-- 
Shawn.