Re: [PATCH] repack.c: rename and unlink pack file if it exists

From: Jeff King <peff@peff.net>
Date: Feb 5, 2014, 20:12 UTC
Message-ID: <20140205201243.GA16899@sigill.intra.peff.net>
In-Reply-To: <xmqq1tzh494e.fsf@gitster.dls.corp.google.com>

On Wed, Feb 05, 2014 at 12:06:41PM -0800, Junio C Hamano wrote:

Show 13 quoted lines

> > Actually, since 1190a1ac, if you have repacked and gotten the same pack
> > name, then you do not have to do any rename dance at all; you can throw
> > away what you just generated because you know that it is byte-for-byte
> > identical.
> >
> > You could collide with a pack created by an older version of git that
> > used the original scheme, but that is quite unlikely (on the order of
> > 2^-160).
> 
> Yes, so in that sense this is not so urgent, but I'm tempted to
> split the original patch into two and merge only the first one to
> 'master' before -rc3 (see below).  The renaming of the variables
> added enough noise to cause me fail to spot a change mixed within.

That sounds very sensible. The only reason I did not follow-up 1190a1ac immediately with a patch to drop the rename code was that it is a sensitive area, and I wanted to be very sure there would be no other fallouts. And then of course I didn't get around to it yet. But following the same logic, trying to do it during -rc would be a terrible idea. :)

The minimal fix you posted below does make sense to me as a stopgap, and we can look into dropping the code entirely during the next cycle. It would be nice to have a test to cover this case, though.

-Peff