builtin/repack.c: implement `--expire-to` for storing pruned objects

When pruning objects with `--cruft`, `git repack` offers some
flexibility when selecting the set of which objects are pruned via the
`--cruft-expiration` option.

This is useful for expiring objects which are older than the grace
period, making races where to-be-pruned objects become reachable and
then ancestors of freshly pushed objects, leaving the repository in a
corrupt state after pruning substantially less likely [1].

But in practice, such races are impossible to avoid entirely, no matter
how long the grace period is. To prevent this race, it is often
advisable to temporarily put a repository into a read-only state. But in
practice, this is not always practical, and so some middle ground would
be nice.

This patch introduces a new option, `--expire-to`, which teaches `git
repack` to write an additional cruft pack containing just the objects
which were pruned from the repository. The caller can specify a
directory outside of the current repository as the destination for this
second cruft pack.

This makes it possible to prune objects from a repository, while still
holding onto a supplemental copy of them outside of the original
repository. Having this copy on-disk makes it substantially easier to
recover objects when the aforementioned race is encountered.

`--expire-to` is implemented in a somewhat convoluted manner, which is
to take advantage of the fact that the first time `write_cruft_pack()`
is called, it adds the name of the cruft pack to the `names` string
list. That means the second time we call `write_cruft_pack()`, objects
in the previously-written cruft pack will be excluded.

