All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 5/6] fetch: teach independent negotiation (no packfile)
Date: Thu, 08 Apr 2021 22:41:53 -0700	[thread overview]
Message-ID: <xmqqczv4nhem.fsf@gitster.g> (raw)
In-Reply-To: <4696c8e901808853d17af10d5a6d95cd4711c6d5.1617929278.git.jonathantanmy@google.com> (Jonathan Tan's message of "Thu, 8 Apr 2021 18:10:02 -0700")

Jonathan Tan <jonathantanmy@google.com> writes:

> There are 2 code paths that do not go through fetch_refs_via_pack() that
> needed to be individually excluded: the bundle transport (excluded
> through requiring smart_options, which the bundle transport doesn't
> support) and transport helpers that do not support takeover.
> Fortunately, none of these support protocol v2.

I am a bit puzzled by this mention of "Fortunately".  If one says
"this shiny new feature only works with protocol v2" and "transport
X does not support protocol v2", doesn't it imply that the shiny new
feature cannot be used with the transport X, which is unfortunate?

I can understand "while interacting with the bundle transport, you
cannot do independent negotiation, but there is nothing to negotiate
with a static file that is a bundle anyway, so nothing is lost" as
an explanation, though.

>  Documentation/technical/protocol-v2.txt |  8 +++
>  builtin/fetch.c                         | 27 +++++++-
>  fetch-pack.c                            | 89 +++++++++++++++++++++++--
>  fetch-pack.h                            | 11 +++
>  object.h                                |  2 +-
>  t/t5701-git-serve.sh                    |  2 +-
>  t/t5702-protocol-v2.sh                  | 89 +++++++++++++++++++++++++
>  transport-helper.c                      | 10 +++
>  transport.c                             | 30 +++++++--
>  transport.h                             |  6 ++
>  upload-pack.c                           | 18 +++--
>  11 files changed, 275 insertions(+), 17 deletions(-)

It is a bit surprising that there isn't much code removed, as I
expected that we'd be factoring out and reusing existing code used
in negotiation for fetching into a new helper function (hence the
existing codepath would lose a lot of code to be replaced by a call
to a new helper function), but that is apparently not what is going
on.

I'll have to revisit this step and the next step tomorrow.

Thanks.

  reply	other threads:[~2021-04-09  5:42 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-09  1:09 [PATCH 0/6] Push negotiation Jonathan Tan
2021-04-09  1:09 ` [PATCH 1/6] fetch-pack: buffer object-format with other args Jonathan Tan
2021-04-09  4:49   ` Junio C Hamano
2021-04-09 16:24     ` Jonathan Tan
2021-04-09  1:09 ` [PATCH 2/6] fetch-pack: refactor process_acks() Jonathan Tan
2021-04-09  5:08   ` Junio C Hamano
2021-05-03 16:30   ` Derrick Stolee
2021-04-09  1:10 ` [PATCH 3/6] fetch-pack: refactor add_haves() Jonathan Tan
2021-04-09  5:20   ` Junio C Hamano
2021-04-09  1:10 ` [PATCH 4/6] fetch-pack: refactor command and capability write Jonathan Tan
2021-04-09  5:27   ` Junio C Hamano
2021-04-09  1:10 ` [PATCH 5/6] fetch: teach independent negotiation (no packfile) Jonathan Tan
2021-04-09  5:41   ` Junio C Hamano [this message]
2021-04-09 16:38     ` Jonathan Tan
2021-05-03 15:25   ` Derrick Stolee
2021-05-03 15:40     ` Derrick Stolee
2021-05-03 21:52     ` Jonathan Tan
2021-04-09  1:10 ` [PATCH 6/6] send-pack: support push negotiation Jonathan Tan
2021-05-03 15:35   ` Derrick Stolee
2021-05-03 22:02     ` Jonathan Tan
2021-05-04 17:26       ` Derrick Stolee
2021-04-30  5:42 ` [PATCH 0/6] Push negotiation Junio C Hamano
2021-04-30 17:33   ` Derrick Stolee
2021-05-04 21:15 ` [PATCH v2 0/5] " Jonathan Tan
2021-05-04 21:15   ` [PATCH v2 1/5] fetch-pack: refactor process_acks() Jonathan Tan
2021-05-04 21:15   ` [PATCH v2 2/5] fetch-pack: refactor add_haves() Jonathan Tan
2021-05-04 21:16   ` [PATCH v2 3/5] fetch-pack: refactor command and capability write Jonathan Tan
2021-05-04 21:16   ` [PATCH v2 4/5] fetch: teach independent negotiation (no packfile) Jonathan Tan
2021-05-05  1:53     ` Junio C Hamano
2021-05-05 16:42       ` Derrick Stolee
2021-05-06  2:12         ` Junio C Hamano
2021-05-05 16:44     ` Derrick Stolee
2021-05-04 21:16   ` [PATCH v2 5/5] send-pack: support push negotiation Jonathan Tan

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=xmqqczv4nhem.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.