From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 28 Apr 2025 22:07:19 GMT
Subject: Re: [PATCH v2 12/13] bulk-checkin: don't fetch promised objects on write
Message-ID: <xmqqikmo9bzc.fsf@gitster.g>
In-Reply-To: <20250425-pks-object-store-cleanups-v2-12-63f1695b7700@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> In an ideal world, we would protect against this by fetching the
> promised object and then performing a collision check. But this feels
> exceedingly expensive and ultimately rather pointless, as more common
> writing paths like `write_loose_object()` don't protect against this
> scenario either.

When writing loose object, wouldn't collision check kick in, and
didn't we compare "existing (not here but virtually here due to
promisor)" object and what write_loose_object() tried to create, at
least before this series which may (or may not; I lost track) have
disabled that check?

I think the overall goal of deprecating the function with long name
with another function with a short-and-sweet name with different
default is a worthy thing, and while I do agree with "as we are
replacing function with another with different default, we need to
pass different flags to keep the same behaviour" early parts of the
series, I am not sure about these latter steps.

Thanks.