Merge branch 'jk/index-pack-dupfix' into maint

gitster · gitster · commit 7e84f4608f9c · 2020-03-17T15:02:22.000-07:00
The index-pack code now diagnoses a bad input packstream that
records the same object twice when it is used as delta base; the
code used to declare a software bug when encountering such an
input, but it is an input error.

* jk/index-pack-dupfix:
  index-pack: downgrade twice-resolved REF_DELTA to die()
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
@@ -1003,7 +1003,9 @@ static struct base_data *find_unresolved_deltas_1(struct base_data *base,
 
 		if (!compare_and_swap_type(&child->real_type, OBJ_REF_DELTA,
 					   base->obj->real_type))
-			BUG("child->real_type != OBJ_REF_DELTA");
+			die("REF_DELTA at offset %"PRIuMAX" already resolved (duplicate base %s?)",
+			    (uintmax_t)child->idx.offset,
+			    oid_to_hex(&base->obj->idx.oid));
 
 		resolve_delta(child, base, result);
 		if (base->ref_first == base->ref_last && base->ofs_last == -1)
diff --git a/t/t5309-pack-delta-cycles.sh b/t/t5309-pack-delta-cycles.sh
@@ -62,13 +62,13 @@ test_expect_success 'index-pack detects REF_DELTA cycles' '
 	test_must_fail git index-pack --fix-thin --stdin <cycle.pack
 '
 
-test_expect_failure 'failover to an object in another pack' '
+test_expect_success 'failover to an object in another pack' '
 	clear_packs &&
 	git index-pack --stdin <ab.pack &&
-	git index-pack --stdin --fix-thin <cycle.pack
+	test_must_fail git index-pack --stdin --fix-thin <cycle.pack
 '
 
-test_expect_failure 'failover to a duplicate object in the same pack' '
+test_expect_success 'failover to a duplicate object in the same pack' '
 	clear_packs &&
 	{
 		pack_header 3 &&
@@ -77,7 +77,7 @@ test_expect_failure 'failover to a duplicate object in the same pack' '
 		pack_obj $A
 	} >recoverable.pack &&
 	pack_trailer recoverable.pack &&
-	git index-pack --fix-thin --stdin <recoverable.pack
+	test_must_fail git index-pack --fix-thin --stdin <recoverable.pack
 '
 
 test_done

Original file line number	Diff line number	Diff line change
`@@ -62,13 +62,13 @@ test_expect_success 'index-pack detects REF_DELTA cycles' '`
`62`	`62`	`test_must_fail git index-pack --fix-thin --stdin <cycle.pack`
`63`	`63`	`'`
`64`	`64`
`65`		`-test_expect_failure 'failover to an object in another pack' '`
	`65`	`+test_expect_success 'failover to an object in another pack' '`
`66`	`66`	`clear_packs &&`
`67`	`67`	`git index-pack --stdin <ab.pack &&`
`68`		`- git index-pack --stdin --fix-thin <cycle.pack`
	`68`	`+ test_must_fail git index-pack --stdin --fix-thin <cycle.pack`
`69`	`69`	`'`
`70`	`70`
`71`		`-test_expect_failure 'failover to a duplicate object in the same pack' '`
	`71`	`+test_expect_success 'failover to a duplicate object in the same pack' '`
`72`	`72`	`clear_packs &&`
`73`	`73`	`{`
`74`	`74`	`pack_header 3 &&`
`@@ -77,7 +77,7 @@ test_expect_failure 'failover to a duplicate object in the same pack' '`
`77`	`77`	`pack_obj $A`
`78`	`78`	`} >recoverable.pack &&`
`79`	`79`	`pack_trailer recoverable.pack &&`
`80`		`- git index-pack --fix-thin --stdin <recoverable.pack`
	`80`	`+ test_must_fail git index-pack --fix-thin --stdin <recoverable.pack`
`81`	`81`	`'`
`82`	`82`
`83`	`83`	`test_done`