On Sun, Mar 1, 2015 at 5:36 AM, Kyle J. McKay <mackyle@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html