On Thu, Sep 05, 2024 at 08:41:16AM -0700, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> 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
