On Tue, Jun 2, 2015 at 2:37 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
>> ---
>>
>> Notes:
>> name it to_free
>>
>> transport.c | 17 +++++++++++++++--
>> 1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/transport.c b/transport.c
>> index 651f0ac..b49fc60 100644
>> --- a/transport.c
>> +++ b/transport.c
>> @@ -496,15 +496,28 @@ static int set_git_option(struct git_transport_options *opts,
>> static int connect_setup(struct transport *transport, int for_push, int verbose)
>> {
>> struct git_transport_data *data = transport->data;
>> + const char *remote_program;
>> + char *to_free = 0;
>
> char *to_free = NULL;
>
>> + remote_program = (for_push ? data->options.receivepack
>> + : data->options.uploadpack);
>> +
>> + if (transport->smart_options->transport_version >= 2) {
>> + to_free = xmalloc(strlen(remote_program) + 12);
>> + sprintf(to_free, "%s-%d", remote_program,
>> + transport->smart_options->transport_version);
>> + remote_program = to_free;
>> + }
>
> Hmph, so everybody else thinks it is interacting with 'upload-pack',
> and this is the only function that knows it is actually talking with
> 'upload-pack-2'?
Yes.
>
> I am wondering why there isn't a separate helper function that
> munges data->options.{uploadpack,receivepack} fields based on
> the value of transport_version that is called _before_ this function
> is called.
That makes sense.
>
> Also, how does this interact with the name of the program the end
> user can specify via "fetch --upload-pack=<program name>" option?
You'd specify --upload-pack=foo-frotz and --transport-version=2
and it would look for foo-frotz-2 instead.
The problem IMHO is we have quite a few places where the
upload-pack binary path can be configured. Either as a command line
option or as a repository configuration.
And the way we're currently architecting the next protocol, the version
is encoded in the file name, which makes sense (an old binary will not
accept a new protocol), so what should happen when
* there is a repository configuration "upload-pack-custom" for upload-pack
for historic reasons. When just switching to a new version, you would need
to add a "upload-pack-custom-2" binary on the server side anyway
* additionally to the configured value you want to play around with the new
protocol, so would you rather just say "--transport-version=2" or also need
to have some sort of "--upload-pack=upload-pack-custom-another-path"
involved? It's easy to forget the second option I believe.
* the user specifies
"--upload-pack=custom-upload-pack-which-talks-version1" and
"--transport-version=2" together. This will fail, but at which stage do we
want to fail?
All these questions lead me to think it's maybe better to make the rest of Git
unaware of the added "-${version}" string and pretend we would be talking to
upload-pack instead.
--
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