Re: [PATCH 0/4] hash.h: support choosing a separate SHA-1 for non-cryptographic uses

[Taylor Blau <me@ttaylorr.com>]

On Thu, Sep 05, 2024 at 08:41:16AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Probably the solution is:
> >
> >   - renaming packfiles into place should use finalize_object_file() to
> >     avoid collisions.  That happens for tmp-objdir migration already,
> >     but we should do it more directly in index-pack (and maybe even
> >     pack-objects). And possibly we should implement an actual
> >     byte-for-byte comparison if we think we saw a collision, rather than
> >     just assuming that the write was effectively a noopi (see the FIXME
> >     in that function). That becomes more important if the checksum gets
> >     more likely to collide accidentally (we essentially ignore the
> >     possibility that sha1 would ever do so).
>
> Yes.  I somehow mistakenly thought that Taylor analized the code
> path when brian raised the "we rename over, overwriting existing
> files" and I included fixing it as one of the steps necessary to
> safely switch the tail sum to weaker and faster hash, but after
> reading the thread again, it seems that I was hallucinating X-<.
> This needs to be corrected.

Just to make sure I understand you here... this is talking about a
prerequisite step for switching index-pack to use a faster hash, right?

If so, I agree, but would note that this series does not yet switch
index-pack to use the non-collision detecting SHA-1 implementation when
available, so that would not be a prerequisite for merging this series.

Thanks,
Taylor