{"thread":{"id":"55119","subject":"[PATCH 0/2] Maintenance: add pack-refs task","startedAt":"2021-02-08T14:53:53Z","lastAt":"2021-02-10T02:45:15Z","messageCount":12,"participants":["Derrick Stolee via GitGitGadget","Taylor Blau","Eric Sunshine","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"416413","messageId":"pull.871.git.1612795943.gitgitgadget@gmail.com","threadId":"55119","inReplyTo":null,"subject":"[PATCH 0/2] Maintenance: add pack-refs task","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-08T14:52:21Z","receivedAt":"2021-02-08T14:53:53Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This patch series adds a new pack-refs task to the maintenance builtin. This\noperation already happens within git gc (and hence the gc task) but it is\neasy to extract. Packing refs does not delete any data, only collects loose\nobjects into a combined file. This makes things faster in subtle ways,\nespecially when a command needs to iterate through refs (especially tags).\n\nCredit for inspiring this goes to Suolong, who asked for this to be added to\nScalar [1]. I've been waiting instead to add it directly to Git and its\nbackground maintenance. Now is the time!\n\n[1] https://github.com/microsoft/scalar/issues/382\n\nI chose to add it to the incremental maintenance strategy at a weekly\ncadence. I'm not sure there is significant value to the difference between\nweekly and daily. It just seems to me that weekly is often enough. Feel free\nto correct me if you have a different opinion.\n\nMy hope is that this patch series could be used as an example for further\nextracting tasks out of the gc task and making them be full maintenance\ntasks. Doing more of these extractions could be a good project for a new\ncontributor.\n\nOne thing that is not implemented in this series is a notion of the behavior\nfor the pack-refs task during git maintenance run --auto. This could be\nadded in the future, but I wanted to focus on getting this behavior into the\nincremental maintenance schedule.\n\nThanks, -Stolee\n\nDerrick Stolee (2):\n maintenance: add pack-refs task\n maintenance: incremental strategy runs pack-refs weekly\n\n Documentation/config/maintenance.txt | 5 +++--\n Documentation/git-maintenance.txt | 6 ++++++\n builtin/gc.c | 23 +++++++++++++++++++----\n t/t7900-maintenance.sh | 24 ++++++++++++++++++++++++\n 4 files changed, 52 insertions(+), 6 deletions(-)\n\n\nbase-commit: fb7fa4a1fd273f22efcafdd13c7f897814fd1eb9\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-871%2Fderrickstolee%2Fmaintenance%2Fpack-refs-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-871/derrickstolee/maintenance/pack-refs-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/871\n-- \ngitgitgadget\n"},{"id":"416414","messageId":"8012d2dc1420b71404b8db23a88474b5fabaed83.1612795943.git.gitgitgadget@gmail.com","threadId":"55119","inReplyTo":"pull.871.git.1612795943.gitgitgadget@gmail.com","subject":"[PATCH 2/2] maintenance: incremental strategy runs pack-refs weekly","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-08T14:52:23Z","receivedAt":"2021-02-08T14:54:29Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen the 'maintenance.strategy' config option is set to 'incremental',\na default maintenance schedule is enabled. Add the 'pack-refs' task to\nthat strategy at the weekly cadence.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/config/maintenance.txt | 5 +++--\n builtin/gc.c | 2 ++\n t/t7900-maintenance.sh | 14 ++++++++++++++\n 3 files changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config/maintenance.txt b/Documentation/config/maintenance.txt\nindex a5ead09e4bc2..18f056213145 100644\n--- a/Documentation/config/maintenance.txt\n+++ b/Documentation/config/maintenance.txt\n@@ -15,8 +15,9 @@ maintenance.strategy::\n * `none`: This default setting implies no task are run at any schedule.\n * `incremental`: This setting optimizes for performing small maintenance\n activities that do not delete any data. This does not schedule the `gc`\n- task, but runs the `prefetch` and `commit-graph` tasks hourly and the\n- `loose-objects` and `incremental-repack` tasks daily.\n+ task, but runs the `prefetch` and `commit-graph` tasks hourly, the\n+ `loose-objects` and `incremental-repack` tasks daily, and the `pack-refs`\n+ task weekly.\n \n maintenance..enabled::\n \tThis boolean config option controls whether the maintenance task\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 8f13c3bb8607..8d365a59d027 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -1352,6 +1352,8 @@ static void initialize_maintenance_strategy(void)\n \t\ttasks[TASK_INCREMENTAL_REPACK].schedule = SCHEDULE_DAILY;\n \t\ttasks[TASK_LOOSE_OBJECTS].enabled = 1;\n \t\ttasks[TASK_LOOSE_OBJECTS].schedule = SCHEDULE_DAILY;\n+\t\ttasks[TASK_PACK_REFS].enabled = 1;\n+\t\ttasks[TASK_PACK_REFS].schedule = SCHEDULE_WEEKLY;\n \t}\n }\n \ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex ef4527823d29..fa92e01fb2c5 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -406,18 +406,32 @@ test_expect_success 'maintenance.strategy inheritance' '\n \t\tgit maintenance run --schedule=hourly --quiet &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/incremental-daily.txt\" \\\n \t\tgit maintenance run --schedule=daily --quiet &&\n+\tGIT_TRACE2_EVENT=\"$(pwd)/incremental-weekly.txt\" \\\n+\t\tgit maintenance run --schedule=weekly --quiet &&\n \n \ttest_subcommand git commit-graph write --split --reachable \\\n \t\t--no-progress \n\nIt is valuable to collect loose refs into a more compressed form. This\nis typically the packed-refs file, although this could be the reftable\nin the future. Having packed refs can be extremely valuable in repos\nwith many tags or remote branches that are not modified by the local\nuser, but still are necessary for other queries.\n\nFor instance, with many exploded refs, commands such as\n\n\tgit describe --tags --exact-match HEAD\n\ncan be very slow (multiple seconds). This command in particular is used\nby terminal prompts to show when a detatched HEAD is pointing to an\nexisting tag, so having it be slow causes significant delays for users.\n\nAdd a new 'pack-refs' maintenance task. This is already a sub-step of\nthe 'gc' task, but users could run this at other intervals if they are\ninterested. Also, if users opt-in to the default background maintenance\nschedule, then the 'gc' task is disabled.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-maintenance.txt | 6 ++++++\n builtin/gc.c | 21 +++++++++++++++++----\n t/t7900-maintenance.sh | 10 ++++++++++\n 3 files changed, 33 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt\nindex 3b432171d608..8cc0225b3304 100644\n--- a/Documentation/git-maintenance.txt\n+++ b/Documentation/git-maintenance.txt\n@@ -145,6 +145,12 @@ incremental-repack::\n \twhich is a special case that attempts to repack all pack-files\n \tinto a single pack-file.\n \n+pack-refs::\n+\tThe `pack-refs` task collects the loose reference files and\n+\tcollects them into a single file. This speeds up operations that\n+\tneed to iterate across many refereences. See linkgit:git-pack-refs[1]\n+\tfor more information.\n+\n OPTIONS\n -------\n --auto::\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 4c40594d660e..8f13c3bb8607 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -54,7 +54,6 @@ static const char *prune_worktrees_expire = \"3.months.ago\";\n static unsigned long big_pack_threshold;\n static unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;\n \n-static struct strvec pack_refs_cmd = STRVEC_INIT;\n static struct strvec reflog = STRVEC_INIT;\n static struct strvec repack = STRVEC_INIT;\n static struct strvec prune = STRVEC_INIT;\n@@ -163,6 +162,15 @@ static void gc_config(void)\n \tgit_config(git_default_config, NULL);\n }\n \n+struct maintenance_run_opts;\n+static int maintenance_task_pack_refs(struct maintenance_run_opts *opts)\n+{\n+\tstruct strvec pack_refs_cmd = STRVEC_INIT;\n+\tstrvec_pushl(&pack_refs_cmd, \"pack-refs\", \"--all\", \"--prune\", NULL);\n+\n+\treturn run_command_v_opt(pack_refs_cmd.v, RUN_GIT_CMD);\n+}\n+\n static int too_many_loose_objects(void)\n {\n \t/*\n@@ -518,8 +526,8 @@ static void gc_before_repack(void)\n \tif (done++)\n \t\treturn;\n \n-\tif (pack_refs && run_command_v_opt(pack_refs_cmd.v, RUN_GIT_CMD))\n-\t\tdie(FAILED_RUN, pack_refs_cmd.v[0]);\n+\tif (pack_refs && maintenance_task_pack_refs(NULL))\n+\t\tdie(FAILED_RUN, \"pack-refs\");\n \n \tif (prune_reflogs && run_command_v_opt(reflog.v, RUN_GIT_CMD))\n \t\tdie(FAILED_RUN, reflog.v[0]);\n@@ -556,7 +564,6 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n \t\tusage_with_options(builtin_gc_usage, builtin_gc_options);\n \n-\tstrvec_pushl(&pack_refs_cmd, \"pack-refs\", \"--all\", \"--prune\", NULL);\n \tstrvec_pushl(&reflog, \"reflog\", \"expire\", \"--all\", NULL);\n \tstrvec_pushl(&repack, \"repack\", \"-d\", \"-l\", NULL);\n \tstrvec_pushl(&prune, \"prune\", \"--expire\", NULL);\n@@ -1224,6 +1231,7 @@ enum maintenance_task_label {\n \tTASK_INCREMENTAL_REPACK,\n \tTASK_GC,\n \tTASK_COMMIT_GRAPH,\n+\tTASK_PACK_REFS,\n \n \t/* Leave as final value */\n \tTASK__COUNT\n@@ -1255,6 +1263,11 @@ static struct maintenance_task tasks[] = {\n \t\tmaintenance_task_commit_graph,\n \t\tshould_write_commit_graph,\n \t},\n+\t[TASK_PACK_REFS] = {\n+\t\t\"pack-refs\",\n+\t\tmaintenance_task_pack_refs,\n+\t\tNULL,\n+\t},\n };\n \n static int compare_tasks_by_selection(const void *a_, const void *b_)\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 78ccf4b33f87..ef4527823d29 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -343,6 +343,16 @@ test_expect_success 'maintenance.incremental-repack.auto' '\n \ttest_subcommand git multi-pack-index write --no-progress after &&\n+\ttest_must_be_empty after\n+'\n+\n test_expect_success '--auto and --schedule incompatible' '\n \ttest_must_fail git maintenance run --auto --schedule=daily 2>err &&\n \ttest_i18ngrep \"at most one\" err\n-- \ngitgitgadget\n\n"},{"id":"416454","messageId":"YCG/X12qKVKu3/pv@nand.local","threadId":"55119","inReplyTo":"pull.871.git.1612795943.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Maintenance: add pack-refs task","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-02-08T22:46:55Z","receivedAt":"2021-02-08T22:47:47Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Feb 08, 2021 at 02:52:21PM +0000, Derrick Stolee via GitGitGadget wrote:\n> I chose to add it to the incremental maintenance strategy at a weekly\n> cadence. I'm not sure there is significant value to the difference between\n> weekly and daily. It just seems to me that weekly is often enough. Feel free\n> to correct me if you have a different opinion.\n\nI don't have any strong opinion about this. At GitHub we run pack-refs\nas part of a maintenance-like job, which varies in frequency between\ndifferent repositories based on a number of factors. In our busiest\nrepository networks, this is almost certainly at least a few times a\nday, if not \"as often as possible.\"\n\nI'd suspect that for any laptop-sized repository, anywhere between daily\nand weekly is just fine.\n\nI suppose weekly is a good starting point, since we can always change\nthe default to run more often if enough users complain.\n\n> My hope is that this patch series could be used as an example for further\n> extracting tasks out of the gc task and making them be full maintenance\n> tasks. Doing more of these extractions could be a good project for a new\n> contributor.\n\nThat's great :-).\n\nThanks,\nTaylor\n"},{"id":"416455","messageId":"YCHA991dv0V0hNb+@nand.local","threadId":"55119","inReplyTo":"33b7a74af4eb45756c3648eb7b002d06e4ec3563.1612795943.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] maintenance: add pack-refs task","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-02-08T22:53:43Z","receivedAt":"2021-02-08T22:54:35Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Feb 08, 2021 at 02:52:22PM +0000, Derrick Stolee via GitGitGadget wrote:\n> +pack-refs::\n> +\tThe `pack-refs` task collects the loose reference files and\n> +\tcollects them into a single file. This speeds up operations that\n> +\tneed to iterate across many refereences. See linkgit:git-pack-refs[1]\n> +\tfor more information.\n> +\n\nDo you think it's worth documenting here or in git-gc(1) that running\nthis has the effect of disabling the same step during gc?\n\n> OPTIONS\n> -------\n> --auto::\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index 4c40594d660e..8f13c3bb8607 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -54,7 +54,6 @@ static const char *prune_worktrees_expire = \"3.months.ago\";\n> static unsigned long big_pack_threshold;\n> static unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;\n>\n> -static struct strvec pack_refs_cmd = STRVEC_INIT;\n> static struct strvec reflog = STRVEC_INIT;\n> static struct strvec repack = STRVEC_INIT;\n> static struct strvec prune = STRVEC_INIT;\n> @@ -163,6 +162,15 @@ static void gc_config(void)\n> \tgit_config(git_default_config, NULL);\n> }\n>\n> +struct maintenance_run_opts;\n> +static int maintenance_task_pack_refs(struct maintenance_run_opts *opts)\n\nIt may be worth calling this \"unused\", since you explicitly pass NULL\nbelow.\n\n> +{\n> +\tstruct strvec pack_refs_cmd = STRVEC_INIT;\n> +\tstrvec_pushl(&pack_refs_cmd, \"pack-refs\", \"--all\", \"--prune\", NULL);\n> +\n> +\treturn run_command_v_opt(pack_refs_cmd.v, RUN_GIT_CMD);\n> +}\n> +\n> static int too_many_loose_objects(void)\n> {\n> \t/*\n> @@ -518,8 +526,8 @@ static void gc_before_repack(void)\n> \tif (done++)\n> \t\treturn;\n>\n> -\tif (pack_refs && run_command_v_opt(pack_refs_cmd.v, RUN_GIT_CMD))\n> -\t\tdie(FAILED_RUN, pack_refs_cmd.v[0]);\n> +\tif (pack_refs && maintenance_task_pack_refs(NULL))\n> +\t\tdie(FAILED_RUN, \"pack-refs\");\n\nHmm. Am I missing where opting into the maintenance step disables this\nin gc? You suggest that it does in the commit message, but I can't seem\nto see it here.\n\n> +test_expect_success 'pack-refs task' '\n> +\tfor n in $(test_seq 1 5)\n> +\tdo\n> +\t\tgit branch -f to-pack/$n HEAD || return 1\n> +\tdone &&\n> +\tgit maintenance run --task=pack-refs &&\n> +\tls .git/refs/heads/ >after &&\n> +\ttest_must_be_empty after\n\nWorth testing that $GIT_DIR/packed-refs exists, too?\n\nThanks,\nTaylor\n"},{"id":"416456","messageId":"CAPig+cTOv_Hq-FM2s8-XaGHz6FSUt5Dr41sOB9wP12tQJ_kcmA@mail.gmail.com","threadId":"55119","inReplyTo":"33b7a74af4eb45756c3648eb7b002d06e4ec3563.1612795943.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] maintenance: add pack-refs task","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-02-08T23:06:56Z","receivedAt":"2021-02-08T23:07:54Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Feb 8, 2021 at 9:52 AM Derrick Stolee via GitGitGadget\n wrote:\n> +pack-refs::\n> + The `pack-refs` task collects the loose reference files and\n> + collects them into a single file. This speeds up operations that\n> + need to iterate across many refereences. See linkgit:git-pack-refs[1]\n> + for more information.\n\ns/refereences/references/\n"},{"id":"416500","messageId":"f8df68ee-32a9-4bd1-99d9-4641b582992d@gmail.com","threadId":"55119","inReplyTo":"YCHA991dv0V0hNb+@nand.local","subject":"Re: [PATCH 1/2] maintenance: add pack-refs task","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-02-09T12:42:22Z","receivedAt":"2021-02-09T12:43:35Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 2/8/2021 5:53 PM, Taylor Blau wrote:\n> On Mon, Feb 08, 2021 at 02:52:22PM +0000, Derrick Stolee via GitGitGadget wrote:\n>> +pack-refs::\n>> +\tThe `pack-refs` task collects the loose reference files and\n>> +\tcollects them into a single file. This speeds up operations that\n>> +\tneed to iterate across many refereences. See linkgit:git-pack-refs[1]\n>> +\tfor more information.\n>> +\n> \n> Do you think it's worth documenting here or in git-gc(1) that running\n> this has the effect of disabling the same step during gc?\n\nIt doesn't disable the step during gc.\n\nPerhaps I should use a better term than \"extract\". The 'gc' task shouldn't\nchange as we make some of its steps also available as independent tasks.\n\nInstead, users can select a subset of its steps by enabling them directly.\nOther such tasks could include:\n\n * prune-reflog\n * full-repack (as opposed to the existing incremental-repack task)\n\n>> +struct maintenance_run_opts;\n>> +static int maintenance_task_pack_refs(struct maintenance_run_opts *opts)\n> \n> It may be worth calling this \"unused\", since you explicitly pass NULL\n> below.\n\nGood idea.\n\n>> +\tif (pack_refs && maintenance_task_pack_refs(NULL))\n>> +\t\tdie(FAILED_RUN, \"pack-refs\");\n> \n> Hmm. Am I missing where opting into the maintenance step disables this\n> in gc? You suggest that it does in the commit message, but I can't seem\n> to see it here.\n\nMy commit message suggests wrong.\n \n>> +test_expect_success 'pack-refs task' '\n>> +\tfor n in $(test_seq 1 5)\n>> +\tdo\n>> +\t\tgit branch -f to-pack/$n HEAD || return 1\n>> +\tdone &&\n>> +\tgit maintenance run --task=pack-refs &&\n>> +\tls .git/refs/heads/ >after &&\n>> +\ttest_must_be_empty after\n> \n> Worth testing that $GIT_DIR/packed-refs exists, too?\n\nThat would start breaking if we change the backing store, right? I\nwant to be sure this doesn't create test breakages with reftable.\n\nI _could_ add a 'test_subcommand' check here, though.\n\nThanks,\n-Stolee\n"},{"id":"416501","messageId":"a4bf6692-9e45-fc2e-66e6-5a3671a1daa9@gmail.com","threadId":"55119","inReplyTo":"CAPig+cTOv_Hq-FM2s8-XaGHz6FSUt5Dr41sOB9wP12tQJ_kcmA@mail.gmail.com","subject":"Re: [PATCH 1/2] maintenance: add pack-refs task","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-02-09T12:42:34Z","receivedAt":"2021-02-09T12:44:17Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 2/8/2021 6:06 PM, Eric Sunshine wrote:\n> On Mon, Feb 8, 2021 at 9:52 AM Derrick Stolee via GitGitGadget\n> wrote:\n>> +pack-refs::\n>> + The `pack-refs` task collects the loose reference files and\n>> + collects them into a single file. This speeds up operations that\n>> + need to iterate across many refereences. See linkgit:git-pack-refs[1]\n>> + for more information.\n> \n> s/refereences/references/\n \nThanks!\n-Stolee\n"},{"id":"416506","messageId":"pull.871.v2.git.1612878149.gitgitgadget@gmail.com","threadId":"55119","inReplyTo":"pull.871.git.1612795943.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] Maintenance: add pack-refs task","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-09T13:42:27Z","receivedAt":"2021-02-09T13:45:25Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This patch series adds a new pack-refs task to the maintenance builtin. This\noperation already happens within git gc (and hence the gc task) but it is\neasy to extract. Packing refs does not delete any data, only collects loose\nobjects into a combined file. This makes things faster in subtle ways,\nespecially when a command needs to iterate through refs (especially tags).\n\nCredit for inspiring this goes to Suolong, who asked for this to be added to\nScalar [1]. I've been waiting instead to add it directly to Git and its\nbackground maintenance. Now is the time!\n\n[1] https://github.com/microsoft/scalar/issues/382\n\nI chose to add it to the incremental maintenance strategy at a weekly\ncadence. I'm not sure there is significant value to the difference between\nweekly and daily. It just seems to me that weekly is often enough. Feel free\nto correct me if you have a different opinion.\n\nMy hope is that this patch series could be used as an example for further\nextracting tasks out of the gc task and making them be full maintenance\ntasks. Doing more of these extractions could be a good project for a new\ncontributor.\n\nOne thing that is not implemented in this series is a notion of the behavior\nfor the pack-refs task during git maintenance run --auto. This could be\nadded in the future, but I wanted to focus on getting this behavior into the\nincremental maintenance schedule.\n\n\nUpdates in V2\n=============\n\n * Fixed doc typo. Thanks, Eric!\n * Updated commit messages to make it clear that the 'pack-refs' step will\n still happen within the 'gc' task.\n * Updated the test to check that we run the correct subcommand.\n * maintenance_task_pack_refs() uses MAYBE_UNUSED on its parameter.\n\nThanks, -Stolee\n\nCc: gitster@pobox.com Cc: sluongng@gmail.com Cc: martin.agren@gmail.com Cc:\nsunshine@sunshineco.com\n\nDerrick Stolee (2):\n maintenance: add pack-refs task\n maintenance: incremental strategy runs pack-refs weekly\n\n Documentation/config/maintenance.txt | 5 +++--\n Documentation/git-maintenance.txt | 6 ++++++\n builtin/gc.c | 23 +++++++++++++++++++----\n t/t7900-maintenance.sh | 26 ++++++++++++++++++++++++++\n 4 files changed, 54 insertions(+), 6 deletions(-)\n\n\nbase-commit: fb7fa4a1fd273f22efcafdd13c7f897814fd1eb9\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-871%2Fderrickstolee%2Fmaintenance%2Fpack-refs-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-871/derrickstolee/maintenance/pack-refs-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/871\n\nRange-diff vs v1:\n\n 1: 33b7a74af4eb ! 1: bedaeb548b06 maintenance: add pack-refs task\n @@ Commit message\n by terminal prompts to show when a detatched HEAD is pointing to an\n existing tag, so having it be slow causes significant delays for users.\n \n - Add a new 'pack-refs' maintenance task. This is already a sub-step of\n - the 'gc' task, but users could run this at other intervals if they are\n - interested. Also, if users opt-in to the default background maintenance\n - schedule, then the 'gc' task is disabled.\n + Add a new 'pack-refs' maintenance task. It runs 'git pack-refs --all\n + --prune' to move loose refs into a packed form. For now, that is the\n + packed-refs file, but could adjust to other file formats in the future.\n +\n + This is the first of several sub-tasks of the 'gc' task that could be\n + extracted to their own tasks. In this process, we should not change the\n + behavior of the 'gc' task since that remains the default way to keep\n + repositories maintained. Creating a new task for one of these sub-tasks\n + only provides more customization options for those choosing to not use\n + the 'gc' task. It is certainly possible to have both the 'gc' and\n + 'pack-refs' tasks enabled and run regularly. While they may repeat\n + effort, they do not conflict in a destructive way.\n +\n + The 'auto_condition' function pointer is left NULL for now. We could\n + extend this in the future to have a condition check if pack-refs should\n + be run during 'git maintenance run --auto'.\n \n Signed-off-by: Derrick Stolee \n \n @@ Documentation/git-maintenance.txt: incremental-repack::\n +pack-refs::\n +\tThe `pack-refs` task collects the loose reference files and\n +\tcollects them into a single file. This speeds up operations that\n -+\tneed to iterate across many refereences. See linkgit:git-pack-refs[1]\n ++\tneed to iterate across many references. See linkgit:git-pack-refs[1]\n +\tfor more information.\n +\n OPTIONS\n @@ builtin/gc.c: static void gc_config(void)\n }\n \n +struct maintenance_run_opts;\n -+static int maintenance_task_pack_refs(struct maintenance_run_opts *opts)\n ++static int maintenance_task_pack_refs(MAYBE_UNUSED struct maintenance_run_opts *opts)\n +{\n +\tstruct strvec pack_refs_cmd = STRVEC_INIT;\n +\tstrvec_pushl(&pack_refs_cmd, \"pack-refs\", \"--all\", \"--prune\", NULL);\n @@ t/t7900-maintenance.sh: test_expect_success 'maintenance.incremental-repack.auto\n +\tdo\n +\t\tgit branch -f to-pack/$n HEAD || return 1\n +\tdone &&\n -+\tgit maintenance run --task=pack-refs &&\n ++\tGIT_TRACE2_EVENT=\"$(pwd)/pack-refs.txt\" \\\n ++\t\tgit maintenance run --task=pack-refs &&\n +\tls .git/refs/heads/ >after &&\n -+\ttest_must_be_empty after\n ++\ttest_must_be_empty after &&\n ++\ttest_subcommand git pack-refs --all --prune \n\nIt is valuable to collect loose refs into a more compressed form. This\nis typically the packed-refs file, although this could be the reftable\nin the future. Having packed refs can be extremely valuable in repos\nwith many tags or remote branches that are not modified by the local\nuser, but still are necessary for other queries.\n\nFor instance, with many exploded refs, commands such as\n\n\tgit describe --tags --exact-match HEAD\n\ncan be very slow (multiple seconds). This command in particular is used\nby terminal prompts to show when a detatched HEAD is pointing to an\nexisting tag, so having it be slow causes significant delays for users.\n\nAdd a new 'pack-refs' maintenance task. It runs 'git pack-refs --all\n--prune' to move loose refs into a packed form. For now, that is the\npacked-refs file, but could adjust to other file formats in the future.\n\nThis is the first of several sub-tasks of the 'gc' task that could be\nextracted to their own tasks. In this process, we should not change the\nbehavior of the 'gc' task since that remains the default way to keep\nrepositories maintained. Creating a new task for one of these sub-tasks\nonly provides more customization options for those choosing to not use\nthe 'gc' task. It is certainly possible to have both the 'gc' and\n'pack-refs' tasks enabled and run regularly. While they may repeat\neffort, they do not conflict in a destructive way.\n\nThe 'auto_condition' function pointer is left NULL for now. We could\nextend this in the future to have a condition check if pack-refs should\nbe run during 'git maintenance run --auto'.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-maintenance.txt | 6 ++++++\n builtin/gc.c | 21 +++++++++++++++++----\n t/t7900-maintenance.sh | 12 ++++++++++++\n 3 files changed, 35 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-maintenance.txt b/Documentation/git-maintenance.txt\nindex 3b432171d608..80ddd33ceba0 100644\n--- a/Documentation/git-maintenance.txt\n+++ b/Documentation/git-maintenance.txt\n@@ -145,6 +145,12 @@ incremental-repack::\n \twhich is a special case that attempts to repack all pack-files\n \tinto a single pack-file.\n \n+pack-refs::\n+\tThe `pack-refs` task collects the loose reference files and\n+\tcollects them into a single file. This speeds up operations that\n+\tneed to iterate across many references. See linkgit:git-pack-refs[1]\n+\tfor more information.\n+\n OPTIONS\n -------\n --auto::\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 4c40594d660e..41bec4f177b3 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -54,7 +54,6 @@ static const char *prune_worktrees_expire = \"3.months.ago\";\n static unsigned long big_pack_threshold;\n static unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;\n \n-static struct strvec pack_refs_cmd = STRVEC_INIT;\n static struct strvec reflog = STRVEC_INIT;\n static struct strvec repack = STRVEC_INIT;\n static struct strvec prune = STRVEC_INIT;\n@@ -163,6 +162,15 @@ static void gc_config(void)\n \tgit_config(git_default_config, NULL);\n }\n \n+struct maintenance_run_opts;\n+static int maintenance_task_pack_refs(MAYBE_UNUSED struct maintenance_run_opts *opts)\n+{\n+\tstruct strvec pack_refs_cmd = STRVEC_INIT;\n+\tstrvec_pushl(&pack_refs_cmd, \"pack-refs\", \"--all\", \"--prune\", NULL);\n+\n+\treturn run_command_v_opt(pack_refs_cmd.v, RUN_GIT_CMD);\n+}\n+\n static int too_many_loose_objects(void)\n {\n \t/*\n@@ -518,8 +526,8 @@ static void gc_before_repack(void)\n \tif (done++)\n \t\treturn;\n \n-\tif (pack_refs && run_command_v_opt(pack_refs_cmd.v, RUN_GIT_CMD))\n-\t\tdie(FAILED_RUN, pack_refs_cmd.v[0]);\n+\tif (pack_refs && maintenance_task_pack_refs(NULL))\n+\t\tdie(FAILED_RUN, \"pack-refs\");\n \n \tif (prune_reflogs && run_command_v_opt(reflog.v, RUN_GIT_CMD))\n \t\tdie(FAILED_RUN, reflog.v[0]);\n@@ -556,7 +564,6 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n \t\tusage_with_options(builtin_gc_usage, builtin_gc_options);\n \n-\tstrvec_pushl(&pack_refs_cmd, \"pack-refs\", \"--all\", \"--prune\", NULL);\n \tstrvec_pushl(&reflog, \"reflog\", \"expire\", \"--all\", NULL);\n \tstrvec_pushl(&repack, \"repack\", \"-d\", \"-l\", NULL);\n \tstrvec_pushl(&prune, \"prune\", \"--expire\", NULL);\n@@ -1224,6 +1231,7 @@ enum maintenance_task_label {\n \tTASK_INCREMENTAL_REPACK,\n \tTASK_GC,\n \tTASK_COMMIT_GRAPH,\n+\tTASK_PACK_REFS,\n \n \t/* Leave as final value */\n \tTASK__COUNT\n@@ -1255,6 +1263,11 @@ static struct maintenance_task tasks[] = {\n \t\tmaintenance_task_commit_graph,\n \t\tshould_write_commit_graph,\n \t},\n+\t[TASK_PACK_REFS] = {\n+\t\t\"pack-refs\",\n+\t\tmaintenance_task_pack_refs,\n+\t\tNULL,\n+\t},\n };\n \n static int compare_tasks_by_selection(const void *a_, const void *b_)\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 78ccf4b33f87..4a8a78769bd6 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -343,6 +343,18 @@ test_expect_success 'maintenance.incremental-repack.auto' '\n \ttest_subcommand git multi-pack-index write --no-progress after &&\n+\ttest_must_be_empty after &&\n+\ttest_subcommand git pack-refs --all --prune err &&\n \ttest_i18ngrep \"at most one\" err\n-- \ngitgitgadget\n\n"},{"id":"416508","messageId":"c38fc9a4170ee8fcc9be9d2a081138786847aff3.1612878149.git.gitgitgadget@gmail.com","threadId":"55119","inReplyTo":"pull.871.v2.git.1612878149.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] maintenance: incremental strategy runs pack-refs weekly","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-09T13:42:29Z","receivedAt":"2021-02-09T13:45:43Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen the 'maintenance.strategy' config option is set to 'incremental',\na default maintenance schedule is enabled. Add the 'pack-refs' task to\nthat strategy at the weekly cadence.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/config/maintenance.txt | 5 +++--\n builtin/gc.c | 2 ++\n t/t7900-maintenance.sh | 14 ++++++++++++++\n 3 files changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config/maintenance.txt b/Documentation/config/maintenance.txt\nindex a5ead09e4bc2..18f056213145 100644\n--- a/Documentation/config/maintenance.txt\n+++ b/Documentation/config/maintenance.txt\n@@ -15,8 +15,9 @@ maintenance.strategy::\n * `none`: This default setting implies no task are run at any schedule.\n * `incremental`: This setting optimizes for performing small maintenance\n activities that do not delete any data. This does not schedule the `gc`\n- task, but runs the `prefetch` and `commit-graph` tasks hourly and the\n- `loose-objects` and `incremental-repack` tasks daily.\n+ task, but runs the `prefetch` and `commit-graph` tasks hourly, the\n+ `loose-objects` and `incremental-repack` tasks daily, and the `pack-refs`\n+ task weekly.\n \n maintenance..enabled::\n \tThis boolean config option controls whether the maintenance task\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 41bec4f177b3..6db9cb39e679 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -1352,6 +1352,8 @@ static void initialize_maintenance_strategy(void)\n \t\ttasks[TASK_INCREMENTAL_REPACK].schedule = SCHEDULE_DAILY;\n \t\ttasks[TASK_LOOSE_OBJECTS].enabled = 1;\n \t\ttasks[TASK_LOOSE_OBJECTS].schedule = SCHEDULE_DAILY;\n+\t\ttasks[TASK_PACK_REFS].enabled = 1;\n+\t\ttasks[TASK_PACK_REFS].schedule = SCHEDULE_WEEKLY;\n \t}\n }\n \ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 4a8a78769bd6..286b18db3cc2 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -408,18 +408,32 @@ test_expect_success 'maintenance.strategy inheritance' '\n \t\tgit maintenance run --schedule=hourly --quiet &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/incremental-daily.txt\" \\\n \t\tgit maintenance run --schedule=daily --quiet &&\n+\tGIT_TRACE2_EVENT=\"$(pwd)/incremental-weekly.txt\" \\\n+\t\tgit maintenance run --schedule=weekly --quiet &&\n \n \ttest_subcommand git commit-graph write --split --reachable \\\n \t\t--no-progress Updates in V2\n> =============\n>\n> * Fixed doc typo. Thanks, Eric!\n> * Updated commit messages to make it clear that the 'pack-refs' step will\n> still happen within the 'gc' task.\n> * Updated the test to check that we run the correct subcommand.\n> * maintenance_task_pack_refs() uses MAYBE_UNUSED on its parameter.\n\nThanks, the range-diff looked sane to me.\n\n Reviewed-by: Taylor Blau \n"}]}