From 8cce2b6eb55946c89bd342424a0b1d093850f592 Mon Sep 17 00:00:00 2001 From: Qin ShiCheng Date: Tue, 8 Sep 2026 22:04:39 +0800 Subject: [PATCH 1/3] pack-objects: reset kept-pack cache for cruft walk When writing a cruft pack with an expiration, pack-objects first collects the recent objects and then walks from them to rescue whatever they reach, expired or not. A pack the caller did not list is marked kept while collecting, so that its objects are not copied into the cruft pack, and unmarked before the walk, so that the walk can go through it. The walk does not see the unmarking. Whether an object sits in a kept pack is answered from a cache that is built on first use and only dropped when asked about a different kind of kept pack. Collecting builds it while the unlisted pack is still marked, the walk asks the same kind of question, and so the unlisted pack stays in it: the walk stops there, and whatever lies beyond it in an expired pack is left out of the cruft pack, to go when that pack is deleted. Whether that happens depends on whether the question changes along the way, which "--honor-pack-keep" does whenever there is a ".keep" file around. Drop the cache after unmarking. Only packfile.c is to touch it, so add repo_invalidate_kept_pack_caches() there, which drops it for every object directory. With the cache gone, the walk goes through every pack but the listed fresh ones, as was intended, including packs borrowed from an alternate. It has to: an object there may refer to a local one, for example when two repositories borrow from each other, or when "git repack --filter-to" has moved some objects to an alternate and left the ones they refer to behind. Without "--local", whatever the walk finds there is copied into the cruft pack, as it already is for loose objects of an alternate; with "--local", nothing is. Either way, the walk may now go through a history far bigger than the repository's own, as it already did whenever there was a ".keep" file around. It also includes packs given with "--keep-pack", whose objects are to be left out. Turn those into "open" kept packs, as "--stdin-packs=follow" does for its "!" packs: the walk goes through them, and their objects are still left out. This is how an unlisted pack with a ".keep" file is treated under "--honor-pack-keep". With "--keep-pack", the walk and the check that leaves the objects of such a pack out ask about different kinds of kept packs, so the cache is rebuilt each time they take turns, as already happens under "--honor-pack-keep" whenever there is a ".keep" file around. repack never gets here with "--keep-pack": it lists the packs it keeps as fresh ones instead. The tests walk from a recent commit to an expired one through a pack pack-objects is not told about, through one given with "--keep-pack", and through one of an alternate holding trees whose blobs "git repack --filter-to" left behind, with and without "--local". Signed-off-by: Qin ShiCheng --- builtin/pack-objects.c | 18 ++++++- odb/source-packed.h | 3 +- packfile.c | 18 ++++++- packfile.h | 7 +++ t/t5329-pack-objects-cruft.sh | 98 +++++++++++++++++++++++++++++++++++ 5 files changed, 140 insertions(+), 4 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index ca3a891dfbce3e..5160e76842b544 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -4294,10 +4294,18 @@ static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs /* * Re-mark only the fresh packs as kept so that objects in * unknown packs do not halt the reachability traversal early. + * A pack given with --keep-pack must not halt it either, but as an + * "open" kept pack its objects are still left out. The kept-pack + * cache was built while the packs were still marked, so drop it + * too. */ - repo_for_each_pack(the_repository, p) + repo_for_each_pack(the_repository, p) { p->pack_keep_in_core = 0; + if (p->pack_keep_in_core_open) + ignore_packed_keep_in_core_open = 1; + } mark_pack_kept_in_core(fresh_packs, 1); + repo_invalidate_kept_pack_caches(the_repository); if (prepare_revision_walk(&revs)) die(_("revision walk setup failed")); @@ -4343,6 +4351,14 @@ static void read_cruft_objects(void) if (item) { item->util = p; + } else if (p->pack_keep_in_core) { + /* + * Only --keep-pack marks a pack as kept this early. + * Make it an "open" kept pack as well, so that its + * objects are still left out once it is unmarked + * before the traversal; until then this has no effect. + */ + p->pack_keep_in_core_open = 1; } else { /* * This pack wasn't mentioned in either the "fresh" or diff --git a/odb/source-packed.h b/odb/source-packed.h index a0f6b5096dcd0f..c8c6088a699e8d 100644 --- a/odb/source-packed.h +++ b/odb/source-packed.h @@ -25,7 +25,8 @@ struct odb_source_packed { * Should not be accessed directly, but via * `packfile_store_get_kept_pack_cache()`. The list of packs gets * invalidated when the stored flags and the flags passed to - * `packfile_store_get_kept_pack_cache()` mismatch. + * `packfile_store_get_kept_pack_cache()` mismatch, or explicitly via + * `repo_invalidate_kept_pack_caches()`. */ struct { struct packed_git **packs; diff --git a/packfile.c b/packfile.c index b32b4cd75dad31..213f930edb584b 100644 --- a/packfile.c +++ b/packfile.c @@ -1881,6 +1881,21 @@ int packfile_fill_entry(struct packed_git *p, return 1; } +static void invalidate_kept_pack_cache(struct odb_source_packed *store) +{ + FREE_AND_NULL(store->kept_cache.packs); + store->kept_cache.flags = 0; +} + +void repo_invalidate_kept_pack_caches(struct repository *r) +{ + struct odb_source_files *files = + odb_source_files_downcast(r->objects->source); + + for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) + invalidate_kept_pack_cache(dir->packed); +} + static void maybe_invalidate_kept_pack_cache(struct odb_source_packed *store, unsigned flags) { @@ -1888,8 +1903,7 @@ static void maybe_invalidate_kept_pack_cache(struct odb_source_packed *store, return; if (store->kept_cache.flags == flags) return; - FREE_AND_NULL(store->kept_cache.packs); - store->kept_cache.flags = 0; + invalidate_kept_pack_cache(store); } struct packed_git **packfile_store_get_kept_pack_cache(struct odb_source_packed *store, diff --git a/packfile.h b/packfile.h index ed3378c45761dd..0ead9e2821c76b 100644 --- a/packfile.h +++ b/packfile.h @@ -143,6 +143,13 @@ enum kept_pack_type { struct packed_git **packfile_store_get_kept_pack_cache(struct odb_source_packed *store, unsigned flags); +/* + * Drop every packfile store's cache of kept packs, so that the next call + * to `packfile_store_get_kept_pack_cache()` rebuilds it, e.g. after + * changing which packs are kept in core. + */ +void repo_invalidate_kept_pack_caches(struct repository *r); + struct pack_window { struct pack_window *next; unsigned char *base; diff --git a/t/t5329-pack-objects-cruft.sh b/t/t5329-pack-objects-cruft.sh index 12cda063730372..4a1056e494379d 100755 --- a/t/t5329-pack-objects-cruft.sh +++ b/t/t5329-pack-objects-cruft.sh @@ -332,6 +332,104 @@ test_expect_success 'cruft trees rescue sub-trees, blobs' ' ) ' +# Build the history "packed", "old", "mid", "new" in the current repository: +# "packed" is in the pack named in "in", "old" has expired, "mid" is in the +# pack whose hash is in $mid and "new" is recent. The objects of packed..new +# end up in "all.objects". +cruft_chain () { + test_commit packed && + git repack -Ad && + basename "$(ls $packdir/pack-*.pack)" >in && + + test_commit old && + test_commit mid && + test_commit new && + + git rev-list --objects --no-object-names packed..old >old && + while read object + do + test-tool chmtime -1000 \ + "$objdir/$(test_oid_to_path $object)" || return 1 + done mid.objects && + mid=$(git pack-objects $packdir/pack all.objects +} + +test_expect_success 'cruft traversal goes through a pack it was not told about' ' + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + cruft_chain && + + # "old" can only be rescued from "new" by walking through + # the "mid" pack. + cruft=$(git pack-objects --cruft \ + --cruft-expiration=750.seconds.ago $packdir/pack actual.raw && + cut -d" " -f1 actual && + sort expect && + test_cmp expect actual + ) +' + +test_expect_success 'cruft traversal goes through a --keep-pack pack' ' + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + cruft_chain && + + # The same, but with "mid" in a pack given with --keep-pack: + # "old" is still rescued, and none of the objects of that + # pack are copied. + cruft=$(git pack-objects --cruft \ + --cruft-expiration=750.seconds.ago \ + --keep-pack=pack-$mid.pack $packdir/pack actual.raw && + cut -d" " -f1 actual && + sort all.sorted && + sort mid.sorted && + comm -23 all.sorted mid.sorted >expect && + test_cmp expect actual + ) +' + +for opt in "" --local +do + test_expect_success "cruft traversal goes through an alternate${opt:+ ($opt)}" ' + test_when_finished "rm -fr repo alt.git" && + git init repo && + git init --bare alt.git && + echo "$(pwd)/alt.git/objects" >repo/.git/objects/info/alternates && + test_commit -C repo base && + test_commit -C repo old && + tree=$(git -C repo rev-parse old^{tree}) && + blob=$(git -C repo rev-parse old:old.t) && + + # Move everything but the blobs to the alternate, then let + # "old" expire: its tree is in the alternate, but its blob + # is in an old local pack. + git -C repo repack -a -d && + git -C repo repack -a -d --filter=object:type=blob \ + --filter-to="$(pwd)/alt.git/objects/pack/pack" && + git -C repo tag -d old && + git -C repo reset --hard base && + git -C repo reflog expire --all --expire=all && + test-tool chmtime -10000 repo/$packdir/pack-*.pack && + + # A recent unreachable commit in "repo" keeps the blob, but only + # if the traversal goes through the tree in the alternate, which + # it has to do even with --local. + outer=$(printf "040000 tree %s\tdir\n" $tree | git -C repo mktree) && + git -C repo commit-tree -m recent $outer >/dev/null && + git -C repo repack --cruft --cruft-expiration=1.hour.ago -d $opt && + git -C repo cat-file -e $blob + ' +done + test_expect_success 'expired objects are pruned' ' git init repo && test_when_finished "rm -fr repo" && From 91ffbdf4fa9beb215f92819c4c6d0ad2640a0c71 Mon Sep 17 00:00:00 2001 From: Qin ShiCheng Date: Wed, 21 Oct 2026 12:36:20 +0800 Subject: [PATCH 2/3] pack-objects: add --ignore-pack-keep-from repack decides which packs to delete before it runs pack-objects, but pack-objects looks for ".keep" files on its own when it opens the packs. A ".keep" that appears in between, for example from a push or fetch whose index-pack wrote a pack identical to an existing one, makes pack-objects treat as kept a pack that repack is about to delete. Its objects are then left out of the new pack, or not loosened, or not kept as unreachable, and they are gone once repack deletes the pack. Not all of that goes through "--honor-pack-keep": loosening objects for "--unpack-unreachable", and collecting them for "--keep-unreachable" and "--cruft", skip packs with a ".keep" file regardless of it. Let the caller name such packs. "--ignore-pack-keep-from=" reads names of local packs, one per line, given and matched as for "--keep-pack", and treats them as if they had no ".keep" file. All of the above go by p->pack_keep, which is read from the disk once, when a pack is first seen, so clearing it for the named packs before anything looks at it covers every one of them. Other packs, including any that show up later, keep whatever their ".keep" files say. The names come from a file because there can be more of them than fit on a command line (on Windows, the whole command line is limited to 32767 characters), and the standard input is already taken in every mode repack uses. Signed-off-by: Qin ShiCheng --- Documentation/git-pack-objects.adoc | 12 ++++++++++ builtin/pack-objects.c | 37 +++++++++++++++++++++++++++++ t/t7700-repack.sh | 37 +++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+) diff --git a/Documentation/git-pack-objects.adoc b/Documentation/git-pack-objects.adoc index ccad938c5b9f1b..43fb206ef0e818 100644 --- a/Documentation/git-pack-objects.adoc +++ b/Documentation/git-pack-objects.adoc @@ -13,6 +13,7 @@ SYNOPSIS [--no-reuse-delta] [--delta-base-offset] [--non-empty] [--local] [--incremental] [--window=] [--depth=] [--revs [--unpacked | --all]] [--keep-pack=] + [--ignore-pack-keep-from=] [--cruft] [--cruft-expiration=