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

From: Jeff King <peff@peff.net>
Date: Jan 22, 2019, 07:12 UTC
Message-ID: <20190122071251.GB28555@sigill.intra.peff.net>
In-Reply-To: <87h8e54n6y.fsf@evledraar.gmail.com>

On Fri, Jan 18, 2019 at 11:26:29PM +0100, Ævar Arnfjörð Bjarmason wrote:

Show 13 quoted lines

> I stand corrected, I thought these still needed to be updated to parse
> anything that wasn't 40 chars, since I hadn't seen anything about these
> formats in the hash transition document.
> 
> So fair enough, let's change that while we're at it, but this seems like
> something that needs to be planned for in more detail / documented in
> the hash transition doc.
> 
> I.e. many (e.g. me) maintain some system-wide skiplist for strict fsck
> cloning of legacy repos. So I can see there being some need for a
> SHA1<->SHA256 map in this case, but since these files might stretch
> across repo boundaries and not be checked into the repo itself this is a
> new use-case that needs thinking about.

My assumption had been that changing your local repository would be a (local) flag day, and you'd update any ancillary files like skiplists, mailmap.blob, etc at the same time. I'm not opposed to making those features more clever, though.

Show 5 quoted lines

> But now that I think about it this sort of thing would be a good
> use-case for just fixing these various historical fsck issues while
> we're at it when possible, e.g. "missing space before email" (probably
> not all could be unambiguously fixed). So instead of sha256<->sha1
> fn(sha256)<->fn(sha1)[1]?

That is a very tempting thing to do, but I think it comes with its own complications. We do not want to do fn(sha1), I don't think; the reason we care about sha1 at all is that those hashes are already set in stone.

There could be a "clean up the data as we convert to sha256" operation, but:

  - it needs to be set in stone from day 1, I'd think. The last thing we
    want is to modify it after conversions are in the wild

  - I think we need to be bi-directional. So it must be a mapping that
    can be undone to retrieve the original bytes, so we can compute
    their "real" sha1.

At which point, I think it might be simpler to just make git more permissive with respect to those minor data errors (and in fact, we are already pretty permissive for the most part in non-fsck operations).

-Peff