diff --git a/builtin/repack.c b/builtin/repack.c index 5d06872d77..8faf7ce2ef 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -457,6 +457,28 @@ int cmd_repack(int argc, existing.repo = repo; existing_packs_collect(&existing, &keep_pack_list); + /* + * Tell pack-objects which packs we may delete. pack-objects looks + * for ".keep" files on its own, and one that appears on such a + * pack from now on must not make it leave out objects that would + * then be deleted with the pack. + */ + if (delete_redundant && + ((pack_everything & ALL_INTO_ONE) || geometry.split_factor) && + existing_packs_has_non_kept(&existing)) { + struct strbuf path = STRBUF_INIT; + struct tempfile *non_kept_packs; + + strbuf_addf(&path, "%s/tmp_non_kept_packs_XXXXXX", + repo_get_object_directory(repo)); + + non_kept_packs = xmks_tempfile(path.buf); + existing_packs_write_non_kept(&existing, non_kept_packs); + po_args.ignore_pack_keep_from = get_tempfile_path(non_kept_packs); + + strbuf_release(&path); + } + if (geometry.split_factor) { if (pack_everything) die(_("options '%s' and '%s' cannot be used together"), "--geometric", "-A/-a"); @@ -644,6 +666,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.ignore_pack_keep_from = po_args.ignore_pack_keep_from; ret = write_cruft_pack(&opts, cruft_expiration, combine_cruft_below_size, &names, diff --git a/repack.c b/repack.c index 4c6d1f507d..e81d9b0de3 100644 --- a/repack.c +++ b/repack.c @@ -40,6 +40,9 @@ void prepare_pack_objects(struct child_process *cmd, strvec_push(&cmd->args, "--delta-base-offset"); if (!args->pack_kept_objects) strvec_push(&cmd->args, "--honor-pack-keep"); + if (args->ignore_pack_keep_from) + strvec_pushf(&cmd->args, "--ignore-pack-keep-from=%s", + args->ignore_pack_keep_from); strvec_push(&cmd->args, out); cmd->git_cmd = 1; cmd->out = -1; @@ -172,6 +175,29 @@ int existing_packs_has_non_kept(const struct existing_packs *existing) return existing->non_kept_packs.nr || existing->cruft_packs.nr; } +void existing_packs_write_non_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->non_kept_packs) + fprintf(out, "%s.pack\n", item->string); + for_each_string_list_item(item, &existing->cruft_packs) + 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 non-kept packs tempfile")); + } +} + static void existing_pack_mark_for_deletion(struct string_list_item *item) { item->util = (void*)((uintptr_t)item->util | DELETE_PACK); diff --git a/repack.h b/repack.h index 61e554e4ed..e3bc840b9a 100644 --- a/repack.h +++ b/repack.h @@ -19,6 +19,11 @@ struct pack_objects_args { int path_walk; int delta_base_offset; int pack_kept_objects; + /* + * A file naming the packs this repack may delete, for + * pack-objects' "--ignore-pack-keep-from"; the path is borrowed. + */ + const char *ignore_pack_keep_from; struct list_objects_filter_options filter_options; }; @@ -28,6 +33,7 @@ struct pack_objects_args { } struct child_process; +struct tempfile; void prepare_pack_objects(struct child_process *cmd, const struct pack_objects_args *args, @@ -80,6 +86,12 @@ struct existing_packs { void existing_packs_collect(struct existing_packs *existing, const struct string_list *extra_keep); int existing_packs_has_non_kept(const struct existing_packs *existing); +/* + * Write the names of the non-kept and cruft packs, the ones a repack may + * delete, to "f", one per line, for pack-objects' "--ignore-pack-keep-from". + */ +void existing_packs_write_non_kept(const struct existing_packs *existing, + struct tempfile *f); int existing_pack_is_marked_for_deletion(struct string_list_item *item); void existing_packs_retain_cruft(struct existing_packs *existing, struct packed_git *cruft); @@ -138,8 +150,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, diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh index 00fe259767..79fa3189f6 100755 --- a/t/t7700-repack.sh +++ b/t/t7700-repack.sh @@ -941,4 +941,53 @@ test_expect_success 'pending objects are repacked appropriately' ' ) ' +# Put a reachable commit and an unreachable one into a single pack X in a +# new repository "keep-race", run "git repack" with the given options and +# have a ".keep" appear on X as pack-objects starts, i.e. after the repack +# has decided to delete X. The unreachable commit is saved in the file +# "keep-race/unreachable". +keep_race_repack () { + git init keep-race && + ( + cd keep-race && + test_commit base && + git commit-tree -m unreachable HEAD^{tree} >unreachable && + + git rev-list --objects --all $(cat unreachable) >objects && + X=$(git pack-objects .git/objects/pack/pack "$(pwd)/.git/objects/pack/pack-$X.keep" && + >"$(pwd)/keep-created" + GIT_EXEC_PATH="$GIT_EXEC_PATH" exec "$GIT_EXEC_PATH/git" "\$@" + EOF + + git --exec-path="$(pwd)/shim" repack "$@" && + test_path_is_file keep-created && + test_path_is_missing .git/objects/pack/pack-$X.pack && + git fsck + ) +} + +for args in "-a -d" "-a -d --filter=blob:none" +do + test_expect_success "a .keep that shows up mid-repack loses nothing: $args" ' + test_when_finished "rm -fr keep-race" && + keep_race_repack $args + ' +done + +for args in "-A -d" "-A -d --pack-kept-objects" "-a -k -d" "--cruft -d" \ + "--cruft --cruft-expiration=1.week.ago -d" +do + test_expect_success "a .keep that shows up mid-repack loses nothing: $args" ' + test_when_finished "rm -fr keep-race" && + keep_race_repack $args && + git -C keep-race cat-file -e $(cat keep-race/unreachable) + ' +done + test_done diff --git a/t/t7703-repack-geometric.sh b/t/t7703-repack-geometric.sh index f3a0650cfe..7a5e3b9ada 100755 --- a/t/t7703-repack-geometric.sh +++ b/t/t7703-repack-geometric.sh @@ -541,4 +541,43 @@ 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 + # rolled up and deleted. Have a ".keep" appear on one of them + # as pack-objects starts, after the repack has decided to + # delete it; its objects must still end up in the new pack. + mkdir shim && + write_script shim/git <<-EOF && + test "\$1" = "pack-objects" && + >"$(pwd)/$packdir/pack-$KEPT.keep" && + >"$(pwd)/keep-created" + GIT_EXEC_PATH="$GIT_EXEC_PATH" exec "$GIT_EXEC_PATH/git" "\$@" + EOF + + git --exec-path="$(pwd)/shim" repack --geometric 2 -d && + test_path_is_file keep-created && + test_path_is_missing $packdir/pack-$KEPT.pack && + + git fsck + ) +' + test_done