Merge branch 'qs/repack-kept-pack-fix' into seen

The behavior of repack and pack-objects has been updated to avoid
losing objects to a '.keep' file that appears mid-run.  Instead of
pack-objects looking for '.keep' files independently, repack now
hands it a snapshot of kept packs via a new --keep-pack-from-file
option.

* qs/repack-kept-pack-fix:
  repack: tell pack-objects which packs are kept
  pack-objects: add --keep-pack-from-file
  pack-objects: sort --keep-pack list for lookup
  pack-objects: reset kept-pack cache for cruft walk
  pack-objects: keep --keep-pack open when following
seen
Junio C Hamano 2026-10-12 09:26:16 -07:00
commit 7a64283cb2
13 changed files with 390 additions and 23 deletions

View File

@ -13,6 +13,7 @@ SYNOPSIS
[--no-reuse-delta] [--delta-base-offset] [--non-empty]
[--local] [--incremental] [--window=<n>] [--depth=<n>]
[--revs [--unpacked | --all]] [--keep-pack=<pack-name>]
[--keep-pack-from-file=<file>]
[--cruft] [--cruft-expiration=<time>]
[--stdout [--filter=<filter-spec>] | <base-name>]
[--shallow] [--keep-true-parents] [--[no-]sparse]
@ -195,6 +196,13 @@ depth is 4095.
leading directory (e.g. `pack-123.pack`). The option could be
specified multiple times to keep multiple packs.

--keep-pack-from-file=<file>::
Read names of packs to keep from `<file>`, one per line, and
treat each of them as if it had been given with `--keep-pack`.
Empty lines are ignored. This is meant for callers such as
linkgit:git-repack[1] that may have to name more packs than fit
on a command line.

--incremental::
This flag causes an object already in a pack to be ignored
even if it would have otherwise been packed.

View File

@ -194,6 +194,7 @@ static const char *const pack_usage[] = {
" [--no-reuse-delta] [--delta-base-offset] [--non-empty]\n"
" [--local] [--incremental] [--window=<n>] [--depth=<n>]\n"
" [--revs [--unpacked | --all]] [--keep-pack=<pack-name>]\n"
" [--keep-pack-from-file=<file>]\n"
" [--cruft] [--cruft-expiration=<time>]\n"
" [--stdout [--filter=<filter-spec>] | <base-name>]\n"
" [--shallow] [--keep-true-parents] [--[no-]sparse]\n"
@ -4338,10 +4339,13 @@ 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.
* The kept-pack cache was built while those packs were still
* marked, so drop it too.
*/
repo_for_each_pack(the_repository, p)
p->pack_keep_in_core = 0;
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"));
@ -5039,27 +5043,54 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)
oid_array_clear(&recent_objects);
}

static void add_extra_kept_packs(const struct string_list *names)
/*
* Read pack names from the file, one per line, as if each of them had
* been given with "--keep-pack".
*/
static void read_keep_pack_list(struct string_list *names, const char *path)
{
struct strbuf buf = STRBUF_INIT;
FILE *fp = xfopen(path, "r");

while (strbuf_getline(&buf, fp) != EOF) {
if (!buf.len)
continue;
string_list_append(names, buf.buf);
}
if (ferror(fp))
die_errno(_("could not read '%s'"), path);
fclose(fp);
strbuf_release(&buf);
}

