git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Stefan Beller" <stefanbeller@googlemail.com>,
	"Torsten Bögershausen" <tboegi@web.de>,
	git@vger.kernel.org
Subject: Re: [PATCH] repack.c: rename and unlink pack file if it exists
Date: Wed, 5 Feb 2014 16:01:29 -0500	[thread overview]
Message-ID: <20140205210129.GA24314@sigill.intra.peff.net> (raw)
In-Reply-To: <xmqqfvnx2s7p.fsf@gitster.dls.corp.google.com>

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

  reply	other threads:[~2014-02-05 21:01 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-04 23:40 [PATCH] repack.c: rename and unlink pack file if it exists Junio C Hamano
2014-02-04 23:43 ` Junio C Hamano
2014-02-05  1:16 ` Jeff King
2014-02-05 17:54   ` Torsten Bögershausen
2014-02-05 20:06   ` Junio C Hamano
2014-02-05 20:12     ` Jeff King
2014-02-05 20:31       ` Junio C Hamano
2014-02-05 20:37         ` Jeff King
2014-02-05 20:54           ` Jeff King
2014-02-05 20:57           ` Junio C Hamano
2014-02-05 21:01             ` Jeff King [this message]
2014-02-05 21:08               ` Junio C Hamano
2014-02-05 21:09                 ` Jeff King
2014-02-05 21:01         ` Torsten Bögershausen
2014-02-05 20:10   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140205210129.GA24314@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=stefanbeller@googlemail.com \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).