Re: [PATCH 2/2] index-pack: detect local corruption in collision check

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 1, 2017, 18:04 UTC
Message-ID: <xmqqshlsuhql.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170401080931.t67jbtta4w4enui6@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> is not a collision but rather local corruption. We should
> reoprt that instead (just like we do if reading the rest of
> the object content fails a few lines later).

s/reoprt/report/ (locally amended while queuing).

> We may want to tighten that up. In the long run, I'd love to see
> unpack-objects go away in favor of teaching index-pack how to write
> loose objects. The two had very similar code once upon a time, but
> index-pack has grown a lot of feature and bugfixes over the years.

This sounds like a nice future to aspire to reach.

Besides having to maintain two separate executables, another downside of the current arrangement is that we have to make the decision between streaming to a single pack and exploding into loose objects too early and base our decision solely on the object count. If we are moving to a single receiver, it is conceivable to switch to a scheme based on the size of the incoming pack (e.g. spool the first N MB and if we run out we write out loose objects, otherwise create a new pack, and dump the spooled part and stream the rest into it while indexing).

Thanks.