Merge branch 'tb/midx-repack-ignore-cruft-packs'

"git multi-pack-index repack/expire" used to repack unreachable
cruft into a new pack, which have been corrected.

* tb/midx-repack-ignore-cruft-packs:
  midx.c: avoid cruft packs with non-zero `repack --batch-size`
  midx.c: remove unnecessary loop condition
  midx.c: replace `xcalloc()` with `CALLOC_ARRAY()`
  midx.c: avoid cruft packs with `repack --batch-size=0`
  midx.c: prevent `expire` from removing the cruft pack
  Documentation/git-multi-pack-index.txt: clarify expire behavior
  Documentation/git-multi-pack-index.txt: fix typo

diff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt
index a48c3d5ea6301abcdda5df021ac8442ed149f23c..3696506eb355e1b11a447a7014dcca370b760555 100644 (file)
--- a/Documentation/git-multi-pack-index.txt
+++ b/Documentation/git-multi-pack-index.txt
@@ -70,9 +70,10 @@ verify::
        Verify the contents of the MIDX file.
 
 expire::
-       Delete the pack-files that are tracked  by the MIDX file, but
-       have no objects referenced by the MIDX. Rewrite the MIDX file
-       afterward to remove all references to these pack-files.
+       Delete the pack-files that are tracked by the MIDX file, but
+       have no objects referenced by the MIDX (with the exception of
+       `.keep` packs and cruft packs). Rewrite the MIDX file afterward
+       to remove all references to these pack-files.
 
 repack::
        Create a new pack-file containing objects in small pack-files

diff --git a/midx.c b/midx.c
index c27d0e5f1510c32ad7cbf0fca8dc1d05ec4917a3..3a8dcfe98e2e937c23d80db3e27fa89ccd36308b 100644 (file)
--- a/midx.c
+++ b/midx.c
@@ -1839,7 +1839,7 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla
                if (prepare_midx_pack(r, m, i))
                        continue;
 
-               if (m->packs[i]->pack_keep)
+               if (m->packs[i]->pack_keep || m->packs[i]->is_cruft)
                        continue;
 
                pack_name = xstrdup(m->packs[i]->pack_name);
@@ -1895,6 +1895,8 @@ static int fill_included_packs_all(struct repository *r,
                        continue;
                if (!pack_kept_objects && m->packs[i]->pack_keep)
                        continue;
+               if (m->packs[i]->is_cruft)
+                       continue;
 
                include_pack[i] = 1;
                count++;
@@ -1910,9 +1912,11 @@ static int fill_included_packs_batch(struct repository *r,
 {
        uint32_t i, packs_to_repack;
        size_t total_size;
-       struct repack_info *pack_info = xcalloc(m->num_packs, sizeof(struct repack_info));
+       struct repack_info *pack_info;
        int pack_kept_objects = 0;
 
+       CALLOC_ARRAY(pack_info, m->num_packs);
+
        repo_config_get_bool(r, "repack.packkeptobjects", &pack_kept_objects);
 
        for (i = 0; i < m->num_packs; i++) {
@@ -1924,7 +1928,7 @@ static int fill_included_packs_batch(struct repository *r,
                pack_info[i].mtime = m->packs[i]->mtime;
        }
 
-       for (i = 0; batch_size && i < m->num_objects; i++) {
+       for (i = 0; i < m->num_objects; i++) {
                uint32_t pack_int_id = nth_midxed_pack_int_id(m, i);
                pack_info[pack_int_id].referenced_objects++;
        }
@@ -1942,6 +1946,8 @@ static int fill_included_packs_batch(struct repository *r,
                        continue;
                if (!pack_kept_objects && p->pack_keep)
                        continue;
+               if (p->is_cruft)
+                       continue;
                if (open_pack_index(p) || !p->num_objects)
                        continue;
 

diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index afbe93f162e854b462774cd5242ddfe40a64bf6c..b5f9b1092229d3d8c1879d41c5c3f02f4ad04923 100755 (executable)
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -784,6 +784,70 @@ test_expect_success 'repack creates a new pack' '
        )
 '
 
+test_expect_success 'repack (all) ignores cruft pack' '
+       git init repo &&
+       test_when_finished "rm -fr repo" &&
+       (
+               cd repo &&
+
+               test_commit base &&
+               test_commit --no-tag unreachable &&
+
+               git reset --hard base &&
+               git reflog expire --all --expire=all &&
+               git repack --cruft -d &&
+
+               git multi-pack-index write &&
+
+               find $objdir/pack | sort >before &&
+               git multi-pack-index repack --batch-size=0 &&
+               find $objdir/pack | sort >after &&
+
+               test_cmp before after
+       )
+'
+
+test_expect_success 'repack (--batch-size) ignores cruft pack' '
+       git init repo &&
+       test_when_finished "rm -fr repo" &&
+       (
+               cd repo &&
+
+               test_commit_bulk 5 &&
+               test_commit --no-tag unreachable &&
+
+               git reset --hard HEAD^ &&
+               git reflog expire --all --expire=all &&
+               git repack --cruft -d &&
+
+               test_commit four &&
+
+               find $objdir/pack -type f -name "*.pack" | sort >before &&
+               git repack -d &&
+               find $objdir/pack -type f -name "*.pack" | sort >after &&
+
+               pack="$(comm -13 before after)" &&
+               test_file_size "$pack" >sz &&
+               # Set --batch-size to twice the size of the pack created
+               # in the previous step, since this is enough to
+               # accommodate it and the cruft pack.
+               #
+               # This means that the MIDX machinery *could* combine the
+               # new and cruft packs together.
+               #
+               # We ensure that it does not below.
+               batch="$((($(cat sz) * 2)))" &&
+
+               git multi-pack-index write &&
+
+               find $objdir/pack | sort >before &&
+               git multi-pack-index repack --batch-size=$batch &&
+               find $objdir/pack | sort >after &&
+
+               test_cmp before after
+       )
+'
+
 test_expect_success 'expire removes repacked packs' '
        (
                cd dup &&
@@ -847,6 +911,36 @@ test_expect_success 'expire respects .keep files' '
        )
 '
 
+test_expect_success 'expiring unreferenced cruft pack retains pack' '
+       git init repo &&
+       test_when_finished "rm -fr repo" &&
+       (
+               cd repo &&
+
+               test_commit base &&
+               test_commit --no-tag unreachable &&
+               unreachable=$(git rev-parse HEAD) &&
+
+               git reset --hard base &&
+               git reflog expire --all --expire=all &&
+               git repack --cruft -d &&
+               mtimes="$(ls $objdir/pack/pack-*.mtimes)" &&
+
+               echo "base..$unreachable" >in &&
+               pack="$(git pack-objects --revs --delta-base-offset \
+                       $objdir/pack/pack <in)" &&
+
+               # Preferring the contents of "$pack" will leave the
+               # cruft pack unreferenced (ie., none of the objects
+               # contained in the cruft pack will have their MIDX copy
+               # selected from the cruft pack).
+               git multi-pack-index write --preferred-pack="pack-$pack.pack" &&
+               git multi-pack-index expire &&
+
+               test_path_is_file "$mtimes"
+       )
+'
+
 test_expect_success 'repack --batch-size=0 repacks everything' '
        cp -r dup dup2 &&
        (