| Message ID | 20210414191403.4387-2-rafaeloliveira.cs@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | prevent `repack` to unpack and delete promisor objects | expand |
> The `git repack -d` command will remove any packfile that becomes > redundant after repacking, and then call `git pruned-packed` for > pruning any unpacked objects. s/pruned-packed/prune-packed/ (note that there is no "d" after "prune") throughout this commit message. Also, if there are any objects pruned, they are packed objects, not unpacked objects. Maybe better to say "...for pruning any objects already in packs". > diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh > index 25b235c063..728a16ad97 100755 > --- a/t/t7700-repack.sh > +++ b/t/t7700-repack.sh > @@ -127,12 +127,7 @@ test_expect_success 'packed unreachable obs in alternate ODB are not loosened' ' > git reset --hard HEAD^ && > test_tick && > git reflog expire --expire=$test_tick --expire-unreachable=$test_tick --all && > - # The pack-objects call on the next line is equivalent to > - # git repack -A -d without the call to prune-packed > - git pack-objects --honor-pack-keep --non-empty --all --reflog \ > - --unpack-unreachable </dev/null pack && > - rm -f .git/objects/pack/* && > - mv pack-* .git/objects/pack/ && > + git repack -A -d --no-prune-packed && This is great! > +test_expect_success '-A -d and --no-prune-packed do not remove loose objects' ' > + test_create_repo repo && > + test_when_finished "rm -rf repo" && > + test_commit -C repo commit && > + git -C repo repack -A -d --no-prune-packed && > + git -C repo count-objects -v >out && > + grep "^prune-packable: 3" out > +' As for the test description, I would prefer "...do not remove loose objects already in packs".
Thanks for reviewing this patch. really appreciate it. However, in the v2, I decided to drop this in favor of just adding a trace2 event that emits all the loosened objects. This is to avoid adding user-visible option just for the sake of testing it. Jonathan Tan <jonathantanmy@google.com> writes: >> The `git repack -d` command will remove any packfile that becomes >> redundant after repacking, and then call `git pruned-packed` for >> pruning any unpacked objects. > > s/pruned-packed/prune-packed/ (note that there is no "d" after "prune") > throughout this commit message. > > Also, if there are any objects pruned, they are packed objects, not > unpacked objects. Maybe better to say "...for pruning any objects > already in packs". > Thanks for catching the typo and the clarification. >> diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh >> index 25b235c063..728a16ad97 100755 >> --- a/t/t7700-repack.sh >> +++ b/t/t7700-repack.sh >> @@ -127,12 +127,7 @@ test_expect_success 'packed unreachable obs in alternate ODB are not loosened' ' >> git reset --hard HEAD^ && >> test_tick && >> git reflog expire --expire=$test_tick --expire-unreachable=$test_tick --all && >> - # The pack-objects call on the next line is equivalent to >> - # git repack -A -d without the call to prune-packed >> - git pack-objects --honor-pack-keep --non-empty --all --reflog \ >> - --unpack-unreachable </dev/null pack && >> - rm -f .git/objects/pack/* && >> - mv pack-* .git/objects/pack/ && >> + git repack -A -d --no-prune-packed && > > This is great! > Unfortunately, with the drop of this patch in v2, we no longer refactor these tests. However, this still might be possible with the trace2 event information that is now-added in the v2. Of course, if this doesn't impact or weakens the test in anyway. >> +test_expect_success '-A -d and --no-prune-packed do not remove loose objects' ' >> + test_create_repo repo && >> + test_when_finished "rm -rf repo" && >> + test_commit -C repo commit && >> + git -C repo repack -A -d --no-prune-packed && >> + git -C repo count-objects -v >out && >> + grep "^prune-packable: 3" out >> +' > > As for the test description, I would prefer "...do not remove loose > objects already in packs". Yes, this is an improvement of the test description.
diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt index 317d63cf0d..6ff7a1be16 100644 --- a/Documentation/git-repack.txt +++ b/Documentation/git-repack.txt @@ -86,6 +86,11 @@ to the new separate pack will be written. this repository (or a direct copy of it) over HTTP or FTP. See linkgit:git-update-server-info[1]. +--no-prune-packed:: + Only useful with `-d`. Do not remove redundant loose object + files by skipping the execution of `git prune-packed`. + See linkgit:git-prune-packed[1]. + --window=<n>:: --depth=<n>:: These two options affect how the objects contained in the pack are diff --git a/builtin/repack.c b/builtin/repack.c index 2847fdfbab..6baaeb979c 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -452,6 +452,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) int keep_unreachable = 0; struct string_list keep_pack_list = STRING_LIST_INIT_NODUP; int no_update_server_info = 0; + int no_prune_packed = 0; struct pack_objects_args po_args = {NULL}; int geometric_factor = 0; @@ -469,6 +470,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix) N_("pass --no-reuse-object to git-pack-objects")), OPT_BOOL('n', NULL, &no_update_server_info, N_("do not run git-update-server-info")), + OPT_BOOL(0, "no-prune-packed", &no_prune_packed, + N_("do not run git-prune-packed")), OPT__QUIET(&po_args.quiet, N_("be quiet")), OPT_BOOL('l', "local", &po_args.local, N_("pass --local to git-pack-objects")), @@ -707,7 +710,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix) } if (!po_args.quiet && isatty(2)) opts |= PRUNE_PACKED_VERBOSE; - prune_packed_objects(opts); + if (!no_prune_packed) + prune_packed_objects(opts); if (!keep_unreachable && (!(pack_everything & LOOSEN_UNREACHABLE) || diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh index 25b235c063..728a16ad97 100755 --- a/t/t7700-repack.sh +++ b/t/t7700-repack.sh @@ -127,12 +127,7 @@ test_expect_success 'packed unreachable obs in alternate ODB are not loosened' ' git reset --hard HEAD^ && test_tick && git reflog expire --expire=$test_tick --expire-unreachable=$test_tick --all && - # The pack-objects call on the next line is equivalent to - # git repack -A -d without the call to prune-packed - git pack-objects --honor-pack-keep --non-empty --all --reflog \ - --unpack-unreachable </dev/null pack && - rm -f .git/objects/pack/* && - mv pack-* .git/objects/pack/ && + git repack -A -d --no-prune-packed && git verify-pack -v -- .git/objects/pack/*.idx >packlist && ! grep "^$coid " packlist && echo >.git/objects/info/alternates && @@ -144,18 +139,22 @@ test_expect_success 'local packed unreachable obs that exist in alternate ODB ar echo "$coid" | git pack-objects --non-empty --all --reflog pack && rm -f .git/objects/pack/* && mv pack-* .git/objects/pack/ && - # The pack-objects call on the next line is equivalent to - # git repack -A -d without the call to prune-packed - git pack-objects --honor-pack-keep --non-empty --all --reflog \ - --unpack-unreachable </dev/null pack && - rm -f .git/objects/pack/* && - mv pack-* .git/objects/pack/ && + git repack -A -d --no-prune-packed && git verify-pack -v -- .git/objects/pack/*.idx >packlist && ! grep "^$coid " && echo >.git/objects/info/alternates && test_must_fail git show $coid ' +test_expect_success '-A -d and --no-prune-packed do not remove loose objects' ' + test_create_repo repo && + test_when_finished "rm -rf repo" && + test_commit -C repo commit && + git -C repo repack -A -d --no-prune-packed && + git -C repo count-objects -v >out && + grep "^prune-packable: 3" out +' + test_expect_success 'objects made unreachable by grafts only are kept' ' test_tick && git commit --allow-empty -m "commit 4" &&
The `git repack -d` command will remove any packfile that becomes redundant after repacking, and then call `git pruned-packed` for pruning any unpacked objects. The command, however, does not offer an option to skip the `pruned-packed` execution in order to allow inspecting the objects before they are removed, forcing developers to either make some temporary code changes or to manually craft the `pack-objects` command in order to skip their deletion: git pack-objects --honor-pack-keep --non-empty --all --reflog \ --unpack-unreachable Let’s teach `repack -d` to take --no-pruned-packed option that will simply skip the call to `git prune-packed`, thus providing an easy way to debug the `repack` command. This also allows us to simplify our test suite by replacing calls to `pack-objects` with `repack`: git repack -A -d --no-prune-packed Moreover, using the repack command will actually test its code path instead of just testing the real repack operation by calling `git pack-objects` directly. Let's refactor two tests in t7700 with the new option. Aside from improving our test suite, this new option will be used in an upcoming commit for testing the behavior of `repack` with partial clone repositories. Signed-off-by: Rafael Silva <rafaeloliveira.cs@gmail.com> --- Documentation/git-repack.txt | 5 +++++ builtin/repack.c | 6 +++++- t/t7700-repack.sh | 23 +++++++++++------------ 3 files changed, 21 insertions(+), 13 deletions(-)