As long as the caller ensures that no objects are expired during the
second pass, this is sufficient to generate a cruft pack containing all
objects which don't appear in any of the new packs written by `git
repack`, including the cruft pack. In other words, all of the objects
which are about to be pruned from the repository.

It is important to note that the destination in `--expire-to` does not
necessarily need to be a Git repository (though it can be) Notably, the
expired packs do not contain all ancestors of expired objects. So if the
source repository contains something like:

              <unreachable>
             /
    C1 --- C2
      \
       refs/heads/master

where C2 is unreachable, but has a parent (C1) which is reachable, and
C2 would be pruned, then the expiry pack will contain only C2, not C1.

[1]: https://lore.kernel.org/git/20190319001829.GL29661@sigill.intra.peff.net/

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
index 0bf13893d8147d872dbedeed9def497b2a5a0fea..4017157949e6d764a619f870c0eed538d3e99f64 100644 (file)
--- a/Documentation/git-repack.txt
+++ b/Documentation/git-repack.txt
@@ -74,6 +74,12 @@ to the new separate pack will be written.
        immediately instead of waiting for the next `git gc` invocation.
        Only useful with `--cruft -d`.
 
+--expire-to=<dir>::
+       Write a cruft pack containing pruned objects (if any) to the
+       directory `<dir>`. This option is useful for keeping a copy of
+       any pruned objects in a separate directory as a backup. Only
+       useful with `--cruft -d`.
+
 -l::
        Pass the `--local` option to 'git pack-objects'. See
        linkgit:git-pack-objects[1].

diff --git a/builtin/repack.c b/builtin/repack.c
index 432fa3f677f1313d938e1b8d88dd1da05d1c2718..a70962ee038b2c4c40aec3fd474089451152c5bb 100644 (file)
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -702,6 +702,10 @@ static int write_cruft_pack(const struct pack_objects_args *args,
         * By the time it is read here, it contains only the pack(s)
         * that were just written, which is exactly the set of packs we
         * want to consider kept.
+        *
+        * If `--expire-to` is given, the double-use served by `names`
+        * ensures that the pack written to `--expire-to` excludes any
+        * objects contained in the cruft pack.
         */
        in = xfdopen(cmd.in, "w");
        for_each_string_list_item(item, names)
@@ -755,6 +759,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
        int geometric_factor = 0;
        int write_midx = 0;
        const char *cruft_expiration = NULL;
+       const char *expire_to = NULL;
 
        struct option builtin_repack_options[] = {
                OPT_BIT('a', NULL, &pack_everything,
@@ -804,6 +809,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
                            N_("find a geometric progression with factor <N>")),
                OPT_BOOL('m', "write-midx", &write_midx,
                           N_("write a multi-pack index of the resulting packs")),
+               OPT_STRING(0, "expire-to", &expire_to, N_("dir"),
+                          N_("pack prefix to store a pack containing pruned objects")),
                OPT_END()
        };
 
@@ -1000,6 +1007,39 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
                                       &existing_kept_packs);
                if (ret)
                        return ret;
+
+               if (delete_redundant && expire_to) {
+                       /*
+                        * If `--expire-to` is given with `-d`, it's possible
+                        * that we're about to prune some objects. With cruft
+                        * packs, pruning is implicit: any objects from existing
+                        * packs that weren't picked up by new packs are removed
+                        * when their packs are deleted.
+                        *
+                        * Generate an additional cruft pack, with one twist:
+                        * `names` now includes the name of the cruft pack
+                        * written in the previous step. So the contents of
+                        * _this_ cruft pack exclude everything contained in the
+                        * existing cruft pack (that is, all of the unreachable
+                        * objects which are no older than
+                        * `--cruft-expiration`).
+                        *
+                        * To make this work, cruft_expiration must become NULL
+                        * so that this cruft pack doesn't actually prune any
+                        * objects. If it were non-NULL, this call would always
+                        * generate an empty pack (since every object not in the
+                        * cruft pack generated above will have an mtime older
+                        * than the expiration).
+                        */
+                       ret = write_cruft_pack(&cruft_po_args, expire_to,
+                                              pack_prefix,
+                                              NULL,
+                                              &names,
+                                              &existing_nonkept_packs,
+                                              &existing_kept_packs);
+                       if (ret)
+                               return ret;
+               }
        }
 
        string_list_sort(&names);

diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index ca45c4cd2c1a8a422e396fd4d29dee8c49c0c10e..17ee6fc2cc022bd8fc2e2138d652f12fe7ee4a31 100755 (executable)
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -482,4 +482,125 @@ test_expect_success '-n overrides repack.updateServerInfo=true' '
        test_server_info_missing
 '
 
+test_expect_success '--expire-to stores pruned objects (now)' '
+       git init expire-to-now &&
+       (
+               cd expire-to-now &&
+
+               git branch -M main &&
+
+               test_commit base &&
+
+               git checkout -b cruft &&
+               test_commit --no-tag cruft &&
+
+               git rev-list --objects --no-object-names main..cruft >moved.raw &&
+               sort moved.raw >moved.want &&
+
+               git rev-list --all --objects --no-object-names >expect.raw &&
+               sort expect.raw >expect &&
+
+               git checkout main &&
+               git branch -D cruft &&
+               git reflog expire --all --expire=all &&
+
+               git init --bare expired.git &&
+               git repack -d \
+                       --cruft --cruft-expiration="now" \
+                       --expire-to="expired.git/objects/pack/pack" &&
+
+               expired="$(ls expired.git/objects/pack/pack-*.idx)" &&
+               test_path_is_file "${expired%.idx}.mtimes" &&
+
+               # Since the `--cruft-expiration` is "now", the effective
+               # behavior is to move _all_ unreachable objects out to
+               # the location in `--expire-to`.
+               git show-index <$expired >expired.raw &&
+               cut -d" " -f2 expired.raw | sort >expired.objects &&
+               git rev-list --all --objects --no-object-names \
+                       >remaining.objects &&
+
+               # ...in other words, the combined contents of this
+               # repository and expired.git should be the same as the
+               # set of objects we started with.
+               cat expired.objects remaining.objects | sort >actual &&
+               test_cmp expect actual &&
+
+               # The "moved" objects (i.e., those in expired.git)
+               # should be the same as the cruft objects which were
+               # expired in the previous step.
+               test_cmp moved.want expired.objects
+       )
+'
+
+test_expect_success '--expire-to stores pruned objects (5.minutes.ago)' '
+       git init expire-to-5.minutes.ago &&
+       (
+               cd expire-to-5.minutes.ago &&
+
+               git branch -M main &&
+
+               test_commit base &&
+
+               # Create two classes of unreachable objects, one which
+               # is older than 5 minutes (stale), and another which is
+               # newer (recent).
+               for kind in stale recent
+               do
+                       git checkout -b $kind main &&
+                       test_commit --no-tag $kind || return 1
+               done &&
+
+               git rev-list --objects --no-object-names main..stale >in &&
+               stale="$(git pack-objects $objdir/pack/pack <in)" &&
+               mtime="$(test-tool chmtime --get =-600 $objdir/pack/pack-$stale.pack)" &&
+
+               # expect holds the set of objects we expect to find in
+               # this repository after repacking
+               git rev-list --objects --no-object-names recent >expect.raw &&
+               sort expect.raw >expect &&
+
+               # moved.want holds the set of objects we expect to find
+               # in expired.git
+               git rev-list --objects --no-object-names main..stale >out &&
+               sort out >moved.want &&
+
+               git checkout main &&
+               git branch -D stale recent &&
+               git reflog expire --all --expire=all &&
+               git prune-packed &&
+
+               git init --bare expired.git &&
+               git repack -d \
+                       --cruft --cruft-expiration=5.minutes.ago \
+                       --expire-to="expired.git/objects/pack/pack" &&
+
+               # Some of the remaining objects in this repository are
+               # unreachable, so use `cat-file --batch-all-objects`
+               # instead of `rev-list` to get their names
+               git cat-file --batch-all-objects --batch-check="%(objectname)" \
+                       >remaining.objects &&
+               sort remaining.objects >actual &&
+               test_cmp expect actual &&
+
+               (
+                       cd expired.git &&
+
+                       expired="$(ls objects/pack/pack-*.mtimes)" &&
+                       test-tool pack-mtimes $(basename $expired) >out &&
+                       cut -d" " -f1 out | sort >../moved.got &&
+
+                       # Ensure that there are as many objects with the
+                       # expected mtime as were moved to expired.git.
+                       #
+                       # In other words, ensure that the recorded
+                       # mtimes of any moved objects was written
+                       # correctly.
+                       grep " $mtime$" out >matching &&
+                       test_line_count = $(wc -l <../moved.want) matching
+               ) &&
+               test_cmp moved.want moved.got
+       )
+'
+
 test_done