Thread (2 messages) 2 messages, 2 authors, 2025-04-29

Re: [PATCH v2 12/13] bulk-checkin: don't fetch promised objects on write

From: Patrick Steinhardt <hidden>
Date: 2025-04-29 06:15:54

On Mon, Apr 28, 2025 at 03:07:19PM -0700, Junio C Hamano wrote:
Patrick Steinhardt [off-list ref] writes:
quoted
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.
Yeah, to be honest I wasn't totally sure whether to include these steps
myself as I anticipated that they will lead to discussions that derail
my original goal, which is to clean up the interfaces in the object
subsystem. I decided to go with these where I thought that my train of
thought is reasonable, but given your comments I'll probably just drop
those patches.

We can still adapt these callsites in the future as needed.

Patrick
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help