Re: [PATCH v2 1/3] Move init_skiplist() outside of fsck

From: Jeff King <peff@peff.net>
Date: Jan 18, 2019, 21:30 UTC
Message-ID: <20190118213018.GA28808@sigill.intra.peff.net>
In-Reply-To: <nycvar.QRO.7.76.6.1901182155020.41@tvgsbejvaqbjf.bet>

On Fri, Jan 18, 2019 at 09:59:21PM +0100, Johannes Schindelin wrote:

Show 16 quoted lines

> By that reasoning all the preparatory work for switching to SHA-256 and
> making the references in the Git code base less tied to SHA-1 would be
> irrelevant now, "because we can cross that bridge when we reach it".
> 
> You are suggesting to incur technical debt here. Let's be smarter about
> this. We do not *have* to incur said technical debt. Nothing (except
> mental laziness) makes use do that.
> 
> Instead, we can make our load "when we reach that bridge" a lot lighter
> by already doing the right thing.
> 
> BTW I totally disagree that the skip list is bound to be SHA-1. It is
> bound to be a list of object names, that's what its purpose is, and just
> because we happen to not yet support other hash algorithms but SHA-1 does
> not mean that the skip list is fixed to SHA-1. It'll always be whatever
> hash algorithm is used in the current repository.

Yeah, I agree with this. In particular, the code has already been modified to use "struct object_id" and parse_oid_hex(). So it is not even like somebody will have to come through later and fix the implementation here, and while they're at it change the "SHA-1" in the message. It has literally already been fixed, and is just waiting on parse_oid_hex() to learn about the new hashes behind the scenes.

IMHO the conversion to object_id probably would have been the time to fix that message so we would not even have to be revisiting the discussion now. But that conversion was such a monumental pain it is hard to fault the authors for not picking up every scrap at that moment. ;)

That is no excuse not to do it now, though.

-Peff