Re: [RFC/PATCH 2/5] upload-pack: support out of band client capability requests

[Duy Nguyen <pclouds@gmail.com>]

On Sat, Feb 28, 2015 at 2:47 PM, Kyle J. McKay <mackyle@gmail.com> wrote:
> On Feb 27, 2015, at 17:01, Stefan Beller wrote:
>>
>> From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>>
>> The only difference from the original protocol client capabilities are
>> negotiated before initial refs advertisment.
>>
>> Client capabilities are sent out of band (upload-pack receives it as
>> the second command line argument). The server sends one pkt-line back
>> advertising its capabilities.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>>
>> Notes:
>>    v1:
>>    I am still undecided if the client should then accept/resend
>>    the capabilities to confirm them, which would make the client the
>>    ultimate decider which capabilities are used.
>>
>>    My gut feeling is to rather let the server make the final decision
>>    for the capabilities, as it will use some requested capabilities
>>    already to not send out all the refs.
>>
>> Documentation/git-upload-pack.txt | 10 +++++++++-
>> upload-pack.c                     | 42
>> +++++++++++++++++++++++++++------------
>> 2 files changed, 38 insertions(+), 14 deletions(-)
>>
>> diff --git a/Documentation/git-upload-pack.txt
>> b/Documentation/git-upload-pack.txt
>> index 0abc806..ad3a89d 100644
>> --- a/Documentation/git-upload-pack.txt
>> +++ b/Documentation/git-upload-pack.txt
>> @@ -9,7 +9,7 @@ git-upload-pack - Send objects packed back to
>> git-fetch-pack
>> SYNOPSIS
>> --------
>> [verse]
>> -'git-upload-pack' [--strict] [--timeout=<n>] <directory>
>> +'git-upload-pack' [--strict] [--timeout=<n>] <directory> [<capabilities>]
>
>
> Isn't the problem with this that passing the extra argument to ssh servers
> will cause them to fail?
>
> Having just looked at the upload-pack.c source it looks to me like trying to
> send "git-upload-pack 'dir' 'capabilities'" to an ssh git server running a
> current version of the code will just end up failing.  I realize the extra
> argument is optional, so does that mean there's no out-of-band support for
> ssh connections since the extra argument would have to be omitted to remain
> compatible?

The client should only trigger this behavior when it knows the server
can deal with it. And that is possible because in the last fetch, the
server has told the client that it's capable of receiving this
capabilities argument. Backward compatibility is a concern at client
side, not server side.

> I've looked at those links and it's unclear to me how they support an
> out-of-band option for ssh, they seem to be targeted at git-daemon.  Maybe
> there's another reference?

For ssh, I think connect.c is the one that constructs and executes ssh command.
-- 
Duy