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

2014-02-05 Thread Torsten Bögershausen
On 2014-02-05 02.16, Jeff King wrote: On Tue, Feb 04, 2014 at 03:40:15PM -0800, Junio C Hamano wrote: * Somehow this came to my private mailbox without Cc to list, so I am forwarding it. I think with 1190a1ac (pack-objects: name pack files after trailer hash, 2013-12-05),

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

2014-02-05 Thread Junio C Hamano
... and this is the other half that is supposed to be only about renaming variables. -- 8 -- From: Torsten Bögershausen tbo...@web.de Date: Sun, 2 Feb 2014 16:09:56 +0100 Subject: [PATCH] repack.c: rename a few variables Rename the variables to match what they are used for: fname for the final

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

2014-02-05 Thread Jeff King
On Wed, Feb 05, 2014 at 12:06:41PM -0800, Junio C Hamano wrote: 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.

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

2014-02-05 Thread Junio C Hamano
Jeff King p...@peff.net writes: 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. Sounds sensible. Run repack -a -d once, and then another

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

2014-02-05 Thread Junio C Hamano
Jeff King p...@peff.net writes: On Tue, Feb 04, 2014 at 03:40:15PM -0800, Junio C Hamano wrote: * Somehow this came to my private mailbox without Cc to list, so I am forwarding it. I think with 1190a1ac (pack-objects: name pack files after trailer hash, 2013-12-05), repacking

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

2014-02-05 Thread Jeff King
On Wed, Feb 05, 2014 at 12:31:34PM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: 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,

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

2014-02-05 Thread Jeff King
On Wed, Feb 05, 2014 at 03:37:40PM -0500, Jeff King wrote: Sounds sensible. Run repack -a -d once, and then another while forcing it to be single threaded, or something? Certainly that's the way to trigger the code, but doing this: [...] ...does not seem to fail, and it does not seem to

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

2014-02-05 Thread Junio C Hamano
Jeff King p...@peff.net writes: On Wed, Feb 05, 2014 at 12:31:34PM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: 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

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

2014-02-05 Thread Torsten Bögershausen
On 2014-02-05 21.31, Junio C Hamano wrote: Jeff King p...@peff.net writes: 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. Sounds

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

2014-02-05 Thread Jeff King
On Wed, Feb 05, 2014 at 12:57:14PM -0800, Junio C Hamano wrote: ...does not seem to fail, and it does not seem to leave any cruft in place. So maybe I am misunderstanding the thing the patch is meant to fix. Is it that we simply do not replace the pack in this instance? Yes. Not just

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

2014-02-05 Thread Junio C Hamano
Jeff King p...@peff.net writes: ... So the fact that this bug exists doesn't really produce any user-visible behavior, and hopefully post-release we would drop the code entirely, and the test would have no reason to exist. Heh, thanks, and I agree with the reasoning for the longer-term

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

2014-02-05 Thread Jeff King
On Wed, Feb 05, 2014 at 01:08:36PM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: ... So the fact that this bug exists doesn't really produce any user-visible behavior, and hopefully post-release we would drop the code entirely, and the test would have no reason to exist.

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

2014-02-04 Thread Junio C Hamano
This comment in builtin/repack.c: First see if there are packs of the same name and if so if we can move them out of the way (this can happen if we repacked immediately after packing fully). When a repo was fully repacked, and is repacked again, we may run into the situation that new

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

2014-02-04 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes: This comment in builtin/repack.c: ... Oops; there was supposed to be these lines before anythning else: From: Torsten Bögershausen tbo...@web.de Date: Sun Feb 2 16:09:56 2014 +0100 First see if there are packs of the same name and

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

2014-02-04 Thread Jeff King
On Tue, Feb 04, 2014 at 03:40:15PM -0800, Junio C Hamano wrote: * Somehow this came to my private mailbox without Cc to list, so I am forwarding it. I think with 1190a1ac (pack-objects: name pack files after trailer hash, 2013-12-05), repacking the same set of objects may have