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....@web.de>
Signed-off-by: Jeff King <p...@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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to