git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Christian Couder <christian.couder@gmail.com>
Cc: git@vger.kernel.org,  John Cai <johncai86@gmail.com>,
	 Patrick Steinhardt <ps@pks.im>
Subject: Re: [PATCH v3 0/3] upload-pack: support a missing-action
Date: Fri, 24 May 2024 11:25:46 -0700	[thread overview]
Message-ID: <xmqq5xv3kqxh.fsf@gitster.g> (raw)
In-Reply-To: <20240524163926.2019648-1-christian.couder@gmail.com> (Christian Couder's message of "Fri, 24 May 2024 18:39:23 +0200")

Christian Couder <christian.couder@gmail.com> writes:

> The changes since v2 are the following:
> ...
>
> Thanks to Junio for his reviews of v1 and v3, and for suggesting the
> most of the above changes.
> ...

>
> Range diff between v2 and v3
> ============================
>
> (Might not be super useful as changes in patch 2/3 are not seen.)
>
> 1:  0a961dd4f5 = 1:  67c761b08a rev-list: refactor --missing=<missing-action>
> 2:  410acc6a39 < -:  ---------- pack-objects: use the missing action API
> -:  ---------- > 2:  7bf04f3096 pack-objects: use the missing action API
> 3:  0f5efb064b ! 3:  bac909a070 upload-pack: allow configuring a missing-action
>     @@ Metadata
>       ## Commit message ##
>          upload-pack: allow configuring a missing-action
>      
>     -    In case some objects are missing from a server, it might still be
>     +    In case some objects are missing from a server, it is sometimes
>          useful to be able to fetch or clone from it if the client already has
>          the missing objects or can get them in some way.
>      
>     -    For example, in case both the server and the client are using a
>     -    separate promisor remote that contain some objects, it can be better
>     -    if the server doesn't try to send such objects back to the client, but
>     -    instead let the client get those objects separately from the promisor
>     -    remote. (The client needs to have the separate promisor remote
>     -    configured, for that to work.)
>     +    Suppose repository S borrows from its "promisor" X, and repository C
>     +    which initially cloned from S borrows from its "promisor" S. If C
>     +    wants an object in order to fill in the gap in its object graph, and
>     +    S does not have it (as S itself has no need for that object), then it
>     +    makes sense to let C go directly to X bypassing S.

Most notably, what is still missing in this iteration, even though I
already pointed it out in the earlier reviews, is that the readers
would not get a good sense of how much trust they need to place on
the other side S, in order to save their repositories from getting
corrupted by S sending an incomplete pack, and what mechanism there
already is to make sure missing objects after fetching such an
incomplete pack from S are all available at X.

In short, I agree with the goal of having "S is borrowing from X, we
cloned from S, we can fill our missing objects by lazily fetching
directly from X" as a feature.  But I want to see it as a safe
feature, but from these patches I do not see how the necessary
safety is guaranteed.

Thanks.

      parent reply	other threads:[~2024-05-24 18:25 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-18 18:40 [PATCH 0/4] upload-pack: support a missing-action Christian Couder
2024-04-18 18:40 ` [PATCH 1/4] rev-list: refactor --missing=<missing-action> Christian Couder
2024-04-18 21:39   ` Junio C Hamano
2024-04-18 18:40 ` [PATCH 2/4] missing: support rejecting --missing=print Christian Couder
2024-04-18 21:47   ` Junio C Hamano
2024-04-18 18:40 ` [PATCH 3/4] pack-objects: use the missing action API Christian Couder
2024-04-18 18:40 ` [PATCH 4/4] upload-pack: allow configuring a missing-action Christian Couder
2024-04-18 19:21 ` [PATCH 0/4] upload-pack: support " Junio C Hamano
2024-05-24 16:39 ` [PATCH v3 0/3] " Christian Couder
2024-05-24 16:39   ` [PATCH v3 1/3] rev-list: refactor --missing=<missing-action> Christian Couder
2024-05-24 16:39   ` [PATCH v3 2/3] pack-objects: use the missing action API Christian Couder
2024-05-24 16:39   ` [PATCH v3 3/3] upload-pack: allow configuring a missing-action Christian Couder
2024-05-24 18:25   ` Junio C Hamano [this message]

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=xmqq5xv3kqxh.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johncai86@gmail.com \
    --cc=ps@pks.im \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).