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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 18, 2019, 20:59 UTC
Message-ID: <nycvar.QRO.7.76.6.1901182155020.41@tvgsbejvaqbjf.bet>
In-Reply-To: <xmqqzhrxlvfj.fsf@gitster-ct.c.googlers.com>

Hi,

On Fri, 18 Jan 2019, Junio C Hamano wrote:

Show 26 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
> > On Thu, Jan 17 2019, Barret Rhoden wrote:
> >
> >> -		die("Could not open skip list: %s", path);
> >> [...]
> >> +		die("Could not open skip list: %s", path);
> >
> > You're just moving this around, but now that this has two uses let's say
> > "Could not open SHA-1 list; %s" or something like that.
> >
> >> +			die("Invalid SHA-1: %s", sb.buf);
> >
> > Unlike Johannes I think it's fine to leave this. This file-format is
> > SHA-1 only now. We can cross the bridge of making it (and others)
> > SHA-256 somehow when we come to that, whether that'll be allowing
> > variable width or a different file.
> 
> I tend to agree.  The Documentation/glossary-contents.txt makes it
> clear that "object name" is the most formal term to use here, with
> synonyms like "object identifier" and much less formal "hash".  For
> now, "SHA-1" is good enough, even though "object name" is acceptable
> if we really want to future-proof.  But I would suspect that people
> would colloquially keep saying Shaah-one even when we start using
> different hash function(s), so such a future-proofing may not be
> worth it ;-)

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.

So no, introducing mentions of "SHA-1" *now* is not a smart thing to do.

Ciao, Johannes