From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff King Subject: Re: [PATCH] repack.c: rename and unlink pack file if it exists Date: Wed, 5 Feb 2014 16:01:29 -0500 Message-ID: <20140205210129.GA24314@sigill.intra.peff.net> References: <20140205011632.GA3923@sigill.intra.peff.net> <20140205201243.GA16899@sigill.intra.peff.net> <20140205203740.GA17077@sigill.intra.peff.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Cc: Stefan Beller , Torsten =?utf-8?Q?B=C3=B6gershausen?= , git@vger.kernel.org To: Junio C Hamano X-From: git-owner@vger.kernel.org Wed Feb 05 22:01:42 2014 Return-path: Envelope-to: gcvg-git-2@plane.gmane.org Received: from vger.kernel.org ([209.132.180.67]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1WB9bJ-00066I-Eg for gcvg-git-2@plane.gmane.org; Wed, 05 Feb 2014 22:01:41 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756180AbaBEVBe (ORCPT ); Wed, 5 Feb 2014 16:01:34 -0500 Received: from cloud.peff.net ([50.56.180.127]:45314 "HELO peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1756164AbaBEVBb (ORCPT ); Wed, 5 Feb 2014 16:01:31 -0500 Received: (qmail 19426 invoked by uid 102); 5 Feb 2014 21:01:31 -0000 Received: from c-71-63-4-13.hsd1.va.comcast.net (HELO sigill.intra.peff.net) (71.63.4.13) (smtp-auth username relayok, mechanism cram-md5) by peff.net (qpsmtpd/0.84) with ESMTPA; Wed, 05 Feb 2014 15:01:31 -0600 Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Wed, 05 Feb 2014 16:01:29 -0500 Content-Disposition: inline In-Reply-To: Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Archived-At: 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 the command finishing OK, but the packfile left by > the first repack needs to be left intact. [...] Thanks for the explanation. Having looked at this now, I'm thinking a test may not be worth the trouble. Due to 1190a1ac, we effectively don't care whether we get the old pack or the new one. 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. > We could use test-chmtime to reset the timestamp of the packfile > generated by the first repack to somewhere reasonably old and then > rerun the repack to see that it is a different file, which may be > more portable than inspecting the inum of the packfile. Yeah, I think that would work. But it sounds like we also need a filesystem in which rename() does not overwrite. So the test would not be portable. -Peff