From: Jeff King <peff@peff.net>
To: Duy Nguyen <pclouds@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH v2 1/3] index-pack: add --unpack-limit to unpack objects
Date: Sun, 8 Sep 2013 02:35:34 -0400 [thread overview]
Message-ID: <20130908063533.GA22949@sigill.intra.peff.net> (raw)
In-Reply-To: <CACsJy8DeJnwJ2hxbKs+ks3Rp6h+uedFA+qt8JTTwyen8sF7XMw@mail.gmail.com>
On Sun, Sep 08, 2013 at 01:28:47PM +0700, Nguyen Thai Ngoc Duy wrote:
> > From a cursory read, this seems fine. If it were done in complete
> > isolation, I'd say it was a slight regression, just because we are doing
> > more I/O for the unpack case, and it is not really saving us any code
> > (it is not like we can throw away unpack-objects, as I think we would
> > want to keep it as a last resort for getting data out of malformed or
> > otherwise non-indexable packs).
>
> I can see unpack-objects is more tolerable on corrupt/incomplete
> packs. If index-pack finds something wrong, it aborts the whole
> process. I think we can make index-pack stop at the first bad object,
> adjust nr_objects, and try to recover as much as possible out of the
> good part. Any other reasons to keep unpack-objects (because I still
> want to kill it)?
No, I don't think there is another reason. The command name may hang
around for historical reasons, but we can always make it an alias for
"index-pack --unpack-limit=0" or whatever.
I do not think index-pack even needs to be particularly clever about
trying to recover. It is mainly that we may do some extra sanity checks
when writing the index (e.g., the recent discussion on avoiding
duplicates in the index), that do not even come up when simply exploding
the pack in a linear fashion. But I don't think there is any fundamental
reason why index-pack could not learn to be as robust when operating in
unpack mode.
As an aside, you cannot just drop the nr_objects that index-pack's
generated index says it contains. Packfile readers double-check that the
.idx and .pack agree on the number of objects (I tried it as a method
for working around duplicate entries :) ).
-Peff
prev parent reply other threads:[~2013-09-08 6:35 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-02 3:05 [PATCH] {fetch,receive}-pack: drop unpack-objects, delay loosing objects until the end Nguyễn Thái Ngọc Duy
2013-09-02 4:38 ` Eric Sunshine
2013-09-03 6:49 ` Jeff King
2013-09-03 11:56 ` Duy Nguyen
2013-09-03 17:25 ` Jeff King
2013-09-06 0:46 ` [PATCH v2 1/3] index-pack: add --unpack-limit to unpack objects Nguyễn Thái Ngọc Duy
2013-09-06 0:46 ` [PATCH v2 2/3] fetch-pack: use index-pack --unpack-limit instead of unpack-objects Nguyễn Thái Ngọc Duy
2013-09-08 4:45 ` Jeff King
2013-09-06 0:46 ` [PATCH v2 3/3] receive-pack: " Nguyễn Thái Ngọc Duy
2013-09-08 4:44 ` [PATCH v2 1/3] index-pack: add --unpack-limit to unpack objects Jeff King
2013-09-08 6:28 ` Duy Nguyen
2013-09-08 6:35 ` Jeff King [this message]
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=20130908063533.GA22949@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=pclouds@gmail.com \
/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).