static void add_extra_kept_packs(struct string_list *names,
enum stdin_packs_mode stdin_packs)
{
struct packed_git *p;

if (!names->nr)
return;

repo_for_each_pack(the_repository, p) {
const char *name = basename(p->pack_name);
int i;
string_list_sort(names);

repo_for_each_pack(the_repository, p) {
if (!p->pack_local)
continue;

for (i = 0; i < names->nr; i++)
if (!fspathcmp(name, names->items[i].string))
break;

if (i < names->nr) {
p->pack_keep_in_core = 1;
ignore_packed_keep_in_core = 1;
if (string_list_has_string(names, basename(p->pack_name))) {
/*
* When following, treat the pack like a "!" pack, not
* a "^" one: nobody said it is closed under
* reachability, so the traversal must be able to go
* through it.
*/
if (stdin_packs == STDIN_PACKS_MODE_FOLLOW) {
p->pack_keep_in_core_open = 1;
ignore_packed_keep_in_core_open = 1;
} else {
p->pack_keep_in_core = 1;
ignore_packed_keep_in_core = 1;
}
continue;
}
}
@ -5171,7 +5202,11 @@ int cmd_pack_objects(int argc,
int rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0;
int rev_list_index = 0;
enum stdin_packs_mode stdin_packs = STDIN_PACKS_MODE_NONE;
struct string_list keep_pack_list = STRING_LIST_INIT_NODUP;
struct string_list keep_pack_list = {
.strdup_strings = 1,
.cmp = fspathcmp,
};
char *keep_pack_from_file = NULL;
struct list_objects_filter_options filter_options =
LIST_OBJECTS_FILTER_INIT;
struct repo_config_values *cfg = repo_config_values(the_repository);
@ -5256,6 +5291,8 @@ int cmd_pack_objects(int argc,
N_("ignore packs that have companion .keep file")),
OPT_STRING_LIST(0, "keep-pack", &keep_pack_list, N_("name"),
N_("ignore this pack")),
OPT_FILENAME(0, "keep-pack-from-file", &keep_pack_from_file,
N_("ignore the packs named in <file>")),
OPT_INTEGER(0, "compression", &cfg->pack_compression_level,
N_("pack compression level")),
OPT_BOOL(0, "keep-true-parents", &grafts_keep_true_parents,
@ -5483,7 +5520,9 @@ int cmd_pack_objects(int argc,
if (progress && all_progress_implied)
progress = 2;

add_extra_kept_packs(&keep_pack_list);
if (keep_pack_from_file)
read_keep_pack_list(&keep_pack_list, keep_pack_from_file);
add_extra_kept_packs(&keep_pack_list, stdin_packs);
if (ignore_packed_keep_on_disk) {
struct packed_git *p;

@ -5577,6 +5616,7 @@ cleanup:
clear_packing_data(&to_pack);
list_objects_filter_release(&filter_options);
string_list_clear(&keep_pack_list, 0);
free(keep_pack_from_file);
strvec_clear(&rp);

return 0;

View File

@ -168,6 +168,7 @@ int cmd_repack(int argc,
struct oidset drop_oids = OIDSET_INIT;
struct pack_geometry geometry = { 0 };
struct tempfile *refs_snapshot = NULL;
struct tempfile *kept_packs_snapshot = NULL;
int i, ret;
int show_progress;

@ -466,6 +467,19 @@ int cmd_repack(int argc,
keep_pack_list.cmp = fspathcmp;
string_list_sort(&keep_pack_list);
existing_packs_collect(&existing, &keep_pack_list);
if (existing.kept_packs.nr) {
struct strbuf path = STRBUF_INIT;

strbuf_addf(&path, "%s/%s_XXXXXX",
repo_get_object_directory(repo), "kept-packs");

kept_packs_snapshot = xmks_tempfile(path.buf);
existing_packs_snapshot_kept(&existing, kept_packs_snapshot);
po_args.kept_packs_snapshot =
get_tempfile_path(kept_packs_snapshot);

strbuf_release(&path);
}

if (geometry.split_factor) {
if (pack_everything)
@ -685,6 +699,7 @@ int cmd_repack(int argc,
cruft_po_args.quiet = po_args.quiet;
cruft_po_args.delta_base_offset = po_args.delta_base_offset;
cruft_po_args.pack_kept_objects = 0;
cruft_po_args.kept_packs_snapshot = po_args.kept_packs_snapshot;

ret = write_cruft_pack(&opts, cruft_expiration,
combine_cruft_below_size, &names,

View File

@ -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;

View File

@ -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,

View File

@ -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;

View File

@ -25,9 +25,6 @@ int write_filtered_pack(const struct write_pack_opts *opts,

strvec_push(&cmd.args, "--stdin-packs");

for_each_string_list_item(item, &existing->kept_packs)
strvec_pushf(&cmd.args, "--keep-pack=%s", item->string);

cmd.in = -1;

ret = start_command(&cmd);

View File

@ -37,8 +37,9 @@ void prepare_pack_objects(struct child_process *cmd,
strvec_push(&cmd->args, "--quiet");
if (args->delta_base_offset)
strvec_push(&cmd->args, "--delta-base-offset");
if (!args->pack_kept_objects)
strvec_push(&cmd->args, "--honor-pack-keep");
if (!args->pack_kept_objects && args->kept_packs_snapshot)
strvec_pushf(&cmd->args, "--keep-pack-from-file=%s",
args->kept_packs_snapshot);
strvec_push(&cmd->args, out);
cmd->git_cmd = 1;
cmd->out = -1;
@ -161,6 +162,35 @@ void existing_packs_collect(struct existing_packs *existing,
strbuf_release(&buf);
}

void existing_packs_snapshot_kept(const struct existing_packs *existing,
struct tempfile *f)
{
struct string_list_item *item;
FILE *out = fdopen_tempfile(f, "w");

if (!out)
die(_("could not open tempfile %s for writing"),
get_tempfile_path(f));

for_each_string_list_item(item, &existing->kept_packs) {
/*
* A newline would split the name in two, and pack-objects
* quietly keeps whichever packs the halves happen to name.
*/
if (strchr(item->string, '\n'))
die(_("cannot keep pack '%s': its name contains a newline"),
item->string);
fprintf(out, "%s.pack\n", item->string);
}

if (close_tempfile_gently(f)) {
int save_errno = errno;
delete_tempfile(&f);
errno = save_errno;
die_errno(_("could not close kept packs snapshot tempfile"));
}
}

int existing_packs_has_non_kept(const struct existing_packs *existing)
{
return existing->non_kept_packs.nr || existing->cruft_packs.nr;

View File

@ -19,6 +19,14 @@ struct pack_objects_args {
int path_walk;
int delta_base_offset;
int pack_kept_objects;
/*
* File naming the packs to leave alone, one "<name>.pack" per line;
* NULL when there are none. pack-objects reads it rather than
* looking for ".keep" files itself, so that a ".keep" created or
* removed while we run cannot make the two of us disagree over
* which packs are being repacked.
*/
const char *kept_packs_snapshot;
struct list_objects_filter_options filter_options;
};

@ -28,6 +36,7 @@ struct pack_objects_args {
}

struct child_process;
struct tempfile;

void prepare_pack_objects(struct child_process *cmd,
const struct pack_objects_args *args,
@ -83,6 +92,12 @@ struct existing_packs {
*/
void existing_packs_collect(struct existing_packs *existing,
const struct string_list *extra_keep);
/*
* Writes the names of the kept packs, one "<name>.pack" per line, into
* the given tempfile, for pack-objects to read with --keep-pack-from-file.
*/
void existing_packs_snapshot_kept(const struct existing_packs *existing,
struct tempfile *f);
int existing_packs_has_non_kept(const struct existing_packs *existing);
int existing_pack_is_marked_for_deletion(struct string_list_item *item);
void existing_packs_retain_cruft(struct existing_packs *existing,
@ -142,8 +157,6 @@ void pack_geometry_remove_redundant(struct pack_geometry *geometry,
bool wrote_incremental_midx);
void pack_geometry_release(struct pack_geometry *geometry);

struct tempfile;

enum repack_write_midx_mode {
REPACK_WRITE_MIDX_NONE,
REPACK_WRITE_MIDX_DEFAULT,

View File

@ -332,6 +332,46 @@ test_expect_success 'cruft trees rescue sub-trees, blobs' '
)
'

test_expect_success 'cruft traversal rescues through a pack it was not told about' '
git init repo &&
test_when_finished "rm -fr repo" &&
(
cd repo &&

test_commit packed &&
git repack -Ad &&
keep="$(basename "$(ls $packdir/pack-*.pack)")" &&

test_commit old &&
test_commit mid &&
test_commit new &&

# "old" has expired, "new" is recent, and "mid" sits in a
# pack that pack-objects is not told about. Rescuing "old"
# from "new" means walking through that pack.
git rev-list --objects --no-object-names packed..old >old &&
while read object
do
test-tool chmtime -1000 \
"$objdir/$(test_oid_to_path $object)" || exit 1
done <old &&
git rev-list --objects --no-object-names old..mid |
git pack-objects $packdir/pack >/dev/null &&
git prune-packed &&

cruft="$(echo $keep | git pack-objects --cruft \
--cruft-expiration=750.seconds.ago \
$packdir/pack)" &&
test-tool pack-mtimes "pack-$cruft.mtimes" >actual.raw &&

cut -d" " -f1 <actual.raw | sort >actual &&
git rev-list --objects --no-object-names packed..new >expect.raw &&
sort <expect.raw >expect &&

test_cmp expect actual
)
'

test_expect_success 'expired objects are pruned' '
git init repo &&
test_when_finished "rm -fr repo" &&

View File

@ -483,6 +483,47 @@ test_expect_success '--stdin-packs=follow with open-excluded packs' '
)
'

test_expect_success '--stdin-packs=follow walks through a --keep-pack pack' '
test_when_finished "rm -fr repo" &&

git init repo &&
(
cd repo &&
git config set maintenance.auto false &&

test_commit A &&
test_commit B &&
test_commit C &&

A="$(echo A | git pack-objects --revs $packdir/pack)" &&
B="$(echo A..B | git pack-objects --revs $packdir/pack)" &&
C="$(echo B..C | git pack-objects --revs $packdir/pack)" &&
B_ONLY="$(git rev-parse B | git pack-objects $packdir/pack)" &&
git prune-packed &&

# Pack C is included and pack A is excluded and closed. The
# commit B is in the kept pack B_ONLY, but its tree and blob
# are only in pack B, which pack-objects is not told about.
# The kept pack keeps B out of the result, and the walk has
# to go through it to rescue the tree and the blob.
P=$(git pack-objects --stdin-packs=follow \
--keep-pack=pack-$B_ONLY.pack $packdir/pack <<-EOF
pack-$C.pack
^pack-$A.pack
EOF
) &&

{
objects_in_packs $C &&
git rev-parse "B^{tree}" B:B.t
} >expect.raw &&
sort expect.raw >expect &&

objects_in_packs $P >actual &&
test_cmp expect actual
)
'

test_expect_success '--stdin-packs with !-delimited pack without follow' '
test_when_finished "rm -fr repo" &&

@ -601,4 +642,50 @@ test_expect_success '--stdin-packs=follow respects delta attributes for subtree
)
'

test_expect_success '--keep-pack-from-file names packs to keep' '
test_when_finished "rm -fr repo" &&

git init repo &&
(
cd repo &&
git config set maintenance.auto false &&

test_commit A &&
test_commit B &&
test_commit C &&

A="$(echo A | git pack-objects --revs $packdir/pack)" &&
B="$(echo A..B | git pack-objects --revs $packdir/pack)" &&
C="$(echo B..C | git pack-objects --revs $packdir/pack)" &&
git prune-packed &&

# Empty lines and names that match no pack are ignored,
# as they would be with --keep-pack.
cat >keep <<-EOF &&
pack-$A.pack

pack-$B.pack
pack-does-not-exist.pack
EOF

P=$(git pack-objects --all --keep-pack=pack-$A.pack \
--keep-pack=pack-$B.pack from-argv </dev/null) &&
packed_objects from-argv-$P.idx >expect &&

P=$(git pack-objects --all --keep-pack-from-file=keep \
from-file </dev/null) &&
packed_objects from-file-$P.idx >actual &&
test_cmp expect actual &&

objects_in_packs $C >expect &&
test_cmp expect actual
)
'

test_expect_success '--keep-pack-from-file with a missing file' '
test_must_fail git pack-objects --stdout \
--keep-pack-from-file=does-not-exist </dev/null 2>err &&
test_grep "could not open .does-not-exist. for reading" err
'

test_done

View File

@ -254,6 +254,49 @@ test_expect_success 'repack --keep-pack' '
)
'

test_expect_success 'repack --keep-pack with --pack-kept-objects' '
test_create_repo keep-pack-kept-objects &&
(
cd keep-pack-kept-objects &&
git config pack.window 0 &&
git config maintenance.auto false &&
P1=$(commit_and_pack 1) &&
P2=$(commit_and_pack 2) &&

# "--pack-kept-objects" is about packs that have a ".keep"
# file. A pack named with "--keep-pack" stays out of the
# result regardless, objects included.
git repack -a -d --pack-kept-objects --keep-pack $P1 &&
ls .git/objects/pack/*.pack >counts &&
test_line_count = 2 counts &&
test-tool find-pack -c 1 HEAD~1 &&
test-tool find-pack -c 1 HEAD~1: &&
git fsck
)
'

test_expect_success FUNNYNAMES 'a kept pack whose name has a newline is refused' '
test_create_repo keep-pack-newline &&
(
cd keep-pack-newline &&
git config maintenance.auto false &&
test_commit base &&
git repack -ad &&

# The names pack-objects is told to keep go one per line, so
# this one would come out as two, and the first of them is
# the name of the pack holding everything else.
victim="$(basename "$(ls .git/objects/pack/pack-*.pack)")" &&
name="$(printf "%s\nother" "$victim")" &&
P=$(git rev-parse HEAD | git pack-objects ".git/objects/pack/$name") &&
>".git/objects/pack/$name-$P.keep" &&

test_must_fail git repack -ad 2>err &&
test_grep "contains a newline" err &&
git fsck
)
'

test_expect_success 'repacking fails when missing .pack actually means missing objects' '
test_create_repo idx-without-pack &&
(

View File

@ -541,4 +541,76 @@ test_expect_success 'geometric repack works with promisor packs' '
)
'

test_expect_success 'a ".keep" that shows up mid-repack does not lose objects' '
test_when_finished "rm -fr race" &&
git init race &&
(
cd race &&

test_commit kept &&
test_commit pack &&

KEPT=$(git pack-objects --revs $packdir/pack <<-EOF
refs/tags/kept
EOF
) &&
git pack-objects --revs $packdir/pack <<-EOF &&
refs/tags/pack
^refs/tags/kept
EOF
git prune-packed &&

# Neither pack is twice the size of the other, so both are
# redundant and get deleted. Have a ".keep" appear on one of
# them as pack-objects starts, after the repack has decided
# to delete it: pack-objects used to notice the ".keep" and
# leave those objects out of the replacement pack.
mkdir shim &&
write_script shim/git <<-EOF &&
test "\$1" = "pack-objects" && >"$(pwd)/$packdir/pack-$KEPT.keep"
GIT_EXEC_PATH="$GIT_EXEC_PATH" exec "$GIT_EXEC_PATH/git" "\$@"
EOF

git --exec-path="$(pwd)/shim" repack --geometric 2 -d &&

git fsck
)
'

test_expect_success 'a kept pack does not stop the traversal from rescuing objects' '
test_when_finished "rm -fr kept-open" &&
git init kept-open &&
(
cd kept-open &&
git config repack.midxMustContainCruft false &&

test_commit a &&
test_commit b &&
b=$(git rev-parse b) &&
git repack -ad &&

# Make "b" unreachable and sweep it, together with its tree
# and blob, into a cruft pack.
git tag -d b &&
git reset --hard a &&
git reflog expire --all --expire=all &&
git repack -ad --cruft &&

# Bring the commit back on its own, in a pack marked as kept.
# Its tree and blob are still only in the cruft pack.
kept=$(echo $b | git pack-objects $packdir/pack) &&
>$packdir/pack-$kept.keep &&

# Build on top of it, so that the repack has to look through
# the kept pack to find out what the new commit depends on.
git update-ref refs/heads/master \
$(git commit-tree a^{tree} -p $b -m c) &&

git repack --geometric 2 -d --write-midx --write-bitmap-index &&
test_path_is_file $packdir/multi-pack-index &&
ls $packdir/multi-pack-index-*.bitmap >bitmaps &&
test_line_count = 1 bitmaps
)
'

test_done