fast-import: invalidate pack_id references after loosening

When loosening a pack, the current pack_id gets reused when checkpointing and the import does not terminate. This causes problems after checkpointing as the object table, branch, and tag lists still contains pre-checkpoint references to the recycled pack_id. Merely clearing the object_table as suggested by Jeff King in http://mid.gmane.org/20160517121330.GA7346@sigill.intra.peff.net is insufficient as the marks set still contains references to object entries. Wrong pack_id references branch and tags lists do not cause errors, but can lead to misleading crash reports and core dumps, so they are also invalidated. Signed-off-by: Eric Wong <e@80x24.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Eric Wong committed May 25, 2016 at 22:54 UTC d2986d0f290a065fb8a534fabfff36c40d37ae97
2 files changed +87 -1
fast-import.c
+30 -1
@@ -597,6 +597,33 @@ static struct object_entry *insert_object(unsigned char *sha1)
597 return e;
598 }
599
600 +static void invalidate_pack_id(unsigned int id)
601 +{
602 + unsigned int h;
603 + unsigned long lu;
604 + struct tag *t;
605 +
606 + for (h = 0; h < ARRAY_SIZE(object_table); h++) {
607 + struct object_entry *e;
608 +
609 + for (e = object_table[h]; e; e = e->next)
610 + if (e->pack_id == id)
611 + e->pack_id = MAX_PACK_ID;
612 + }
613 +
614 + for (lu = 0; lu < branch_table_sz; lu++) {
615 + struct branch *b;
616 +
617 + for (b = branch_table[lu]; b; b = b->table_next_branch)
618 + if (b->pack_id == id)
619 + b->pack_id = MAX_PACK_ID;
620 + }
621 +
622 + for (t = first_tag; t; t = t->next_tag)
623 + if (t->pack_id == id)
624 + t->pack_id = MAX_PACK_ID;
625 +}
626 +
627 static unsigned int hc_str(const char *s, size_t len)
628 {
629 unsigned int r = 0;
@@ -993,8 +1020,10 @@ static void end_packfile(void)
1020 cur_pack_sha1, pack_size);
1021
1022 if (object_count <= unpack_limit) {
996 - if (!loosen_small_pack(pack_data))
1023 + if (!loosen_small_pack(pack_data)) {
1024 + invalidate_pack_id(pack_id);
1025 goto discard_pack;
1026 + }
1027 }
1028
1029 close(pack_data->pack_fd);
t/t9302-fast-import-unpack-limit.sh
+57
@@ -45,4 +45,61 @@ test_expect_success 'bigger packs are preserved' '
45 test $(find .git/objects/pack -type f | wc -l) -eq 2
46 '
47
48 +test_expect_success 'lookups after checkpoint works' '
49 + hello_id=$(echo hello | git hash-object --stdin -t blob) &&
50 + id="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE" &&
51 + before=$(git rev-parse refs/heads/master^0) &&
52 + (
53 + cat <<-INPUT_END &&
54 + blob
55 + mark :1
56 + data 6
57 + hello
58 +
59 + commit refs/heads/master
60 + mark :2
61 + committer $id
62 + data <<COMMIT
63 + checkpoint after this
64 + COMMIT
65 + from refs/heads/master^0
66 + M 100644 :1 hello
67 +
68 + # pre-checkpoint
69 + cat-blob :1
70 + cat-blob $hello_id
71 + checkpoint
72 + # post-checkpoint
73 + cat-blob :1
74 + cat-blob $hello_id
75 + INPUT_END
76 +
77 + n=0 &&
78 + from=$before &&
79 + while test x"$from" = x"$before"
80 + do
81 + if test $n -gt 30
82 + then
83 + echo >&2 "checkpoint did not update branch"
84 + exit 1
85 + else
86 + n=$(($n + 1))
87 + fi &&
88 + sleep 1 &&
89 + from=$(git rev-parse refs/heads/master^0)
90 + done &&
91 + cat <<-INPUT_END &&
92 + commit refs/heads/master
93 + committer $id
94 + data <<COMMIT
95 + make sure from "unpacked sha1 reference" works, too
96 + COMMIT
97 + from $from
98 + INPUT_END
99 + echo done
100 + ) | git -c fastimport.unpackLimit=100 fast-import --done &&
101 + test $(find .git/objects/?? -type f | wc -l) -eq 6 &&
102 + test $(find .git/objects/pack -type f | wc -l) -eq 2
103 +'
104 +
105 test_done