git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Justin Tobler <jltobler@gmail.com>
Cc: git@vger.kernel.org, karthik.188@gmail.com
Subject: Re: [RFC PATCH 2/2] builtin/receive-pack: add option to skip connectivity check
Date: Wed, 7 May 2025 15:28:37 +0200	[thread overview]
Message-ID: <aBtgBQxv4_NanE-r@pks.im> (raw)
In-Reply-To: <20250507030249.4802-3-jltobler@gmail.com>

On Tue, May 06, 2025 at 10:02:49PM -0500, Justin Tobler wrote:
> During git-receive-pack(1), connectivity of the object graph is
> validated to ensure that the received packfile does not leave the
> repository in a broken state.
> 
> Generally, this check is critical to avoid an incomplete receieved

s/receieved/received/

> packfile from corrupting a repository. In situations where server
> operators validate the connectivity of incoming objects outside of Git,
> such a check may be redundant.

This is a bit handwavy. _I_ know why we at GitLab are doing this, but
other readers won't have the necessary context to be able to judge
whether this really is a good idea. I think the important question to
answer is: why does the server side want to perform the check if Git
already does it anyway? Why is it in a better position to do so? And why
can't we instead have Git itself perform it in the same "better" way?

Ultimately it boils down to having more knowledge around exactly how Git
is being used on the server side. With the additional information we can
make better decisions and we can make assumptions that a general user of
the connectivity check cannot do.

Most importantly, we know that all objects in the repository will always
be fully connected. Received packfiles get filtered, so we won't ever
accept an object that isn't fully connected. Neither will Gitaly write
such an object.

So what we can do in Gitaly specifically is similar to what I proposed
in [1] a while ago: we simply walk all received objects and then verify
that the edges resolve to objects in the existing repository. The idea
was rightfully shot down because we cannot assume in general that a mere
object walk in the quarantine directory really means that all objects
are fully connected. But with the extra knowledge we have in Gitaly we
can do this optimization indeed.

Patrick

[1]: <cover.1621451532.git.ps@pks.im>

  reply	other threads:[~2025-05-07 13:28 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-07  3:02 [RFC PATCH 0/2] builtin/receive-pack: introduce option to skip connectivity checks Justin Tobler
2025-05-07  3:02 ` [RFC PATCH 1/2] t5412: test receive-pack connectivity check Justin Tobler
2025-05-07 13:28   ` Patrick Steinhardt
2025-05-19 21:08     ` Justin Tobler
2025-05-07  3:02 ` [RFC PATCH 2/2] builtin/receive-pack: add option to skip " Justin Tobler
2025-05-07 13:28   ` Patrick Steinhardt [this message]
2025-05-07 17:20     ` Junio C Hamano
2025-05-20  1:49 ` [PATCH 0/2] builtin/receive-pack: introduce option to skip connectivity checks Justin Tobler
2025-05-20  1:49   ` [PATCH 1/2] t5410: test receive-pack connectivity check Justin Tobler
2025-05-20  9:11     ` Karthik Nayak
2025-05-20  1:49   ` [PATCH 2/2] builtin/receive-pack: add option to skip " Justin Tobler
2025-05-20  5:17     ` Patrick Steinhardt
2025-05-20 15:10       ` Justin Tobler
2025-05-20  9:16     ` Karthik Nayak
2025-05-20 16:32   ` [PATCH v2 0/2] builtin/receive-pack: introduce option to skip connectivity checks Justin Tobler
2025-05-20 16:32     ` [PATCH v2 1/2] t5410: test receive-pack connectivity check Justin Tobler
2025-05-20 16:32     ` [PATCH v2 2/2] builtin/receive-pack: add option to skip " Justin Tobler
2025-06-02 15:01       ` Johannes Schindelin
2025-06-02 15:59         ` Justin Tobler
2025-06-02 16:06           ` Johannes Schindelin
2025-06-02 18:16             ` Junio C Hamano
2025-06-03 12:40           ` Patrick Steinhardt
2025-06-05 10:17             ` Johannes Schindelin
2025-06-05 11:00               ` Patrick Steinhardt
2025-05-22  9:09     ` [PATCH v2 0/2] builtin/receive-pack: introduce option to skip connectivity checks Karthik Nayak

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=aBtgBQxv4_NanE-r@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=jltobler@gmail.com \
    --cc=karthik.188@gmail.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 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).