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

[Duy Nguyen <pclouds@gmail.com>]

On Sun, Mar 1, 2015 at 5:36 AM, Kyle J. McKay <mackyle@gmail.com> wrote:
>
> On Feb 28, 2015, at 03:22, Duy Nguyen wrote:
>>
>> 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.
>
>
> This I assume you're referring to this change in connect.c from [1]:
>
> @@ -729,6 +734,8 @@ struct child_process *git_connect(int fd[2], const char
> *url,
>                         conn->use_shell = 1;
>                 }
>                 argv_array_push(&argv, cmd.buf);
> +               if (service_flags)
> +                       argv_array_push(&argv, service_flags);
>                 conn->argv = argv.argv;
>                 if (start_command(conn))
>                         die("unable to fork");
>
> That's not going to work for ssh servers running a stock git-shell and I
> haven't seen any updates to shell.c to match.  git-shell does not allow
> anything other than one argument to be passed to
> git-upload-pack/git-receive-pack.
>
> When shell.c calls do_generic_cmd and it calls sq_dequote on its argument
> that contains "'dir' 'service-flags'" it's going to return NULL and shell.c
> will die("bad argument").  So I don't see how this supports ssh as-is even
> if you know in advance the server supports the new protocol.  I don't see
> any changes to shell.c in that uploadpack2 branch nor in this patch series.

You're right. If we continue to pass client capabilities as an extra
argument, then git-shell needs updates.
-- 
Duy