On Tue, Sep 26, 2023 at 5:34 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Jiang Xin <worldhello.net@xxxxxxxxx> writes:
>
> > From: Jiang Xin <zhiyou.jx@xxxxxxxxxxxxxxx>
> >
> > After successfully connecting to the smart transport by calling
> > "process_connect_service()" in "connect_helper()", run "do_take_over()"
> > to replace the old vtable with a new one which has methods ready for
> > the smart transport connection.
>
> The existing pattern among all callers of process_connect() seems to
> be
>
> if (process_connect(...)) {
> do_take_over();
> ... dispatch to the underlying method ...
> }
> ... otherwise implement the fallback ...
>
> where the return value from process_connect() is the return value of
> the call it makes to process_connect_service().
>
> And the only other caller of process_connect_service() is
> connect_helper(), so in that sense, making a call to do_take_over()
> when process_connect_service() succeeds in the helper does make
> things consistent. The connect_helper() function being static, the
> helper transport is the only transport that gets affected, but how
> has it been working without having this do_take_over() step? An
> obvious related question is if it has been working so far, would it
> break if we have do_take_over() added here?
The connect_helper() function is used as the connect method of the
vtable in "transport-helper.c", and we use the function
"transport_connect()" in "transport.c" to call this connect method of
vtable. The only place that we call transport_connect() to setup a
connection is in "builtin/archive.c". So it won't break others if we
add do_take_over() in connect_helper().
In fact, it was not "git archive" that made me discover this issue.
When I implemented a fetch proxy and added a new caller for
transport_connect(), I found that the HTTP protocol didn't work, so I
dug it out.
