Sun Chao (my former colleague at Huawei) found a bug of git-pack-redundant. If there are too many packs and many of them overlap each other, running `git pack-redundant --all` will exhaust all memories and the process will be killed by kernel. There is a script in commit log of commit 3/6, which can be used to create a repository with lots of redundant packs. Running `git pack-redundant --all` in it can reproduce this issue. ## Changes since reroll v7 1. Rewrite [PATCH v9 1/6] (t5323: test cases for git-pack-redundant) * Add many tables for relationship of packs and objects. * Change dir in subshell and fixed other issues. 2. New patch file from Sun Chao: [PATCH v9 3/6] (pack-redundant: delete redundant code) 3. Squash patches (remove unused functions) to patch 4/6 (new algorithm to find min packs). ## Range diff 1: 799e804d5e < -: ---------- t5323: test cases for git-pack-redundant -: ---------- > 1: c8dbf8cef2 t5323: test cases for git-pack-redundant 2: 520f6277fb = 2: a6300516d7 pack-redundant: delay creation of unique_objects -: ---------- > 3: fb71973df5 pack-redundant: delete redundant code 3: ab1c2c4950 ! 4: 9963d1c49f pack-redundant: new algorithm to find min packs @@ -76,6 +76,113 @@ diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c --- a/builtin/pack-redundant.c +++ b/builtin/pack-redundant.c +@@ + struct llist *all_objects; + } *local_packs = NULL, *altodb_packs = NULL; + +-struct pll { +- struct pll *next; +- struct pack_list *pl; +-}; +- + static struct llist_item *free_nodes; + + static inline void llist_item_put(struct llist_item *item) +@@ + return new_item; + } + +-static void llist_free(struct llist *list) +-{ +- while ((list->back = list->front)) { +- list->front = list->front->next; +- llist_item_put(list->back); +- } +- free(list); +-} +- + static inline void llist_init(struct llist **list) + { + *list = xmalloc(sizeof(struct llist)); +@@ + } + } + +-static void pll_free(struct pll *l) +-{ +- struct pll *old; +- struct pack_list *opl; +- +- while (l) { +- old = l; +- while (l->pl) { +- opl = l->pl; +- l->pl = opl->next; +- free(opl); +- } +- l = l->next; +- free(old); +- } +-} +- +-/* all the permutations have to be free()d at the same time, +- * since they refer to each other +- */ +-static struct pll * get_permutations(struct pack_list *list, int n) +-{ +- struct pll *subset, *ret = NULL, *new_pll = NULL; +- +- if (list == NULL || pack_list_size(list) < n || n == 0) +- return NULL; +- +- if (n == 1) { +- while (list) { +- new_pll = xmalloc(sizeof(*new_pll)); +- new_pll->pl = NULL; +- pack_list_insert(&new_pll->pl, list); +- new_pll->next = ret; +- ret = new_pll; +- list = list->next; +- } +- return ret; +- } +- +- while (list->next) { +- subset = get_permutations(list->next, n - 1); +- while (subset) { +- new_pll = xmalloc(sizeof(*new_pll)); +- new_pll->pl = subset->pl; +- pack_list_insert(&new_pll->pl, list); +- new_pll->next = ret; +- ret = new_pll; +- subset = subset->next; +- } +- list = list->next; +- } +- return ret; +-} +- +-static int is_superset(struct pack_list *pl, struct llist *list) +-{ +- struct llist *diff; +- +- diff = llist_copy(list); +- +- while (pl) { +- llist_sorted_difference_inplace(diff, pl->all_objects); +- if (diff->size == 0) { /* we're done */ +- llist_free(diff); +- return 1; +- } +- pl = pl->next; +- } +- llist_free(diff); +- return 0; +-} +- + static size_t sizeof_union(struct packed_git *p1, struct packed_git *p2) + { + size_t ret = 0; @@ return ret; } @@ -221,56 +328,56 @@ --- a/t/t5323-pack-redundant.sh +++ b/t/t5323-pack-redundant.sh @@ - P2:$P2 - EOF - + # ALL | x x x x x x x x x x x x x x x x x x + # + ############################################################################# -test_expect_success 'one of pack-2/pack-3 is redundant' ' -+test_expect_failure 'one of pack-2/pack-3 is redundant' ' - git pack-redundant --all >out && - format_packfiles <out >actual && - test_cmp expected actual ++test_expect_failure 'one of pack-2/pack-3 is redundant (failed on Mac)' ' + ( + cd "$master_repo" && + cat >expect <<-EOF && @@ - P6:$P6 - EOF - + # ALL | x x x x x x x x x x x x x x x x x x x + # + ############################################################################# -test_expect_success 'pack 2, 4, and 6 are redundant' ' -+test_expect_failure 'pack 2, 4, and 6 are redundant' ' - git pack-redundant --all >out && - format_packfiles <out >actual && - test_cmp expected actual ++test_expect_failure 'pack 2, 4, and 6 are redundant (failed on Mac)' ' + ( + cd "$master_repo" && + cat >expect <<-EOF && @@ - P8:$P8 - EOF - + # ALL | x x x x x x x x x x x x x x x x x x x + # + ############################################################################# -test_expect_success 'pack-8 (subset of pack-1) is also redundant' ' -+test_expect_failure 'pack-8 (subset of pack-1) is also redundant' ' - git pack-redundant --all >out && - format_packfiles <out >actual && - test_cmp expected actual ++test_expect_failure 'pack-8 (subset of pack-1) is also redundant (failed on Mac)' ' + ( + cd "$master_repo" && + cat >expect <<-EOF && @@ - test_must_be_empty out + ) ' -test_expect_success 'remove redundant packs and pass fsck' ' -+test_expect_failure 'remove redundant packs and pass fsck' ' - git pack-redundant --all | xargs rm && - git fsck --no-progress && - git pack-redundant --all >out && ++test_expect_failure 'remove redundant packs and pass fsck (failed on Mac)' ' + ( + cd "$master_repo" && + git pack-redundant --all | xargs rm && @@ - printf "../../master.git/objects" >objects/info/alternates + ) ' -test_expect_success 'no redundant packs without --alt-odb' ' -+test_expect_failure 'no redundant packs without --alt-odb' ' - git pack-redundant --all >out && - test_must_be_empty out - ' ++test_expect_failure 'no redundant packs without --alt-odb (failed on Mac)' ' + ( + cd "$shared_repo" && + git pack-redundant --all >out && @@ - P7:$P7 - EOF - + # ALL | x x x x x x x x x x x x x x x x x x x + # + ############################################################################# -test_expect_success 'pack-redundant --verbose: show duplicate packs in stderr' ' -+test_expect_failure 'pack-redundant --verbose: show duplicate packs in stderr' ' - git pack-redundant --all --verbose >out 2>out.err && - test_must_be_empty out && - grep "pack$" out.err | format_packfiles >actual && ++test_expect_failure 'pack-redundant --verbose: show duplicate packs in stderr (failed on Mac)' ' + ( + cd "$shared_repo" && + cat >expect <<-EOF && 4: 3c3a7ea40f < -: ---------- pack-redundant: remove unused functions 5: bc4b681f40 ! 5: b8f80ad454 pack-redundant: rename pack_list.all_objects @@ -115,11 +115,7 @@ + alt->remaining_objects); local = local->next; } -- llist_sorted_difference_inplace(all_objects, alt->all_objects); -+ llist_sorted_difference_inplace(all_objects, alt->remaining_objects); alt = alt->next; - } - } @@ return NULL; 6: 6cfba5b4b2 ! 6: 8a12ad699e pack-redundant: consistent sort method @@ -83,60 +83,71 @@ --- a/t/t5323-pack-redundant.sh +++ b/t/t5323-pack-redundant.sh @@ - ' - - cat >expected <<EOF --P2:$P2 -+P3:$P3 - EOF - --test_expect_failure 'one of pack-2/pack-3 is redundant' ' + # | T A B C D E F G H I J K L M N O P Q R + # ----+-------------------------------------- + # P1 | x x x x x x x x +-# P2* | ! ! ! ! ! ! ! +-# P3 | x x x x x x ++# P2 | x x x x x x x ++# P3* | ! ! ! ! ! ! + # P4 | x x x x x + # P5 | x x x x + # ----+-------------------------------------- + # ALL | x x x x x x x x x x x x x x x x x x + # + ############################################################################# +-test_expect_failure 'one of pack-2/pack-3 is redundant (failed on Mac)' ' +test_expect_success 'one of pack-2/pack-3 is redundant' ' - git pack-redundant --all >out && - format_packfiles <out >actual && - test_cmp expected actual + ( + cd "$master_repo" && + cat >expect <<-EOF && +- P2:$P2 ++ P3:$P3 + EOF + git pack-redundant --all >out && + format_packfiles <out >actual && @@ - P6:$P6 - EOF - --test_expect_failure 'pack 2, 4, and 6 are redundant' ' + # ALL | x x x x x x x x x x x x x x x x x x x + # + ############################################################################# +-test_expect_failure 'pack 2, 4, and 6 are redundant (failed on Mac)' ' +test_expect_success 'pack 2, 4, and 6 are redundant' ' - git pack-redundant --all >out && - format_packfiles <out >actual && - test_cmp expected actual + ( + cd "$master_repo" && + cat >expect <<-EOF && @@ - P8:$P8 - EOF - --test_expect_failure 'pack-8 (subset of pack-1) is also redundant' ' + # ALL | x x x x x x x x x x x x x x x x x x x + # + ############################################################################# +-test_expect_failure 'pack-8 (subset of pack-1) is also redundant (failed on Mac)' ' +test_expect_success 'pack-8 (subset of pack-1) is also redundant' ' - git pack-redundant --all >out && - format_packfiles <out >actual && - test_cmp expected actual + ( + cd "$master_repo" && + cat >expect <<-EOF && @@ - test_must_be_empty out + ) ' --test_expect_failure 'remove redundant packs and pass fsck' ' +-test_expect_failure 'remove redundant packs and pass fsck (failed on Mac)' ' +test_expect_success 'remove redundant packs and pass fsck' ' - git pack-redundant --all | xargs rm && - git fsck --no-progress && - git pack-redundant --all >out && + ( + cd "$master_repo" && + git pack-redundant --all | xargs rm && @@ - printf "../../master.git/objects" >objects/info/alternates + ) ' --test_expect_failure 'no redundant packs without --alt-odb' ' +-test_expect_failure 'no redundant packs without --alt-odb (failed on Mac)' ' +test_expect_success 'no redundant packs without --alt-odb' ' - git pack-redundant --all >out && - test_must_be_empty out - ' + ( + cd "$shared_repo" && + git pack-redundant --all >out && @@ - P7:$P7 - EOF - --test_expect_failure 'pack-redundant --verbose: show duplicate packs in stderr' ' + # ALL | x x x x x x x x x x x x x x x x x x x + # + ############################################################################# +-test_expect_failure 'pack-redundant --verbose: show duplicate packs in stderr (failed on Mac)' ' +test_expect_success 'pack-redundant --verbose: show duplicate packs in stderr' ' - git pack-redundant --all --verbose >out 2>out.err && - test_must_be_empty out && - grep "pack$" out.err | format_packfiles >actual && + ( + cd "$shared_repo" && + cat >expect <<-EOF && Jiang Xin (4): t5323: test cases for git-pack-redundant pack-redundant: delay creation of unique_objects pack-redundant: rename pack_list.all_objects pack-redundant: consistent sort method Sun Chao (2): pack-redundant: delete redundant code pack-redundant: new algorithm to find min packs builtin/pack-redundant.c | 232 +++++++---------- t/t5323-pack-redundant.sh | 510 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 602 insertions(+), 140 deletions(-) create mode 100755 t/t5323-pack-redundant.sh -- 2.20.1.103.ged0fc2ca7b