{"thread":{"id":"62691","subject":"[PATCH] gc: add `--expire-to` option","startedAt":"2024-12-24T11:53:01Z","lastAt":"2025-02-04T18:15:34Z","messageCount":13,"participants":["ZheNing Hu via GitGitGadget","ZheNing Hu","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"509550","messageId":"pull.1843.git.1735041177817.gitgitgadget@gmail.com","threadId":"62691","inReplyTo":null,"subject":"[PATCH] gc: add `--expire-to` option","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-12-24T11:52:57Z","receivedAt":"2024-12-24T11:53:01Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nThis commit extends the functionality of `git gc`\nby adding a new option, `--expire-to=`. Previously,\nthis feature was implemented in `git repack` (see 91badeb),\nallowing users to specify a directory where unreachable and\nexpired cruft packs are stored during garbage collection.\nHowever, users had to run `git repack --cruft --expire-to=`\nfollowed by `git prune` to achieve similar results within `git gc`.\n\nBy introducing `--expire-to=` directly into `git gc`,\nwe simplify the process for users who wish to manage their\nrepository's cleanup more efficiently. This change involves\npassing the `--expire-to=` parameter through to `git repack`,\nmaking it easier for users to set up a backup location for cruft\npacks that will be pruned.\n\nSigned-off-by: ZheNing Hu \n---\n gc: add --expire-to option\n \n I want to perform a \"safe\" garbage collection for the Git repository on\n the server, which avoids data corruption issues caused by concurrent\n pushes during git GC. To achieve this, I currently need to use git\n repack --cruft --expire-to= and git prune in combination. However,\n it would be simpler if we could directly use --expire-to= with the\n git-gc command.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1843%2Fadlternative%2Fzh%2Fgc-expire-to-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1843/adlternative/zh/gc-expire-to-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1843\n\n Documentation/git-gc.txt | 6 ++++++\n builtin/gc.c | 6 +++++-\n t/t6500-gc.sh | 6 ++++++\n 3 files changed, 17 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt\nindex 370e22faaeb..b4c0cf02972 100644\n--- a/Documentation/git-gc.txt\n+++ b/Documentation/git-gc.txt\n@@ -69,6 +69,12 @@ be performed as well.\n \tthe `--max-cruft-size` option of linkgit:git-repack[1] for\n \tmore.\n \n+--expire-to=::\n+\tWhen packing unreachable objects into a cruft pack, write a cruft\n+\tpack containing pruned objects (if any) to the directory ``.\n+\tSee the `--expire-to` option of linkgit:git-repack[1] for\n+\tmore.\n+\n --prune=::\n \tPrune loose objects older than date (default is 2 weeks ago,\n \toverridable by the config variable `gc.pruneExpire`).\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex d52735354c9..77904694c9f 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -136,6 +136,7 @@ struct gc_config {\n \tchar *prune_worktrees_expire;\n \tchar *repack_filter;\n \tchar *repack_filter_to;\n+\tchar *repack_expire_to;\n \tunsigned long big_pack_threshold;\n \tunsigned long max_delta_cache_size;\n };\n@@ -441,6 +442,8 @@ static void add_repack_all_option(struct gc_config *cfg,\n \t\tif (cfg->max_cruft_size)\n \t\t\tstrvec_pushf(&repack, \"--max-cruft-size=%lu\",\n \t\t\t\t cfg->max_cruft_size);\n+\t\tif (cfg->repack_expire_to)\n+\t\t\tstrvec_pushf(&repack, \"--expire-to=%s\", cfg->repack_expire_to);\n \t} else {\n \t\tstrvec_push(&repack, \"-A\");\n \t\tif (cfg->prune_expire)\n@@ -675,7 +678,6 @@ struct repository *repo UNUSED)\n \tconst char *prune_expire_sentinel = \"sentinel\";\n \tconst char *prune_expire_arg = prune_expire_sentinel;\n \tint ret;\n-\n \tstruct option builtin_gc_options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"suppress progress reporting\")),\n \t\t{ OPTION_STRING, 0, \"prune\", &prune_expire_arg, N_(\"date\"),\n@@ -694,6 +696,8 @@ struct repository *repo UNUSED)\n \t\t\t PARSE_OPT_NOCOMPLETE),\n \t\tOPT_BOOL(0, \"keep-largest-pack\", &keep_largest_pack,\n \t\t\t N_(\"repack all other packs except the largest pack\")),\n+\t\tOPT_STRING(0, \"expire-to\", &cfg.repack_expire_to, N_(\"dir\"),\n+\t\t\t N_(\"pack prefix to store a pack containing pruned objects\")),\n \t\tOPT_END()\n \t};\n \ndiff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\nindex ee074b99b70..d4b0653a9b7 100755\n--- a/t/t6500-gc.sh\n+++ b/t/t6500-gc.sh\n@@ -339,6 +339,12 @@ test_expect_success 'gc.maxCruftSize sets appropriate repack options' '\n \ttest_subcommand $cruft_max_size_opts --max-cruft-size=3145728 and git prune in combination. However, it would be simpler\nif we could directly use --expire-to= with the git-gc command.\n\nZheNing Hu (2):\n gc: add `--expire-to` option\n fix(gc): make --prune=now compatible with --expire-to\n\n Documentation/git-gc.txt | 6 ++++++\n builtin/gc.c | 9 +++++++--\n t/t6500-gc.sh | 6 ++++++\n 3 files changed, 19 insertions(+), 2 deletions(-)\n\n\nbase-commit: 92999a42db1c5f43f330e4f2bca4026b5b81576f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1843%2Fadlternative%2Fzh%2Fgc-expire-to-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1843/adlternative/zh/gc-expire-to-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1843\n\nRange-diff vs v1:\n\n 1: 14e94bf04e5 = 1: 14e94bf04e5 gc: add `--expire-to` option\n -: ----------- > 2: 579757957d2 fix(gc): make --prune=now compatible with --expire-to\n\n-- \ngitgitgadget\n"},{"id":"509757","messageId":"14e94bf04e5ae3895eb61253be9c6d0a0fe56328.1735611513.git.gitgitgadget@gmail.com","threadId":"62691","inReplyTo":"pull.1843.v2.git.1735611513.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] gc: add `--expire-to` option","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-12-31T02:18:32Z","receivedAt":"2024-12-31T02:18:38Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nThis commit extends the functionality of `git gc`\nby adding a new option, `--expire-to=`. Previously,\nthis feature was implemented in `git repack` (see 91badeb),\nallowing users to specify a directory where unreachable and\nexpired cruft packs are stored during garbage collection.\nHowever, users had to run `git repack --cruft --expire-to=`\nfollowed by `git prune` to achieve similar results within `git gc`.\n\nBy introducing `--expire-to=` directly into `git gc`,\nwe simplify the process for users who wish to manage their\nrepository's cleanup more efficiently. This change involves\npassing the `--expire-to=` parameter through to `git repack`,\nmaking it easier for users to set up a backup location for cruft\npacks that will be pruned.\n\nSigned-off-by: ZheNing Hu \n---\n Documentation/git-gc.txt | 6 ++++++\n builtin/gc.c | 6 +++++-\n t/t6500-gc.sh | 6 ++++++\n 3 files changed, 17 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt\nindex 370e22faaeb..b4c0cf02972 100644\n--- a/Documentation/git-gc.txt\n+++ b/Documentation/git-gc.txt\n@@ -69,6 +69,12 @@ be performed as well.\n \tthe `--max-cruft-size` option of linkgit:git-repack[1] for\n \tmore.\n \n+--expire-to=::\n+\tWhen packing unreachable objects into a cruft pack, write a cruft\n+\tpack containing pruned objects (if any) to the directory ``.\n+\tSee the `--expire-to` option of linkgit:git-repack[1] for\n+\tmore.\n+\n --prune=::\n \tPrune loose objects older than date (default is 2 weeks ago,\n \toverridable by the config variable `gc.pruneExpire`).\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex d52735354c9..77904694c9f 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -136,6 +136,7 @@ struct gc_config {\n \tchar *prune_worktrees_expire;\n \tchar *repack_filter;\n \tchar *repack_filter_to;\n+\tchar *repack_expire_to;\n \tunsigned long big_pack_threshold;\n \tunsigned long max_delta_cache_size;\n };\n@@ -441,6 +442,8 @@ static void add_repack_all_option(struct gc_config *cfg,\n \t\tif (cfg->max_cruft_size)\n \t\t\tstrvec_pushf(&repack, \"--max-cruft-size=%lu\",\n \t\t\t\t cfg->max_cruft_size);\n+\t\tif (cfg->repack_expire_to)\n+\t\t\tstrvec_pushf(&repack, \"--expire-to=%s\", cfg->repack_expire_to);\n \t} else {\n \t\tstrvec_push(&repack, \"-A\");\n \t\tif (cfg->prune_expire)\n@@ -675,7 +678,6 @@ struct repository *repo UNUSED)\n \tconst char *prune_expire_sentinel = \"sentinel\";\n \tconst char *prune_expire_arg = prune_expire_sentinel;\n \tint ret;\n-\n \tstruct option builtin_gc_options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"suppress progress reporting\")),\n \t\t{ OPTION_STRING, 0, \"prune\", &prune_expire_arg, N_(\"date\"),\n@@ -694,6 +696,8 @@ struct repository *repo UNUSED)\n \t\t\t PARSE_OPT_NOCOMPLETE),\n \t\tOPT_BOOL(0, \"keep-largest-pack\", &keep_largest_pack,\n \t\t\t N_(\"repack all other packs except the largest pack\")),\n+\t\tOPT_STRING(0, \"expire-to\", &cfg.repack_expire_to, N_(\"dir\"),\n+\t\t\t N_(\"pack prefix to store a pack containing pruned objects\")),\n \t\tOPT_END()\n \t};\n \ndiff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\nindex ee074b99b70..d4b0653a9b7 100755\n--- a/t/t6500-gc.sh\n+++ b/t/t6500-gc.sh\n@@ -339,6 +339,12 @@ test_expect_success 'gc.maxCruftSize sets appropriate repack options' '\n \ttest_subcommand $cruft_max_size_opts --max-cruft-size=3145728 \n\nThe original `git gc --prune=now` attempted to delete all\nunreachable objects. However, after the introduction of\n`--cruft` and `--expire-to=` in git gc, `--prune=now`\ncan now compress unreachable objects into a cruft pack and\nstore them in the specified instead of deleting them\ndirectly. This is beneficial for recovery in case of data\ncorruption during repository GC. Therefore, update the\nhandling logic of `--prune=now` in gc so that `-a` parameter\nis only passed to the repack command when neither `--cruft`\nnor `--expire-to` are used.\n\nSigned-off-by: ZheNing Hu \n---\n builtin/gc.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 77904694c9f..8656e1caff0 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -433,7 +433,8 @@ static int keep_one_pack(struct string_list_item *item, void *data UNUSED)\n static void add_repack_all_option(struct gc_config *cfg,\n \t\t\t\t struct string_list *keep_pack)\n {\n-\tif (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\"))\n+\tif (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\")\n+\t\t&& !(cfg->cruft_packs && cfg->repack_expire_to))\n \t\tstrvec_push(&repack, \"-a\");\n \telse if (cfg->cruft_packs) {\n \t\tstrvec_push(&repack, \"--cruft\");\n-- \ngitgitgadget\n"},{"id":"509819","messageId":"CAOLTT8TKC5cKPudWjfBtaXLRHV+ca6cX2hkbpS7L66P9Vkgt7w@mail.gmail.com","threadId":"62691","inReplyTo":"14e94bf04e5ae3895eb61253be9c6d0a0fe56328.1735611513.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/2] gc: add `--expire-to` option","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2025-01-03T04:57:21Z","receivedAt":"2025-01-03T04:57:34Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"ZheNing Hu via GitGitGadget 于2024年12月31日周二 10:18写道：\n>\n> From: ZheNing Hu \n>\n> This commit extends the functionality of `git gc`\n> by adding a new option, `--expire-to=`. Previously,\n> this feature was implemented in `git repack` (see 91badeb),\n> allowing users to specify a directory where unreachable and\n> expired cruft packs are stored during garbage collection.\n> However, users had to run `git repack --cruft --expire-to=`\n> followed by `git prune` to achieve similar results within `git gc`.\n>\n> By introducing `--expire-to=` directly into `git gc`,\n> we simplify the process for users who wish to manage their\n> repository's cleanup more efficiently. This change involves\n> passing the `--expire-to=` parameter through to `git repack`,\n> making it easier for users to set up a backup location for cruft\n> packs that will be pruned.\n>\n> Signed-off-by: ZheNing Hu \n> ---\n> Documentation/git-gc.txt | 6 ++++++\n> builtin/gc.c | 6 +++++-\n> t/t6500-gc.sh | 6 ++++++\n> 3 files changed, 17 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt\n> index 370e22faaeb..b4c0cf02972 100644\n> --- a/Documentation/git-gc.txt\n> +++ b/Documentation/git-gc.txt\n> @@ -69,6 +69,12 @@ be performed as well.\n> the `--max-cruft-size` option of linkgit:git-repack[1] for\n> more.\n>\n> +--expire-to=::\n> + When packing unreachable objects into a cruft pack, write a cruft\n> + pack containing pruned objects (if any) to the directory ``.\n> + See the `--expire-to` option of linkgit:git-repack[1] for\n> + more.\n> +\n> --prune=::\n> Prune loose objects older than date (default is 2 weeks ago,\n> overridable by the config variable `gc.pruneExpire`).\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index d52735354c9..77904694c9f 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -136,6 +136,7 @@ struct gc_config {\n> char *prune_worktrees_expire;\n> char *repack_filter;\n> char *repack_filter_to;\n> + char *repack_expire_to;\n> unsigned long big_pack_threshold;\n> unsigned long max_delta_cache_size;\n> };\n> @@ -441,6 +442,8 @@ static void add_repack_all_option(struct gc_config *cfg,\n> if (cfg->max_cruft_size)\n> strvec_pushf(&repack, \"--max-cruft-size=%lu\",\n> cfg->max_cruft_size);\n> + if (cfg->repack_expire_to)\n> + strvec_pushf(&repack, \"--expire-to=%s\", cfg->repack_expire_to);\n> } else {\n> strvec_push(&repack, \"-A\");\n> if (cfg->prune_expire)\n> @@ -675,7 +678,6 @@ struct repository *repo UNUSED)\n> const char *prune_expire_sentinel = \"sentinel\";\n> const char *prune_expire_arg = prune_expire_sentinel;\n> int ret;\n> -\n> struct option builtin_gc_options[] = {\n> OPT__QUIET(&quiet, N_(\"suppress progress reporting\")),\n> { OPTION_STRING, 0, \"prune\", &prune_expire_arg, N_(\"date\"),\n> @@ -694,6 +696,8 @@ struct repository *repo UNUSED)\n> PARSE_OPT_NOCOMPLETE),\n> OPT_BOOL(0, \"keep-largest-pack\", &keep_largest_pack,\n> N_(\"repack all other packs except the largest pack\")),\n> + OPT_STRING(0, \"expire-to\", &cfg.repack_expire_to, N_(\"dir\"),\n> + N_(\"pack prefix to store a pack containing pruned objects\")),\n> OPT_END()\n> };\n>\n> diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\n> index ee074b99b70..d4b0653a9b7 100755\n> --- a/t/t6500-gc.sh\n> +++ b/t/t6500-gc.sh\n> @@ -339,6 +339,12 @@ test_expect_success 'gc.maxCruftSize sets appropriate repack options' '\n> test_subcommand $cruft_max_size_opts --max-cruft-size=3145728 '\n>\n> +test_expect_success '--expire-to sets appropriate repack options' '\n> + mkdir expired &&\n> + GIT_TRACE2_EVENT=$(pwd)/trace2.txt git -C cruft--max-size gc --cruft --expire-to=./expired/pack &&\n> + test_subcommand $cruft_max_size_opts --expire-to=./expired/pack +'\n> +\n> run_and_wait_for_gc () {\n> # We read stdout from gc for the side effect of waiting until the\n> # background gc process exits, closing its fd 9. Furthermore, the\n> --\n> gitgitgadget\n>\n\nHi, Jeff King, could you come and help take a look at this patch?\nI would be very grateful if you have time!\n\nZheNing Hu\n"},{"id":"510390","messageId":"CAOLTT8Qj=mA6j55HOShobQSkCn-VCbPoMChE_y1xBo=O4dyFxw@mail.gmail.com","threadId":"62691","inReplyTo":"14e94bf04e5ae3895eb61253be9c6d0a0fe56328.1735611513.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/2] gc: add `--expire-to` option","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2025-01-13T07:12:01Z","receivedAt":"2025-01-13T07:12:15Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"This patch has been sitting for weeks with no review. Does anyone want\nto help take a look?\n\nZheNing Hu via GitGitGadget 于2024年12月31日周二 10:18写道：\n>\n> From: ZheNing Hu \n>\n> This commit extends the functionality of `git gc`\n> by adding a new option, `--expire-to=`. Previously,\n> this feature was implemented in `git repack` (see 91badeb),\n> allowing users to specify a directory where unreachable and\n> expired cruft packs are stored during garbage collection.\n> However, users had to run `git repack --cruft --expire-to=`\n> followed by `git prune` to achieve similar results within `git gc`.\n>\n> By introducing `--expire-to=` directly into `git gc`,\n> we simplify the process for users who wish to manage their\n> repository's cleanup more efficiently. This change involves\n> passing the `--expire-to=` parameter through to `git repack`,\n> making it easier for users to set up a backup location for cruft\n> packs that will be pruned.\n>\n> Signed-off-by: ZheNing Hu \n> ---\n> Documentation/git-gc.txt | 6 ++++++\n> builtin/gc.c | 6 +++++-\n> t/t6500-gc.sh | 6 ++++++\n> 3 files changed, 17 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt\n> index 370e22faaeb..b4c0cf02972 100644\n> --- a/Documentation/git-gc.txt\n> +++ b/Documentation/git-gc.txt\n> @@ -69,6 +69,12 @@ be performed as well.\n> the `--max-cruft-size` option of linkgit:git-repack[1] for\n> more.\n>\n> +--expire-to=::\n> + When packing unreachable objects into a cruft pack, write a cruft\n> + pack containing pruned objects (if any) to the directory ``.\n> + See the `--expire-to` option of linkgit:git-repack[1] for\n> + more.\n> +\n> --prune=::\n> Prune loose objects older than date (default is 2 weeks ago,\n> overridable by the config variable `gc.pruneExpire`).\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index d52735354c9..77904694c9f 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -136,6 +136,7 @@ struct gc_config {\n> char *prune_worktrees_expire;\n> char *repack_filter;\n> char *repack_filter_to;\n> + char *repack_expire_to;\n> unsigned long big_pack_threshold;\n> unsigned long max_delta_cache_size;\n> };\n> @@ -441,6 +442,8 @@ static void add_repack_all_option(struct gc_config *cfg,\n> if (cfg->max_cruft_size)\n> strvec_pushf(&repack, \"--max-cruft-size=%lu\",\n> cfg->max_cruft_size);\n> + if (cfg->repack_expire_to)\n> + strvec_pushf(&repack, \"--expire-to=%s\", cfg->repack_expire_to);\n> } else {\n> strvec_push(&repack, \"-A\");\n> if (cfg->prune_expire)\n> @@ -675,7 +678,6 @@ struct repository *repo UNUSED)\n> const char *prune_expire_sentinel = \"sentinel\";\n> const char *prune_expire_arg = prune_expire_sentinel;\n> int ret;\n> -\n> struct option builtin_gc_options[] = {\n> OPT__QUIET(&quiet, N_(\"suppress progress reporting\")),\n> { OPTION_STRING, 0, \"prune\", &prune_expire_arg, N_(\"date\"),\n> @@ -694,6 +696,8 @@ struct repository *repo UNUSED)\n> PARSE_OPT_NOCOMPLETE),\n> OPT_BOOL(0, \"keep-largest-pack\", &keep_largest_pack,\n> N_(\"repack all other packs except the largest pack\")),\n> + OPT_STRING(0, \"expire-to\", &cfg.repack_expire_to, N_(\"dir\"),\n> + N_(\"pack prefix to store a pack containing pruned objects\")),\n> OPT_END()\n> };\n>\n> diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\n> index ee074b99b70..d4b0653a9b7 100755\n> --- a/t/t6500-gc.sh\n> +++ b/t/t6500-gc.sh\n> @@ -339,6 +339,12 @@ test_expect_success 'gc.maxCruftSize sets appropriate repack options' '\n> test_subcommand $cruft_max_size_opts --max-cruft-size=3145728 '\n>\n> +test_expect_success '--expire-to sets appropriate repack options' '\n> + mkdir expired &&\n> + GIT_TRACE2_EVENT=$(pwd)/trace2.txt git -C cruft--max-size gc --cruft --expire-to=./expired/pack &&\n> + test_subcommand $cruft_max_size_opts --expire-to=./expired/pack +'\n> +\n> run_and_wait_for_gc () {\n> # We read stdout from gc for the side effect of waiting until the\n> # background gc process exits, closing its fd 9. Furthermore, the\n> --\n> gitgitgadget\n>\n"},{"id":"510405","messageId":"20250113091750.GA794747@coredump.intra.peff.net","threadId":"62691","inReplyTo":"579757957d21faaa8dd9228a191d82f663e93c03.1735611513.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/2] fix(gc): make --prune=now compatible with --expire-to","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-01-13T09:17:50Z","receivedAt":"2025-01-13T09:17:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 31, 2024 at 02:18:33AM +0000, ZheNing Hu via GitGitGadget wrote:\n\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index 77904694c9f..8656e1caff0 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -433,7 +433,8 @@ static int keep_one_pack(struct string_list_item *item, void *data UNUSED)\n> static void add_repack_all_option(struct gc_config *cfg,\n> \t\t\t\t struct string_list *keep_pack)\n> {\n> -\tif (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\"))\n> +\tif (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\")\n> +\t\t&& !(cfg->cruft_packs && cfg->repack_expire_to))\n> \t\tstrvec_push(&repack, \"-a\");\n\nI expected to see a mention of repack_expire_to here, but not\ncfg->cruft_packs. These two are AND-ed together so we are only disabling\n\"repack -a\" when both options (\"--expire-to\" and \"--cruft\") are passed.\nCan we --expire-to without cruft? I.e., what should happen with:\n\n git gc --expire-to=some-path --prune=now --no-cruft\n\nLooking at the underlying git-repack, it seems that we only respect\n--expire-to at all when used with \"--cruft\", and don't otherwise\nconsider it. Which is what the manpage says (\"Only useful with --cruft\n-d\").\n\nBut if we look at this proposed patch for example:\n\n https://lore.kernel.org/git/48438876fb42a889110e100a6c42ca84e93aac49.1733011259.git.me@ttaylorr.com/\n\nthen it is expanding how --expire-to is used during the pruning step.\nOTOH, I think the way your patch 1 is structured means that we'd always\npass --expire-to to git-repack anyway, and I _think_ even with the patch\nlinked above that \"repack -a -d --expire-to=whatever\" would do the right\nthing.\n\nIn which case the problem really is the combination of cruft packs and\nexpire-to. Just cruft packs by themselves do not need to override using\n\"-a\" for \"--prune=now\" because we know that any such cruft pack would be\nempty.\n\nSo I think this logic is correct. Taylor might have more thoughts,\nthough (and ideas on whether he intends to revisit that earlier patch).\n\nI do think this change should probably be done as part of patch 1,\nrather than introducing a buggy state and then fixing it in patch 2.\n\n-Peff\n"},{"id":"510557","messageId":"CAOLTT8TaDBtpgXRAj9yjiJB-h8vGjd6bEg29gOiOUwvQXbzZ3g@mail.gmail.com","threadId":"62691","inReplyTo":"20250113091750.GA794747@coredump.intra.peff.net","subject":"Re: [PATCH v2 2/2] fix(gc): make --prune=now compatible with --expire-to","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2025-01-15T07:56:18Z","receivedAt":"2025-01-15T07:56:32Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Jeff King 于2025年1月13日周一 17:17写道：\n>\n> On Tue, Dec 31, 2024 at 02:18:33AM +0000, ZheNing Hu via GitGitGadget wrote:\n>\n> > diff --git a/builtin/gc.c b/builtin/gc.c\n> > index 77904694c9f..8656e1caff0 100644\n> > --- a/builtin/gc.c\n> > +++ b/builtin/gc.c\n> > @@ -433,7 +433,8 @@ static int keep_one_pack(struct string_list_item *item, void *data UNUSED)\n> > static void add_repack_all_option(struct gc_config *cfg,\n> > struct string_list *keep_pack)\n> > {\n> > - if (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\"))\n> > + if (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\")\n> > + && !(cfg->cruft_packs && cfg->repack_expire_to))\n> > strvec_push(&repack, \"-a\");\n>\n> I expected to see a mention of repack_expire_to here, but not\n> cfg->cruft_packs. These two are AND-ed together so we are only disabling\n> \"repack -a\" when both options (\"--expire-to\" and \"--cruft\") are passed.\n> Can we --expire-to without cruft? I.e., what should happen with:\n>\n> git gc --expire-to=some-path --prune=now --no-cruft\n>\n> Looking at the underlying git-repack, it seems that we only respect\n> --expire-to at all when used with \"--cruft\", and don't otherwise\n> consider it. Which is what the manpage says (\"Only useful with --cruft\n> -d\").\n>\n\nYes, this is the current state of git-repack. The --expire-to option can\nonly be used with --cruft, which is why I use cruft_packs && repack_expire_to\nas a double safeguard.\n\nWhen using --no-cruft, the option --expire-to becomes irrelevant.\nSo leaving `git gc --prune=now` as is at this point: passing -a as a\nparameter to repack seems reasonable.\n\n> But if we look at this proposed patch for example:\n>\n> https://lore.kernel.org/git/48438876fb42a889110e100a6c42ca84e93aac49.1733011259.git.me@ttaylorr.com/\n>\n> then it is expanding how --expire-to is used during the pruning step.\n> OTOH, I think the way your patch 1 is structured means that we'd always\n> pass --expire-to to git-repack anyway, and I _think_ even with the patch\n> linked above that \"repack -a -d --expire-to=whatever\" would do the right\n> thing.\n>\n\nI've taken a look at the patch, and I believe Taylor's changes are primarily\naimed at extending the --expire-to functionality within the --cruft feature,\nrather than expecting --expire-to to be used on its own.\n\n> In which case the problem really is the combination of cruft packs and\n> expire-to. Just cruft packs by themselves do not need to override using\n> \"-a\" for \"--prune=now\" because we know that any such cruft pack would be\n> empty.\n>\n> So I think this logic is correct. Taylor might have more thoughts,\n> though (and ideas on whether he intends to revisit that earlier patch).\n>\n> I do think this change should probably be done as part of patch 1,\n> rather than introducing a buggy state and then fixing it in patch 2.\n>\n\nYes, I agree with that, and perhaps a single patch will suffice.\n\n> -Peff\n\n- ZheNing Hu\n"},{"id":"510633","messageId":"pull.1843.v3.git.1736994932003.gitgitgadget@gmail.com","threadId":"62691","inReplyTo":"pull.1843.v2.git.1735611513.gitgitgadget@gmail.com","subject":"[PATCH v3] gc: add `--expire-to` option","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-01-16T02:35:31Z","receivedAt":"2025-01-16T02:35:35Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nThis commit extends the functionality of `git gc`\nby adding a new option, `--expire-to=`. Previously,\nthis feature was implemented in `git repack` (see 91badeb),\nallowing users to specify a directory where unreachable and\nexpired cruft packs are stored during garbage collection.\nHowever, users had to run `git repack --cruft --expire-to=`\nfollowed by `git prune` to achieve similar results within `git gc`.\n\nBy introducing `--expire-to=` directly into `git gc`,\nwe simplify the process for users who wish to manage their\nrepository's cleanup more efficiently. This change involves\npassing the `--expire-to=` parameter through to `git repack`,\nmaking it easier for users to set up a backup location for cruft\npacks that will be pruned.\n\nNote: When git-gc is used with both `--cruft` and `--expire-to`,\nit does not pass `-a` to git-repack to delete all unreachable\nobjects as `git gc --prune=now` originally did. Instead, it\ngenerates a cruft pack in the directory specified by expire-to.\n\nSigned-off-by: ZheNing Hu \n---\n gc: add --expire-to option\n \n I want to perform a \"safe\" garbage collection for the Git repository on\n the server, which avoids data corruption issues caused by concurrent\n pushes during git GC. To achieve this, I currently need to use git\n repack --cruft --expire-to= and git prune in combination. However,\n it would be simpler if we could directly use --expire-to= with the\n git-gc command.\n \n v1: add --expire-to option to gc v1 -> v2: fix git gc --prune=now with\n --expire-to v2 -> v3: squash two patch into one patch\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1843%2Fadlternative%2Fzh%2Fgc-expire-to-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1843/adlternative/zh/gc-expire-to-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1843\n\nRange-diff vs v2:\n\n 1: 14e94bf04e5 ! 1: 0842ec34948 gc: add `--expire-to` option\n @@ Commit message\n making it easier for users to set up a backup location for cruft\n packs that will be pruned.\n \n + Note: When git-gc is used with both `--cruft` and `--expire-to`,\n + it does not pass `-a` to git-repack to delete all unreachable\n + objects as `git gc --prune=now` originally did. Instead, it\n + generates a cruft pack in the directory specified by expire-to.\n +\n Signed-off-by: ZheNing Hu \n \n ## Documentation/git-gc.txt ##\n @@ builtin/gc.c: struct gc_config {\n \tunsigned long big_pack_threshold;\n \tunsigned long max_delta_cache_size;\n };\n +@@ builtin/gc.c: static int keep_one_pack(struct string_list_item *item, void *data UNUSED)\n + static void add_repack_all_option(struct gc_config *cfg,\n + \t\t\t\t struct string_list *keep_pack)\n + {\n +-\tif (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\"))\n ++\tif (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\")\n ++\t\t&& !(cfg->cruft_packs && cfg->repack_expire_to))\n + \t\tstrvec_push(&repack, \"-a\");\n + \telse if (cfg->cruft_packs) {\n + \t\tstrvec_push(&repack, \"--cruft\");\n @@ builtin/gc.c: static void add_repack_all_option(struct gc_config *cfg,\n \t\tif (cfg->max_cruft_size)\n \t\t\tstrvec_pushf(&repack, \"--max-cruft-size=%lu\",\n 2: 579757957d2 < -: ----------- fix(gc): make --prune=now compatible with --expire-to\n\n\n Documentation/git-gc.txt | 6 ++++++\n builtin/gc.c | 9 +++++++--\n t/t6500-gc.sh | 6 ++++++\n 3 files changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt\nindex 370e22faaeb..b4c0cf02972 100644\n--- a/Documentation/git-gc.txt\n+++ b/Documentation/git-gc.txt\n@@ -69,6 +69,12 @@ be performed as well.\n \tthe `--max-cruft-size` option of linkgit:git-repack[1] for\n \tmore.\n \n+--expire-to=::\n+\tWhen packing unreachable objects into a cruft pack, write a cruft\n+\tpack containing pruned objects (if any) to the directory ``.\n+\tSee the `--expire-to` option of linkgit:git-repack[1] for\n+\tmore.\n+\n --prune=::\n \tPrune loose objects older than date (default is 2 weeks ago,\n \toverridable by the config variable `gc.pruneExpire`).\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex d52735354c9..8656e1caff0 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -136,6 +136,7 @@ struct gc_config {\n \tchar *prune_worktrees_expire;\n \tchar *repack_filter;\n \tchar *repack_filter_to;\n+\tchar *repack_expire_to;\n \tunsigned long big_pack_threshold;\n \tunsigned long max_delta_cache_size;\n };\n@@ -432,7 +433,8 @@ static int keep_one_pack(struct string_list_item *item, void *data UNUSED)\n static void add_repack_all_option(struct gc_config *cfg,\n \t\t\t\t struct string_list *keep_pack)\n {\n-\tif (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\"))\n+\tif (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\")\n+\t\t&& !(cfg->cruft_packs && cfg->repack_expire_to))\n \t\tstrvec_push(&repack, \"-a\");\n \telse if (cfg->cruft_packs) {\n \t\tstrvec_push(&repack, \"--cruft\");\n@@ -441,6 +443,8 @@ static void add_repack_all_option(struct gc_config *cfg,\n \t\tif (cfg->max_cruft_size)\n \t\t\tstrvec_pushf(&repack, \"--max-cruft-size=%lu\",\n \t\t\t\t cfg->max_cruft_size);\n+\t\tif (cfg->repack_expire_to)\n+\t\t\tstrvec_pushf(&repack, \"--expire-to=%s\", cfg->repack_expire_to);\n \t} else {\n \t\tstrvec_push(&repack, \"-A\");\n \t\tif (cfg->prune_expire)\n@@ -675,7 +679,6 @@ struct repository *repo UNUSED)\n \tconst char *prune_expire_sentinel = \"sentinel\";\n \tconst char *prune_expire_arg = prune_expire_sentinel;\n \tint ret;\n-\n \tstruct option builtin_gc_options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"suppress progress reporting\")),\n \t\t{ OPTION_STRING, 0, \"prune\", &prune_expire_arg, N_(\"date\"),\n@@ -694,6 +697,8 @@ struct repository *repo UNUSED)\n \t\t\t PARSE_OPT_NOCOMPLETE),\n \t\tOPT_BOOL(0, \"keep-largest-pack\", &keep_largest_pack,\n \t\t\t N_(\"repack all other packs except the largest pack\")),\n+\t\tOPT_STRING(0, \"expire-to\", &cfg.repack_expire_to, N_(\"dir\"),\n+\t\t\t N_(\"pack prefix to store a pack containing pruned objects\")),\n \t\tOPT_END()\n \t};\n \ndiff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\nindex ee074b99b70..d4b0653a9b7 100755\n--- a/t/t6500-gc.sh\n+++ b/t/t6500-gc.sh\n@@ -339,6 +339,12 @@ test_expect_success 'gc.maxCruftSize sets appropriate repack options' '\n \ttest_subcommand $cruft_max_size_opts --max-cruft-size=3145728 writes:\n\n> From: ZheNing Hu \n>\n> This commit extends the functionality of `git gc`\n> by adding a new option, `--expire-to=`. Previously,\n> this feature was implemented in `git repack` (see 91badeb),\n> allowing users to specify a directory where unreachable and\n> expired cruft packs are stored during garbage collection.\n> However, users had to run `git repack --cruft --expire-to=`\n> followed by `git prune` to achieve similar results within `git gc`.\n>\n> By introducing `--expire-to=` directly into `git gc`,\n> we simplify the process for users who wish to manage their\n> repository's cleanup more efficiently. This change involves\n> passing the `--expire-to=` parameter through to `git repack`,\n> making it easier for users to set up a backup location for cruft\n> packs that will be pruned.\n\nToday I do not have enough time to do my usual commit log message\ncritique. Please use \"git show -s --format=reference\" when\nreferring to an earlier commit.\n\n> Note: When git-gc is used with both `--cruft` and `--expire-to`,\n> it does not pass `-a` to git-repack to delete all unreachable\n> objects as `git gc --prune=now` originally did. Instead, it\n> generates a cruft pack in the directory specified by expire-to.\n\nIs this less important than \"we added --expire-to to gc that is\npassed down to underlying repack\" in the previous paragraph?\n\nNot removing the unreachables too early with \"repack -a\" is an\nessential part of the design of this new feature to allow us not to\nlose the cruft objects, so I was a bit surprised that this was\ndescribed as a \"Note:\".\n\n> diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt\n> index 370e22faaeb..b4c0cf02972 100644\n> --- a/Documentation/git-gc.txt\n> +++ b/Documentation/git-gc.txt\n> @@ -69,6 +69,12 @@ be performed as well.\n> \tthe `--max-cruft-size` option of linkgit:git-repack[1] for\n> \tmore.\n> \n> +--expire-to=::\n> +\tWhen packing unreachable objects into a cruft pack, write a cruft\n> +\tpack containing pruned objects (if any) to the directory ``.\n> +\tSee the `--expire-to` option of linkgit:git-repack[1] for\n> +\tmore.\n\nDoes \"When packing unreachable objects into a cruft pack\" mean that\nthis option is only meaningful with \"--cruft\"? As \"--cruft\" is on\nby default, is it an error to pass \"--no-cruft\" when you use this\noption?\n\n\"for more\" -> \"for more information\" or something?\n\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index d52735354c9..8656e1caff0 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -136,6 +136,7 @@ struct gc_config {\n> \tchar *prune_worktrees_expire;\n> \tchar *repack_filter;\n> \tchar *repack_filter_to;\n> +\tchar *repack_expire_to;\n> \tunsigned long big_pack_threshold;\n> \tunsigned long max_delta_cache_size;\n> };\n> @@ -432,7 +433,8 @@ static int keep_one_pack(struct string_list_item *item, void *data UNUSED)\n> static void add_repack_all_option(struct gc_config *cfg,\n> \t\t\t\t struct string_list *keep_pack)\n> {\n> -\tif (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\"))\n> +\tif (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\")\n> +\t\t&& !(cfg->cruft_packs && cfg->repack_expire_to))\n> \t\tstrvec_push(&repack, \"-a\");\n\nHmph. When \"--expire-to=\" is given, we are dropping these\nunreachable objects right away, but we said \"--no-cruft\", then we\nsay \"repack -a\". If we have both \"--cruft\" and \"--expire-to=\",\nthen ...\n\n> \telse if (cfg->cruft_packs) {\n> \t\tstrvec_push(&repack, \"--cruft\");\n> @@ -441,6 +443,8 @@ static void add_repack_all_option(struct gc_config *cfg,\n> \t\tif (cfg->max_cruft_size)\n> \t\t\tstrvec_pushf(&repack, \"--max-cruft-size=%lu\",\n> \t\t\t\t cfg->max_cruft_size);\n> +\t\tif (cfg->repack_expire_to)\n> +\t\t\tstrvec_pushf(&repack, \"--expire-to=%s\", cfg->repack_expire_to);\n\n... we do the usual \"repack --cruft --expire-to=\" in the next\nblock.\n\n> @@ -675,7 +679,6 @@ struct repository *repo UNUSED)\n> \tconst char *prune_expire_sentinel = \"sentinel\";\n> \tconst char *prune_expire_arg = prune_expire_sentinel;\n> \tint ret;\n> -\n> \tstruct option builtin_gc_options[] = {\n> \t\tOPT__QUIET(&quiet, N_(\"suppress progress reporting\")),\n> \t\t{ OPTION_STRING, 0, \"prune\", &prune_expire_arg, N_(\"date\"),\n\nOK.\n\n> @@ -694,6 +697,8 @@ struct repository *repo UNUSED)\n> \t\t\t PARSE_OPT_NOCOMPLETE),\n> \t\tOPT_BOOL(0, \"keep-largest-pack\", &keep_largest_pack,\n> \t\t\t N_(\"repack all other packs except the largest pack\")),\n> +\t\tOPT_STRING(0, \"expire-to\", &cfg.repack_expire_to, N_(\"dir\"),\n> +\t\t\t N_(\"pack prefix to store a pack containing pruned objects\")),\n> \t\tOPT_END()\n> \t};\n\nOK.\n\n> diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\n> index ee074b99b70..d4b0653a9b7 100755\n> --- a/t/t6500-gc.sh\n> +++ b/t/t6500-gc.sh\n> @@ -339,6 +339,12 @@ test_expect_success 'gc.maxCruftSize sets appropriate repack options' '\n> \ttest_subcommand $cruft_max_size_opts --max-cruft-size=3145728 '\n> \n> +test_expect_success '--expire-to sets appropriate repack options' '\n> +\tmkdir expired &&\n> +\tGIT_TRACE2_EVENT=$(pwd)/trace2.txt git -C cruft--max-size gc --cruft --expire-to=./expired/pack &&\n> +\ttest_subcommand $cruft_max_size_opts --expire-to=./expired/pack +'\n\nAs \"--cruft\" is on by default, the command line does not have to\nhave it, but being explicit is good.\n\nShould we also see what happens when \"--no-cruft\" is given?\n\nThanks.\n"},{"id":"511105","messageId":"CAOLTT8RFqseTSMFXjw--d2BBVAb=7x_mrOaJr4VHcio99H2rMg@mail.gmail.com","threadId":"62691","inReplyTo":"xmqqzfjqr526.fsf@gitster.g","subject":"Re: [PATCH v3] gc: add `--expire-to` option","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2025-01-23T03:42:35Z","receivedAt":"2025-01-23T03:42:48Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano 于2025年1月17日周五 02:23写道：\n>\n> \"ZheNing Hu via GitGitGadget\" writes:\n>\n> > From: ZheNing Hu \n> >\n> > This commit extends the functionality of `git gc`\n> > by adding a new option, `--expire-to=`. Previously,\n> > this feature was implemented in `git repack` (see 91badeb),\n> > allowing users to specify a directory where unreachable and\n> > expired cruft packs are stored during garbage collection.\n> > However, users had to run `git repack --cruft --expire-to=`\n> > followed by `git prune` to achieve similar results within `git gc`.\n> >\n> > By introducing `--expire-to=` directly into `git gc`,\n> > we simplify the process for users who wish to manage their\n> > repository's cleanup more efficiently. This change involves\n> > passing the `--expire-to=` parameter through to `git repack`,\n> > making it easier for users to set up a backup location for cruft\n> > packs that will be pruned.\n>\n> Today I do not have enough time to do my usual commit log message\n> critique. Please use \"git show -s --format=reference\" when\n> referring to an earlier commit.\n>\n\nOkay, I will change to using this format.\n\n> > Note: When git-gc is used with both `--cruft` and `--expire-to`,\n> > it does not pass `-a` to git-repack to delete all unreachable\n> > objects as `git gc --prune=now` originally did. Instead, it\n> > generates a cruft pack in the directory specified by expire-to.\n>\n> Is this less important than \"we added --expire-to to gc that is\n> passed down to underlying repack\" in the previous paragraph?\n>\n\nI had thought that adding --expire-to to gc was key in this patch,\nbut the change to the implementation of --prune=now should\nindeed be mentioned more.\n\n> Not removing the unreachables too early with \"repack -a\" is an\n> essential part of the design of this new feature to allow us not to\n> lose the cruft objects, so I was a bit surprised that this was\n> described as a \"Note:\".\n>\n\nYou're right. This section shouldn't use a note; it should provide\na more detailed explanation instead.\n\n> > diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt\n> > index 370e22faaeb..b4c0cf02972 100644\n> > --- a/Documentation/git-gc.txt\n> > +++ b/Documentation/git-gc.txt\n> > @@ -69,6 +69,12 @@ be performed as well.\n> > the `--max-cruft-size` option of linkgit:git-repack[1] for\n> > more.\n> >\n> > +--expire-to=::\n> > + When packing unreachable objects into a cruft pack, write a cruft\n> > + pack containing pruned objects (if any) to the directory ``.\n> > + See the `--expire-to` option of linkgit:git-repack[1] for\n> > + more.\n>\n> Does \"When packing unreachable objects into a cruft pack\" mean that\n> this option is only meaningful with \"--cruft\"? As \"--cruft\" is on\n> by default, is it an error to pass \"--no-cruft\" when you use this\n> option?\n>\n\nIt (--expired-to) can currently only be used together with --cruft.\nUsing --no-cruft together with --expire-to will not result in an error,\nbut --expired-to will not take effect either.\n\nI should mention in the document that --expire-to and --cruft\nneed to be used together, otherwise --expire-to will not\nhave any effect.\n\n> \"for more\" -> \"for more information\" or something?\n>\n\nOK, \"for more information\".\n\n> > diff --git a/builtin/gc.c b/builtin/gc.c\n> > index d52735354c9..8656e1caff0 100644\n> > --- a/builtin/gc.c\n> > +++ b/builtin/gc.c\n> > @@ -136,6 +136,7 @@ struct gc_config {\n> > char *prune_worktrees_expire;\n> > char *repack_filter;\n> > char *repack_filter_to;\n> > + char *repack_expire_to;\n> > unsigned long big_pack_threshold;\n> > unsigned long max_delta_cache_size;\n> > };\n> > @@ -432,7 +433,8 @@ static int keep_one_pack(struct string_list_item *item, void *data UNUSED)\n> > static void add_repack_all_option(struct gc_config *cfg,\n> > struct string_list *keep_pack)\n> > {\n> > - if (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\"))\n> > + if (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\")\n> > + && !(cfg->cruft_packs && cfg->repack_expire_to))\n> > strvec_push(&repack, \"-a\");\n>\n> Hmph. When \"--expire-to=\" is given, we are dropping these\n> unreachable objects right away, but we said \"--no-cruft\", then we\n> say \"repack -a\". If we have both \"--cruft\" and \"--expire-to=\",\n> then ...\n>\n> > else if (cfg->cruft_packs) {\n> > strvec_push(&repack, \"--cruft\");\n> > @@ -441,6 +443,8 @@ static void add_repack_all_option(struct gc_config *cfg,\n> > if (cfg->max_cruft_size)\n> > strvec_pushf(&repack, \"--max-cruft-size=%lu\",\n> > cfg->max_cruft_size);\n> > + if (cfg->repack_expire_to)\n> > + strvec_pushf(&repack, \"--expire-to=%s\", cfg->repack_expire_to);\n>\n> ... we do the usual \"repack --cruft --expire-to=\" in the next\n> block.\n>\n> > @@ -675,7 +679,6 @@ struct repository *repo UNUSED)\n> > const char *prune_expire_sentinel = \"sentinel\";\n> > const char *prune_expire_arg = prune_expire_sentinel;\n> > int ret;\n> > -\n> > struct option builtin_gc_options[] = {\n> > OPT__QUIET(&quiet, N_(\"suppress progress reporting\")),\n> > { OPTION_STRING, 0, \"prune\", &prune_expire_arg, N_(\"date\"),\n>\n> OK.\n>\n> > @@ -694,6 +697,8 @@ struct repository *repo UNUSED)\n> > PARSE_OPT_NOCOMPLETE),\n> > OPT_BOOL(0, \"keep-largest-pack\", &keep_largest_pack,\n> > N_(\"repack all other packs except the largest pack\")),\n> > + OPT_STRING(0, \"expire-to\", &cfg.repack_expire_to, N_(\"dir\"),\n> > + N_(\"pack prefix to store a pack containing pruned objects\")),\n> > OPT_END()\n> > };\n>\n> OK.\n>\n> > diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\n> > index ee074b99b70..d4b0653a9b7 100755\n> > --- a/t/t6500-gc.sh\n> > +++ b/t/t6500-gc.sh\n> > @@ -339,6 +339,12 @@ test_expect_success 'gc.maxCruftSize sets appropriate repack options' '\n> > test_subcommand $cruft_max_size_opts --max-cruft-size=3145728 > '\n> >\n> > +test_expect_success '--expire-to sets appropriate repack options' '\n> > + mkdir expired &&\n> > + GIT_TRACE2_EVENT=$(pwd)/trace2.txt git -C cruft--max-size gc --cruft --expire-to=./expired/pack &&\n> > + test_subcommand $cruft_max_size_opts --expire-to=./expired/pack > +'\n>\n> As \"--cruft\" is on by default, the command line does not have to\n> have it, but being explicit is good.\n>\n> Should we also see what happens when \"--no-cruft\" is given?\n>\n\n --expire-to with --no-cruft will still run repack -a, I will add\ncorresponding tests.\n\n> Thanks.\n\nThanks.\n"},{"id":"511143","messageId":"pull.1843.v4.git.1737704954987.gitgitgadget@gmail.com","threadId":"62691","inReplyTo":"pull.1843.v3.git.1736994932003.gitgitgadget@gmail.com","subject":"[PATCH v4] gc: add `--expire-to` option","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-01-24T07:49:14Z","receivedAt":"2025-01-24T07:49:19Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nThis commit extends the functionality of `git gc`\nby adding a new option, `--expire-to=`. Previously,\nthis feature was implemented in 91badeba32 (builtin/repack.c:\nimplement `--expire-to` for storing pruned objects, 2022-10-24),\nwhich allowing users to specify a directory where unreachable\nand expired cruft packs are stored during garbage collection.\nHowever, users had to run `git repack --cruft --expire-to=`\nfollowed by `git prune` to achieve similar results within `git gc`.\n\nBy introducing `--expire-to=` directly into `git gc`,\nwe simplify the process for users who wish to manage their\nrepository's cleanup more efficiently. This change involves\npassing the `--expire-to=` parameter through to `git repack`,\nmaking it easier for users to set up a backup location for cruft\npacks that will be pruned.\n\nDue to the original `git gc --prune=now` deleting all unreachable\nobjects by passing the `-a` parameter to git repack. With the\naddition of the `--cruft` and `--expire-to` options, it is necessary\nto modify this default behavior: instead of deleting these\nunreachable objects, they should be merged into a cruft pack and\ncollected in a specified directory. Therefore, we do not pass `-a`\nto the repack command but instead pass `--cruft`, `--expire-to`,\nand `--cruft-expiration=now` to repack.\n\nSigned-off-by: ZheNing Hu \n---\n gc: add --expire-to option\n \n I want to perform a \"safe\" garbage collection for the Git repository on\n the server, which avoids data corruption issues caused by concurrent\n pushes during git GC. To achieve this, I currently need to use git\n repack --cruft --expire-to= and git prune in combination. However,\n it would be simpler if we could directly use --expire-to= with the\n git-gc command.\n \n v1: add --expire-to option to gc v1 -> v2: fix git gc --prune=now with\n --expire-to v2 -> v3: squash two patch into one patch v3 -> v4: modify\n docs, commit message, and give more tests\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1843%2Fadlternative%2Fzh%2Fgc-expire-to-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1843/adlternative/zh/gc-expire-to-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/1843\n\nRange-diff vs v3:\n\n 1: 0842ec34948 ! 1: 6946ccde275 gc: add `--expire-to` option\n @@ Commit message\n \n This commit extends the functionality of `git gc`\n by adding a new option, `--expire-to=`. Previously,\n - this feature was implemented in `git repack` (see 91badeb),\n - allowing users to specify a directory where unreachable and\n - expired cruft packs are stored during garbage collection.\n + this feature was implemented in 91badeba32 (builtin/repack.c:\n + implement `--expire-to` for storing pruned objects, 2022-10-24),\n + which allowing users to specify a directory where unreachable\n + and expired cruft packs are stored during garbage collection.\n However, users had to run `git repack --cruft --expire-to=`\n followed by `git prune` to achieve similar results within `git gc`.\n \n @@ Commit message\n making it easier for users to set up a backup location for cruft\n packs that will be pruned.\n \n - Note: When git-gc is used with both `--cruft` and `--expire-to`,\n - it does not pass `-a` to git-repack to delete all unreachable\n - objects as `git gc --prune=now` originally did. Instead, it\n - generates a cruft pack in the directory specified by expire-to.\n + Due to the original `git gc --prune=now` deleting all unreachable\n + objects by passing the `-a` parameter to git repack. With the\n + addition of the `--cruft` and `--expire-to` options, it is necessary\n + to modify this default behavior: instead of deleting these\n + unreachable objects, they should be merged into a cruft pack and\n + collected in a specified directory. Therefore, we do not pass `-a`\n + to the repack command but instead pass `--cruft`, `--expire-to`,\n + and `--cruft-expiration=now` to repack.\n \n Signed-off-by: ZheNing Hu \n \n @@ Documentation/git-gc.txt: be performed as well.\n +--expire-to=::\n +\tWhen packing unreachable objects into a cruft pack, write a cruft\n +\tpack containing pruned objects (if any) to the directory ``.\n ++\tThis option only has an effect when used together with `--cruft`.\n +\tSee the `--expire-to` option of linkgit:git-repack[1] for\n -+\tmore.\n ++\tmore information.\n +\n --prune=::\n \tPrune loose objects older than date (default is 2 weeks ago,\n @@ t/t6500-gc.sh: test_expect_success 'gc.maxCruftSize sets appropriate repack opti\n \ttest_subcommand $cruft_max_size_opts --max-cruft-size=3145728 ::\n+\tWhen packing unreachable objects into a cruft pack, write a cruft\n+\tpack containing pruned objects (if any) to the directory ``.\n+\tThis option only has an effect when used together with `--cruft`.\n+\tSee the `--expire-to` option of linkgit:git-repack[1] for\n+\tmore information.\n+\n --prune=::\n \tPrune loose objects older than date (default is 2 weeks ago,\n \toverridable by the config variable `gc.pruneExpire`).\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex d52735354c9..8656e1caff0 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -136,6 +136,7 @@ struct gc_config {\n \tchar *prune_worktrees_expire;\n \tchar *repack_filter;\n \tchar *repack_filter_to;\n+\tchar *repack_expire_to;\n \tunsigned long big_pack_threshold;\n \tunsigned long max_delta_cache_size;\n };\n@@ -432,7 +433,8 @@ static int keep_one_pack(struct string_list_item *item, void *data UNUSED)\n static void add_repack_all_option(struct gc_config *cfg,\n \t\t\t\t struct string_list *keep_pack)\n {\n-\tif (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\"))\n+\tif (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\")\n+\t\t&& !(cfg->cruft_packs && cfg->repack_expire_to))\n \t\tstrvec_push(&repack, \"-a\");\n \telse if (cfg->cruft_packs) {\n \t\tstrvec_push(&repack, \"--cruft\");\n@@ -441,6 +443,8 @@ static void add_repack_all_option(struct gc_config *cfg,\n \t\tif (cfg->max_cruft_size)\n \t\t\tstrvec_pushf(&repack, \"--max-cruft-size=%lu\",\n \t\t\t\t cfg->max_cruft_size);\n+\t\tif (cfg->repack_expire_to)\n+\t\t\tstrvec_pushf(&repack, \"--expire-to=%s\", cfg->repack_expire_to);\n \t} else {\n \t\tstrvec_push(&repack, \"-A\");\n \t\tif (cfg->prune_expire)\n@@ -675,7 +679,6 @@ struct repository *repo UNUSED)\n \tconst char *prune_expire_sentinel = \"sentinel\";\n \tconst char *prune_expire_arg = prune_expire_sentinel;\n \tint ret;\n-\n \tstruct option builtin_gc_options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"suppress progress reporting\")),\n \t\t{ OPTION_STRING, 0, \"prune\", &prune_expire_arg, N_(\"date\"),\n@@ -694,6 +697,8 @@ struct repository *repo UNUSED)\n \t\t\t PARSE_OPT_NOCOMPLETE),\n \t\tOPT_BOOL(0, \"keep-largest-pack\", &keep_largest_pack,\n \t\t\t N_(\"repack all other packs except the largest pack\")),\n+\t\tOPT_STRING(0, \"expire-to\", &cfg.repack_expire_to, N_(\"dir\"),\n+\t\t\t N_(\"pack prefix to store a pack containing pruned objects\")),\n \t\tOPT_END()\n \t};\n \ndiff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\nindex ee074b99b70..74f7bd09046 100755\n--- a/t/t6500-gc.sh\n+++ b/t/t6500-gc.sh\n@@ -339,6 +339,39 @@ test_expect_success 'gc.maxCruftSize sets appropriate repack options' '\n \ttest_subcommand $cruft_max_size_opts --max-cruft-size=3145728 writes:\n\n> From: ZheNing Hu \n>\n> This commit extends the functionality of `git gc`\n> by adding a new option, `--expire-to=`. Previously,\n> this feature was implemented in 91badeba32 (builtin/repack.c:\n> implement `--expire-to` for storing pruned objects, 2022-10-24),\n> which allowing users to specify a directory where unreachable\n> and expired cruft packs are stored during garbage collection.\n> However, users had to run `git repack --cruft --expire-to=`\n> followed by `git prune` to achieve similar results within `git gc`.\n>\n> By introducing `--expire-to=` directly into `git gc`,\n> we simplify the process for users who wish to manage their\n> repository's cleanup more efficiently. This change involves\n> passing the `--expire-to=` parameter through to `git repack`,\n> making it easier for users to set up a backup location for cruft\n> packs that will be pruned.\n>\n> Due to the original `git gc --prune=now` deleting all unreachable\n> objects by passing the `-a` parameter to git repack. With the\n> addition of the `--cruft` and `--expire-to` options, it is necessary\n> to modify this default behavior: instead of deleting these\n> unreachable objects, they should be merged into a cruft pack and\n> collected in a specified directory. Therefore, we do not pass `-a`\n> to the repack command but instead pass `--cruft`, `--expire-to`,\n> and `--cruft-expiration=now` to repack.\n>\n> Signed-off-by: ZheNing Hu \n> ---\n\nThis hasn't seen any reaction for a while.\n\nDoes anybody have further comments? Otherwise let's mark it for\n'next'.\n\nThanks.\n\n> Documentation/git-gc.txt | 7 +++++++\n> builtin/gc.c | 9 +++++++--\n> t/t6500-gc.sh | 33 +++++++++++++++++++++++++++++++++\n> 3 files changed, 47 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt\n> index 370e22faaeb..0eac8e85f08 100644\n> --- a/Documentation/git-gc.txt\n> +++ b/Documentation/git-gc.txt\n> @@ -69,6 +69,13 @@ be performed as well.\n> \tthe `--max-cruft-size` option of linkgit:git-repack[1] for\n> \tmore.\n> \n> +--expire-to=::\n> +\tWhen packing unreachable objects into a cruft pack, write a cruft\n> +\tpack containing pruned objects (if any) to the directory ``.\n> +\tThis option only has an effect when used together with `--cruft`.\n> +\tSee the `--expire-to` option of linkgit:git-repack[1] for\n> +\tmore information.\n> +\n> --prune=::\n> \tPrune loose objects older than date (default is 2 weeks ago,\n> \toverridable by the config variable `gc.pruneExpire`).\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index d52735354c9..8656e1caff0 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -136,6 +136,7 @@ struct gc_config {\n> \tchar *prune_worktrees_expire;\n> \tchar *repack_filter;\n> \tchar *repack_filter_to;\n> +\tchar *repack_expire_to;\n> \tunsigned long big_pack_threshold;\n> \tunsigned long max_delta_cache_size;\n> };\n> @@ -432,7 +433,8 @@ static int keep_one_pack(struct string_list_item *item, void *data UNUSED)\n> static void add_repack_all_option(struct gc_config *cfg,\n> \t\t\t\t struct string_list *keep_pack)\n> {\n> -\tif (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\"))\n> +\tif (cfg->prune_expire && !strcmp(cfg->prune_expire, \"now\")\n> +\t\t&& !(cfg->cruft_packs && cfg->repack_expire_to))\n> \t\tstrvec_push(&repack, \"-a\");\n> \telse if (cfg->cruft_packs) {\n> \t\tstrvec_push(&repack, \"--cruft\");\n> @@ -441,6 +443,8 @@ static void add_repack_all_option(struct gc_config *cfg,\n> \t\tif (cfg->max_cruft_size)\n> \t\t\tstrvec_pushf(&repack, \"--max-cruft-size=%lu\",\n> \t\t\t\t cfg->max_cruft_size);\n> +\t\tif (cfg->repack_expire_to)\n> +\t\t\tstrvec_pushf(&repack, \"--expire-to=%s\", cfg->repack_expire_to);\n> \t} else {\n> \t\tstrvec_push(&repack, \"-A\");\n> \t\tif (cfg->prune_expire)\n> @@ -675,7 +679,6 @@ struct repository *repo UNUSED)\n> \tconst char *prune_expire_sentinel = \"sentinel\";\n> \tconst char *prune_expire_arg = prune_expire_sentinel;\n> \tint ret;\n> -\n> \tstruct option builtin_gc_options[] = {\n> \t\tOPT__QUIET(&quiet, N_(\"suppress progress reporting\")),\n> \t\t{ OPTION_STRING, 0, \"prune\", &prune_expire_arg, N_(\"date\"),\n> @@ -694,6 +697,8 @@ struct repository *repo UNUSED)\n> \t\t\t PARSE_OPT_NOCOMPLETE),\n> \t\tOPT_BOOL(0, \"keep-largest-pack\", &keep_largest_pack,\n> \t\t\t N_(\"repack all other packs except the largest pack\")),\n> +\t\tOPT_STRING(0, \"expire-to\", &cfg.repack_expire_to, N_(\"dir\"),\n> +\t\t\t N_(\"pack prefix to store a pack containing pruned objects\")),\n> \t\tOPT_END()\n> \t};\n> \n> diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\n> index ee074b99b70..74f7bd09046 100755\n> --- a/t/t6500-gc.sh\n> +++ b/t/t6500-gc.sh\n> @@ -339,6 +339,39 @@ test_expect_success 'gc.maxCruftSize sets appropriate repack options' '\n> \ttest_subcommand $cruft_max_size_opts --max-cruft-size=3145728 '\n> \n> +test_expect_success '--expire-to sets repack --expire-to' '\n> +\trm -rf expired &&\n> +\tmkdir expired &&\n> +\texpire_to=\"$(pwd)/expired/pack\" &&\n> +\tGIT_TRACE2_EVENT=$(pwd)/trace2.txt git -C cruft--max-size gc --cruft --expire-to=\"$expire_to\" &&\n> +\ttest_subcommand $cruft_max_size_opts --expire-to=\"$expire_to\" +'\n> +\n> +test_expect_success '--expire-to with --prune=now sets repack --expire-to' '\n> +\trm -rf expired &&\n> +\tmkdir expired &&\n> +\texpire_to=\"$(pwd)/expired/pack\" &&\n> +\tGIT_TRACE2_EVENT=$(pwd)/trace2.txt git -C cruft--max-size gc --cruft --prune=now --expire-to=\"$expire_to\" &&\n> +\ttest_subcommand git repack -d -l --cruft --cruft-expiration=now --expire-to=\"$expire_to\" +'\n> +\n> +\n> +test_expect_success '--expire-to with --no-cruft sets repack -A' '\n> +\trm -rf expired &&\n> +\tmkdir expired &&\n> +\texpire_to=\"$(pwd)/expired/pack\" &&\n> +\tGIT_TRACE2_EVENT=$(pwd)/trace2.txt git -C cruft--max-size gc --no-cruft --expire-to=\"$expire_to\" &&\n> +\ttest_subcommand git repack -d -l -A --unpack-unreachable=2.weeks.ago +'\n> +\n> +test_expect_success '--expire-to with --no-cruft sets repack -a' '\n> +\trm -rf expired &&\n> +\tmkdir expired &&\n> +\texpire_to=\"$(pwd)/expired/pack\" &&\n> +\tGIT_TRACE2_EVENT=$(pwd)/trace2.txt git -C cruft--max-size gc --no-cruft --prune=now --expire-to=\"$expire_to\" &&\n> +\ttest_subcommand git repack -d -l -a +'\n> +\n> run_and_wait_for_gc () {\n> \t# We read stdout from gc for the side effect of waiting until the\n> \t# background gc process exits, closing its fd 9. Furthermore, the\n>\n> base-commit: 92999a42db1c5f43f330e4f2bca4026b5b81576f\n"}]}