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

From: Shawn O. Pearce <spearce@spearce.org>
Date: Dec 1, 2009, 19:22 UTC
Message-ID: <20091201192233.GL21299@spearce.org>
In-Reply-To: <1259675838-14692-5-git-send-email-ilari.liusvaara@elisanet.fi>

Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:

Show 12 quoted lines

> 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.

Show 9 quoted lines

> +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.