From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "René Scharfe" <l.s.r@web.de>,
"Shawn Pearce" <spearce@spearce.org>,
"Martin von Gagern" <Martin.vGagern@gmx.net>,
git@vger.kernel.org
Subject: [PATCH 2/2] index-pack: handle duplicate base objects gracefully
Date: Fri, 29 Aug 2014 16:58:10 -0400 [thread overview]
Message-ID: <20140829205809.GB7060@peff.net> (raw)
In-Reply-To: <20140829205538.GD29456@peff.net>
If a pack contains duplicates of an object, and if that
object has any deltas pointing at it with REF_DELTA, we will
try to resolve the deltas twice. While unusual, this is not
strictly an error, but we currently die() with an unhelpful
message. We should instead silently ignore the delta and
move on. The first resolution already filled in any data we
needed (like the sha1).
The duplicate base object is not our concern during the
resolution phase, and the .idx-writing phase will decide
whether to complain or allow it (based on whether --strict
is set).
Based-on-work-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/index-pack.c | 18 ++++++++++--------
t/t5308-pack-detect-duplicates.sh | 15 +++++++++++++++
2 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index eebf1a8..acc30a9 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -936,20 +936,22 @@ static struct base_data *find_unresolved_deltas_1(struct base_data *base,
link_base_data(prev_base, base);
}
- if (base->ref_first <= base->ref_last) {
+ while (base->ref_first <= base->ref_last) {
struct object_entry *child = objects + deltas[base->ref_first].obj_no;
- struct base_data *result = alloc_base_data();
+ struct base_data *result = NULL;
- if (!compare_and_swap_type(&child->real_type, OBJ_REF_DELTA,
- base->obj->real_type))
- die("BUG: child->real_type != OBJ_REF_DELTA");
+ if (compare_and_swap_type(&child->real_type, OBJ_REF_DELTA,
+ base->obj->real_type)) {
+ result = alloc_base_data();
+ resolve_delta(child, base, result);
+ }
- resolve_delta(child, base, result);
if (base->ref_first == base->ref_last && base->ofs_last == -1)
free_base_data(base);
-
base->ref_first++;
- return result;
+
+ if (result)
+ return result;
}
if (base->ofs_first <= base->ofs_last) {
diff --git a/t/t5308-pack-detect-duplicates.sh b/t/t5308-pack-detect-duplicates.sh
index 9c5a876..50f7a69 100755
--- a/t/t5308-pack-detect-duplicates.sh
+++ b/t/t5308-pack-detect-duplicates.sh
@@ -77,4 +77,19 @@ test_expect_success 'index-pack can reject packs with duplicates' '
test_expect_code 1 git cat-file -e $LO_SHA1
'
+test_expect_success 'duplicated delta base does not trigger assert' '
+ clear_packs &&
+ {
+ A=01d7713666f4de822776c7622c10f1b07de280dc &&
+ B=e68fe8129b546b101aee9510c5328e7f21ca1d18 &&
+ pack_header 3 &&
+ pack_obj $A $B &&
+ pack_obj $B &&
+ pack_obj $B
+ } >dups.pack &&
+ pack_trailer dups.pack &&
+ git index-pack --stdin <dups.pack &&
+ test_must_fail git index-pack --stdin --strict <dups.pack
+'
+
test_done
--
2.1.0.346.ga0367b9
next prev parent reply other threads:[~2014-08-29 20:58 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-21 11:35 [BUG] resolved deltas Petr Stodulka
2014-08-21 18:25 ` Petr Stodulka
2014-08-22 19:41 ` Martin von Gagern
2014-08-23 10:12 ` René Scharfe
2014-08-23 10:56 ` Jeff King
2014-08-23 11:04 ` Jeff King
2014-08-23 11:18 ` Jeff King
2014-08-25 16:39 ` René Scharfe
2014-08-28 22:08 ` Jeff King
2014-08-28 22:15 ` Jeff King
2014-08-28 23:04 ` Jeff King
2014-08-28 22:22 ` Jeff King
2014-08-28 23:14 ` Junio C Hamano
2014-08-29 20:55 ` Jeff King
2014-08-29 20:57 ` [PATCH 1/2] index-pack: fix race condition with duplicate bases Jeff King
2014-08-29 20:58 ` Jeff King [this message]
2014-08-29 21:56 ` [PATCH 2/2] index-pack: handle duplicate base objects gracefully Junio C Hamano
2014-08-29 22:08 ` Jeff King
2014-08-30 2:59 ` Shawn Pearce
2014-08-30 13:16 ` Jeff King
2014-08-30 16:00 ` René Scharfe
2014-08-31 15:17 ` Jeff King
2014-08-31 16:30 ` René Scharfe
2014-08-31 1:10 ` Shawn Pearce
2014-08-31 15:24 ` Jeff King
2014-08-31 22:23 ` Junio C Hamano
2014-08-30 13:23 ` [PATCH 3/2] t5309: mark delta-cycle failover tests as passing Jeff King
2014-08-31 15:15 ` Jeff King
2014-09-02 17:19 ` Junio C Hamano
2014-08-25 17:19 ` [BUG] resolved deltas Shawn Pearce
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=20140829205809.GB7060@peff.net \
--to=peff@peff.net \
--cc=Martin.vGagern@gmx.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=l.s.r@web.de \
--cc=spearce@spearce.org \
/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).