pack-object: tolerate broken packs that have duplicated objects

When --reuse-delta is in effect (which is the default), and an existing pack in the repository has the same object registered twice (e.g. one copy in a non-delta format and the other copy in a delta against some other object), an attempt to repack the repository can result in a cyclic delta dependency, causing write_one() function to infinitely recurse into itself. Detect such a case and break the loopy dependency by writing out an object that is involved in such a loop in the non-delta format. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Nov 16, 2011 at 22:04 UTC f63c79dbc8c5670533557d311ed0e81fcfc9fe3a
1 file changed +43 -12
builtin/pack-objects.c
+43 -12
@@ -404,25 +404,56 @@ static unsigned long write_object(struct sha1file *f,
404 return hdrlen + datalen;
405 }
406
407 -static int write_one(struct sha1file *f,
408 - struct object_entry *e,
409 - off_t *offset)
407 +enum write_one_status {
408 + WRITE_ONE_SKIP = -1, /* already written */
409 + WRITE_ONE_BREAK = 0, /* writing this will bust the limit; not written */
410 + WRITE_ONE_WRITTEN = 1, /* normal */
411 + WRITE_ONE_RECURSIVE = 2 /* already scheduled to be written */
412 +};
413 +
414 +static enum write_one_status write_one(struct sha1file *f,
415 + struct object_entry *e,
416 + off_t *offset)
417 {
418 unsigned long size;
419 + int recursing;
420
413 - /* offset is non zero if object is written already. */
414 - if (e->idx.offset || e->preferred_base)
415 - return -1;
421 + /*
422 + * we set offset to 1 (which is an impossible value) to mark
423 + * the fact that this object is involved in "write its base
424 + * first before writing a deltified object" recursion.
425 + */
426 + recursing = (e->idx.offset == 1);
427 + if (recursing) {
428 + warning("recursive delta detected for object %s",
429 + sha1_to_hex(e->idx.sha1));
430 + return WRITE_ONE_RECURSIVE;
431 + } else if (e->idx.offset || e->preferred_base) {
432 + /* offset is non zero if object is written already. */
433 + return WRITE_ONE_SKIP;
434 + }
435
436 /* if we are deltified, write out base object first. */
418 - if (e->delta && !write_one(f, e->delta, offset))
419 - return 0;
437 + if (e->delta) {
438 + e->idx.offset = 1; /* now recurse */
439 + switch (write_one(f, e->delta, offset)) {
440 + case WRITE_ONE_RECURSIVE:
441 + /* we cannot depend on this one */
442 + e->delta = NULL;
443 + break;
444 + default:
445 + break;
446 + case WRITE_ONE_BREAK:
447 + e->idx.offset = recursing;
448 + return WRITE_ONE_BREAK;
449 + }
450 + }
451
452 e->idx.offset = *offset;
453 size = write_object(f, e, *offset);
454 if (!size) {
424 - e->idx.offset = 0;
425 - return 0;
455 + e->idx.offset = recursing;
456 + return WRITE_ONE_BREAK;
457 }
458 written_list[nr_written++] = &e->idx;
459
@@ -430,7 +461,7 @@ static int write_one(struct sha1file *f,
461 if (signed_add_overflows(*offset, size))
462 die("pack too large for current definition of off_t");
463 *offset += size;
433 - return 1;
464 + return WRITE_ONE_WRITTEN;
465 }
466
467 static void write_pack_file(void)
@@ -468,7 +499,7 @@ static void write_pack_file(void)
499 offset = sizeof(hdr);
500 nr_written = 0;
501 for (; i < nr_objects; i++) {
471 - if (!write_one(f, objects + i, &offset))
502 + if (write_one(f, objects + i, &offset) == WRITE_ONE_BREAK)
503 break;
504 display_progress(progress_state, written);
505 }