From: Junio C Hamano <gitster@pobox•com>
To: Stefan Beller <sbeller@google•com>
Cc: "git\@vger.kernel.org" <git@vger•kernel.org>,
Duy Nguyen <pclouds@gmail•com>, Jeff King <peff@peff•net>
Subject: Re: [RFCv2 10/16] transport: connect_setup appends protocol version number
Date: Tue, 02 Jun 2015 15:27:44 -0700 [thread overview]
Message-ID: <xmqqa8whzqxr.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CAGZ79kbnX_kyuvj73PGcO7OBOj7CfdouARrqNWEkCnUfdN=DqQ@mail.gmail.com> (Stefan Beller's message of "Tue, 2 Jun 2015 15:09:23 -0700")
Stefan Beller <sbeller@google•com> writes:
>> 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.
Hmph, but then everybody would know that it is now interacting with
upload-pack-2; I think it probably is a better way to go.
In any case, all codepaths other than what actually runs exec()
should not be basing their decision on the program names---that is
what you added transport_version field for, and they should look
at that field if they want to switch behaviour between protocol
versions anyway, so I think we can live with either way.
>> 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.
Hmm, that's an unfortunate interaction. If you wrote a new protocol
driver that talks v2, it may be natural to say
git fetch --upload-pack=a.out --transport-version=2
when you want to test it, and we do not want to invoke a.out-2 in
that case.
> 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.
At least shouldn't you be able to tell if we are using compiled-in
default or user specified value (whether configuration/command line)?
Does the code that initialise data->options.{upload,receive}pack
fields with the compiled-in default values know what protocol
version is going to be used at that point? I think that is where
the appending of "-2" should be done; in other words, I think
addition of "-2" should be done _ONLY_ for compiled-in default
values, and it should be done not just for exec() but should be
visible to everybody, including those who are debugging and inspect
data->options.uploadpack field. What we spawn must match what is
stored there.
next prev parent reply other threads:[~2015-06-02 22:27 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-02 0:02 [RFCv2 00/16] Protocol version 2 Stefan Beller
2015-06-02 0:02 ` [RFCv2 01/16] stringlist: add from_space_separated_string Stefan Beller
2015-06-02 9:42 ` Duy Nguyen
2015-06-02 15:10 ` Eric Sunshine
2015-06-02 17:54 ` Stefan Beller
2015-06-02 0:02 ` [RFCv2 02/16] upload-pack: make client capability parsing code a separate function Stefan Beller
2015-06-02 0:02 ` [RFCv2 03/16] connect: rewrite feature parsing to work on string_list Stefan Beller
2015-06-02 18:48 ` Junio C Hamano
2015-06-02 0:02 ` [RFCv2 04/16] upload-pack-2: Implement the version 2 of upload-pack Stefan Beller
2015-06-02 18:59 ` Junio C Hamano
2015-06-02 23:08 ` Stefan Beller
2015-06-02 0:02 ` [RFCv2 05/16] remote.h: Change get_remote_heads return to void Stefan Beller
2015-06-02 21:17 ` Junio C Hamano
2015-06-02 21:25 ` Stefan Beller
2015-06-02 21:41 ` Junio C Hamano
2015-06-02 0:02 ` [RFCv2 06/16] remote.h: add new struct for options Stefan Beller
2015-06-02 21:18 ` Junio C Hamano
2015-06-02 21:40 ` Stefan Beller
2015-06-02 21:43 ` Junio C Hamano
2015-06-02 0:02 ` [RFCv2 07/16] transport: add infrastructure to support a protocol version number Stefan Beller
2015-06-02 0:02 ` [RFCv2 08/16] transport: select transport version via command line or config Stefan Beller
2015-06-02 0:02 ` [RFCv2 09/16] remote.h: add get_remote_capabilities, request_capabilities Stefan Beller
2015-06-02 21:24 ` Junio C Hamano
2015-06-02 0:02 ` [RFCv2 10/16] transport: connect_setup appends protocol version number Stefan Beller
2015-06-02 9:58 ` Duy Nguyen
2015-06-02 18:04 ` Stefan Beller
2015-06-02 21:37 ` Junio C Hamano
2015-06-02 22:09 ` Stefan Beller
2015-06-02 22:27 ` Junio C Hamano [this message]
2015-06-02 0:02 ` [RFCv2 11/16] remote: have preselect_capabilities Stefan Beller
2015-06-02 21:45 ` Junio C Hamano
2015-06-02 0:02 ` [RFCv2 12/16] transport: get_refs_via_connect exchanges capabilities before refs Stefan Beller
2015-06-02 21:55 ` Junio C Hamano
2015-06-02 22:19 ` Stefan Beller
2015-06-02 0:02 ` [RFCv2 13/16] fetch-pack: use the configured transport protocol Stefan Beller
2015-06-02 9:55 ` Duy Nguyen
2015-06-02 10:02 ` Duy Nguyen
2015-06-02 11:32 ` Ilari Liusvaara
2015-06-02 0:02 ` [RFCv2 14/16] t5544: add a test case for the new protocol Stefan Beller
2015-06-03 0:16 ` Eric Sunshine
2015-06-02 0:02 ` [RFCv2 15/16] Documentation/technical/pack-protocol: Mention http as possible protocol Stefan Beller
2015-06-02 21:57 ` Junio C Hamano
2015-06-02 22:00 ` Junio C Hamano
2015-06-02 0:02 ` [RFCv2 16/16] Document protocol version 2 Stefan Beller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqa8whzqxr.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=pclouds@gmail$(echo .)com \
--cc=peff@peff$(echo .)net \
--cc=sbeller@google$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox