Git development
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Kristofer Karlsson <krka@spotify.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Kristofer Karlsson via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org,
	Christian Couder <christian.couder@gmail.com>
Subject: Re: [PATCH] rev-list: ignore replacements in the connectivity check
Date: Tue, 3 Nov 2026 11:17:16 +0100	[thread overview]
Message-ID: <aum0rJ6T-OFWrMqW@pks.im> (raw)
In-Reply-To: <CAL71e4M7yFnTzYD0eVVGRmzkzSXbU0KZ-JScQOyB4-dB5xkAHA@mail.gmail.com>

On Tue, Nov 03, 2026 at 09:35:07AM +0100, Kristofer Karlsson wrote:
> On Mon, 2 Nov 2026 at 23:05, Junio C Hamano <gitster@pobox.com> wrote:
> >
> > "Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
> > writes:
> >
> > > Teach the connectivity check to ignore replacements as well, so
> > > that it verifies the same objects that the commands relying on its
> > > result will read.
> >
> > Sounds like absolutely the right thing to do.  We might want to say
> > that we check both the original and the replacement, i.e., instead
> > of ignoring replace refs, we ensure that the commits, trees, and
> > blobs reachable from the replacement objects all exist by treating
> > those objects as if they were also at the tips of the connectivity
> > graph.
> >
> > [snip]
> > We can address the "shouldn't we make sure that replacement objects
> > used when the resulting history is in use are backed by objects
> > reachable from them?" issue separately.

Yeah, I was wondering whether we shouldn't verify both original and
replacement object. Previously we checked the latter, now we check the
former. But is either of those variants the correct thing to do?

> I have not fully thought this through yet, but I'm happy
> to follow up with further strengthening if there's a gap here.
> 
> My preliminary thinking is that since replacements are
> implemented as regular refs (refs/replace/OLD -> NEW),
> their reachable objects should already be covered by
> ordinary connectivity checks when those refs are fetched
> or pushed.

That's fair, I guess. The question though is whether we can rely on that
property when the replace ref was already written locally. But I'm not
sure that we even _should_ care about that. It's arguably the
responsibility of the user that wrote the replace ref to make sure that
the new object is valid. And in the context of the connectivity check
done as part of a push we always assume that existing refs are fully
connected anyway.

So overall I'm aligned with this change.

Patrick

  reply	other threads:[~2026-11-03 10:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-02 15:47 [PATCH] rev-list: ignore replacements in the connectivity check Kristofer Karlsson via GitGitGadget
2026-11-02 22:05 ` Junio C Hamano
2026-11-03  8:35   ` Kristofer Karlsson
2026-11-03 10:17     ` Patrick Steinhardt [this message]
2026-11-03 18:36     ` Junio C Hamano
2026-11-03  9:54 ` Karthik Nayak
2026-11-03 10:51   ` Kristofer Karlsson
2026-11-03 10:17 ` Patrick Steinhardt
2026-11-03 10:38   ` Kristofer Karlsson

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=aum0rJ6T-OFWrMqW@pks.im \
    --to=ps@pks.im \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=krka@spotify.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