{"thread":{"id":"58461","subject":"[PATCH 0/7] midx: ignore cruft pack with `repack`, `expire`","startedAt":"2022-09-20T01:55:44Z","lastAt":"2022-09-22T14:14:38Z","messageCount":11,"participants":["Taylor Blau","Christian Couder","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"463253","messageId":"cover.1663638929.git.me@ttaylorr.com","threadId":"58461","inReplyTo":null,"subject":"[PATCH 0/7] midx: ignore cruft pack with `repack`, `expire`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-09-20T01:55:37Z","receivedAt":"2022-09-20T01:55:44Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"This series fixes a pair of bugs that were originally pointed out by\nGregory Szorc in [1].\n\nNamely, that both `git multi-pack-index repack` and `git\nmulti-pack-index expire` can cause us to \"absorb\" the cruft pack,\ndistributing its objects into a new pack, and removing its metadata.\n\nThis is worth avoiding, since even though it doesn't result in object\ncorruption, this bug removes semi-important metadata contained in the\n.mtimes file, which controls how fast objects leave the repository\nduring a pruning GC.\n\nThis series teaches both sub-commands to avoid any cruft pack(s),\npreserving their metadata.\n\nThanks in advance for your review.\n\n[1]: https://lore.kernel.org/git/CAKQoGanPBec6wRO6uWrETaoJXdszpjRWytXaJwx6jw0mrrj-gQ@mail.gmail.com/\n\nTaylor Blau (7):\n Documentation/git-multi-pack-index.txt: fix typo\n Documentation/git-multi-pack-index.txt: clarify expire behavior\n midx.c: prevent `expire` from removing the cruft pack\n midx.c: avoid cruft packs with `repack --batch-size=0`\n midx.c: replace `xcalloc()` with `CALLOC_ARRAY()`\n midx.c: remove unnecessary loop condition\n midx.c: avoid cruft packs with non-zero `repack --batch-size`\n\n Documentation/git-multi-pack-index.txt | 7 +-\n midx.c | 12 +++-\n t/t5319-multi-pack-index.sh | 94 ++++++++++++++++++++++++++\n 3 files changed, 107 insertions(+), 6 deletions(-)\n\n-- \n2.37.0.1.g1379af2e9d\n"},{"id":"463254","messageId":"3a3c7db81ed16b73a5474f6795afd5d3a36d381b.1663638929.git.me@ttaylorr.com","threadId":"58461","inReplyTo":"cover.1663638929.git.me@ttaylorr.com","subject":"[PATCH 1/7] Documentation/git-multi-pack-index.txt: fix typo","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-09-20T01:55:40Z","receivedAt":"2022-09-20T01:55:49Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Remove the extra space character between \"tracked\" and \"by\", which dates\nback to when this paragraph was originally written in cff9711616\n(multi-pack-index: prepare for 'expire' subcommand, 2019-06-10).\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-multi-pack-index.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt\nindex a48c3d5ea6..b4a2378cd8 100644\n--- a/Documentation/git-multi-pack-index.txt\n+++ b/Documentation/git-multi-pack-index.txt\n@@ -70,7 +70,7 @@ verify::\n \tVerify the contents of the MIDX file.\n \n expire::\n-\tDelete the pack-files that are tracked \tby the MIDX file, but\n+\tDelete the pack-files that are tracked by the MIDX file, but\n \thave no objects referenced by the MIDX. Rewrite the MIDX file\n \tafterward to remove all references to these pack-files.\n \n-- \n2.37.0.1.g1379af2e9d\n\n"},{"id":"463255","messageId":"4c9296c0ea6f5048c429ccb24fc202c1eb58f2ef.1663638929.git.me@ttaylorr.com","threadId":"58461","inReplyTo":"cover.1663638929.git.me@ttaylorr.com","subject":"[PATCH 2/7] Documentation/git-multi-pack-index.txt: clarify expire behavior","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-09-20T01:55:42Z","receivedAt":"2022-09-20T01:55:58Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The `expire` sub-command of `git multi-pack-index` will never expire\n`.keep` packs, regardless of whether or not any of their objects were\nselected in the MIDX.\n\nThis has always been the case since 19575c7c8e (multi-pack-index:\nimplement 'expire' subcommand, 2019-06-10), which came after cff9711616\n(multi-pack-index: prepare for 'expire' subcommand, 2019-06-10), when\nthis documentation was originally written.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-multi-pack-index.txt | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt\nindex b4a2378cd8..11e6dc53e3 100644\n--- a/Documentation/git-multi-pack-index.txt\n+++ b/Documentation/git-multi-pack-index.txt\n@@ -71,8 +71,9 @@ verify::\n \n expire::\n \tDelete the pack-files that are tracked by the MIDX file, but\n-\thave no objects referenced by the MIDX. Rewrite the MIDX file\n-\tafterward to remove all references to these pack-files.\n+\thave no objects referenced by the MIDX (with the exception of\n+\t`.keep` packs). Rewrite the MIDX file afterward to remove all\n+\treferences to these pack-files.\n \n repack::\n \tCreate a new pack-file containing objects in small pack-files\n-- \n2.37.0.1.g1379af2e9d\n\n"},{"id":"463256","messageId":"3ae9903d2df491e291ab975c56ec78aa13d95655.1663638929.git.me@ttaylorr.com","threadId":"58461","inReplyTo":"cover.1663638929.git.me@ttaylorr.com","subject":"[PATCH 3/7] midx.c: prevent `expire` from removing the cruft pack","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-09-20T01:55:45Z","receivedAt":"2022-09-20T01:56:01Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The `expire` sub-command unlinks any packs that are (a) contained in the\nMIDX, but (b) have no objects referenced by the MIDX.\n\nThis sub-command ignores `.keep` packs, which remain on-disk even if\nthey have no objects referenced by the MIDX. Cruft packs, however,\naren't given the same treatment: if none of the objects contained in the\ncruft pack are selected from the cruft pack by the MIDX, then the cruft\npack is eligible to be expired.\n\nThis is less than desireable, since the cruft pack has important\nmetadata about the individual object mtimes, which is useful to\ndetermine how quickly an object should age out of the repository when\npruning.\n\nOrdinarily, we wouldn't expect the contents of a cruft pack to\nduplicated across non-cruft packs (and we'd expect to see the MIDX\nselect all cruft objects from other sources even less often). But\nnonetheless, it is still possible to trick the `expire` sub-command into\nremoving the `.mtimes` file in this circumstance.\n\nTeach the `expire` sub-command to ignore cruft packs in the same manner\nas it does `.keep` packs, in order to keep their metadata around, even\nwhen they are unreferenced by the MIDX.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-multi-pack-index.txt | 4 ++--\n midx.c | 2 +-\n t/t5319-multi-pack-index.sh | 30 ++++++++++++++++++++++++++\n 3 files changed, 33 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt\nindex 11e6dc53e3..3696506eb3 100644\n--- a/Documentation/git-multi-pack-index.txt\n+++ b/Documentation/git-multi-pack-index.txt\n@@ -72,8 +72,8 @@ verify::\n expire::\n \tDelete the pack-files that are tracked by the MIDX file, but\n \thave no objects referenced by the MIDX (with the exception of\n-\t`.keep` packs). Rewrite the MIDX file afterward to remove all\n-\treferences to these pack-files.\n+\t`.keep` packs and cruft packs). Rewrite the MIDX file afterward\n+\tto remove all references to these pack-files.\n \n repack::\n \tCreate a new pack-file containing objects in small pack-files\ndiff --git a/midx.c b/midx.c\nindex c27d0e5f15..bff5b99933 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1839,7 +1839,7 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla\n \t\tif (prepare_midx_pack(r, m, i))\n \t\t\tcontinue;\n \n-\t\tif (m->packs[i]->pack_keep)\n+\t\tif (m->packs[i]->pack_keep || m->packs[i]->is_cruft)\n \t\t\tcontinue;\n \n \t\tpack_name = xstrdup(m->packs[i]->pack_name);\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex afbe93f162..2d51b09680 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -847,6 +847,36 @@ test_expect_success 'expire respects .keep files' '\n \t)\n '\n \n+test_expect_success 'expiring unreferenced cruft pack retains pack' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit base &&\n+\t\ttest_commit --no-tag unreachable &&\n+\t\tunreachable=$(git rev-parse HEAD) &&\n+\n+\t\tgit reset --hard base &&\n+\t\tgit reflog expire --all --expire=all &&\n+\t\tgit repack --cruft -d &&\n+\t\tmtimes=\"$(ls $objdir/pack/pack-*.mtimes)\" &&\n+\n+\t\techo \"base..$unreachable\" >in &&\n+\t\tpack=\"$(git pack-objects --revs --delta-base-offset \\\n+\t\t\t$objdir/pack/pack \n---\n midx.c | 2 ++\n t/t5319-multi-pack-index.sh | 23 +++++++++++++++++++++++\n 2 files changed, 25 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex bff5b99933..05bcfc6f02 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1895,6 +1895,8 @@ static int fill_included_packs_all(struct repository *r,\n \t\t\tcontinue;\n \t\tif (!pack_kept_objects && m->packs[i]->pack_keep)\n \t\t\tcontinue;\n+\t\tif (m->packs[i]->is_cruft)\n+\t\t\tcontinue;\n \n \t\tinclude_pack[i] = 1;\n \t\tcount++;\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 2d51b09680..d967d92c20 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -784,6 +784,29 @@ test_expect_success 'repack creates a new pack' '\n \t)\n '\n \n+test_expect_success 'repack (all) ignores cruft pack' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit base &&\n+\t\ttest_commit --no-tag unreachable &&\n+\n+\t\tgit reset --hard base &&\n+\t\tgit reflog expire --all --expire=all &&\n+\t\tgit repack --cruft -d &&\n+\n+\t\tgit multi-pack-index write &&\n+\n+\t\tfind $objdir/pack | sort >before &&\n+\t\tgit multi-pack-index repack --batch-size=0 &&\n+\t\tfind $objdir/pack | sort >after &&\n+\n+\t\ttest_cmp before after\n+\t)\n+'\n+\n test_expect_success 'expire removes repacked packs' '\n \t(\n \t\tcd dup &&\n-- \n2.37.0.1.g1379af2e9d\n\n"},{"id":"463258","messageId":"cdc57439c7a827d63dbbc62c094323a19a745c3b.1663638929.git.me@ttaylorr.com","threadId":"58461","inReplyTo":"cover.1663638929.git.me@ttaylorr.com","subject":"[PATCH 5/7] midx.c: replace `xcalloc()` with `CALLOC_ARRAY()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-09-20T01:55:50Z","receivedAt":"2022-09-20T01:56:06Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Replace a direct invocation of Git's `xcalloc()` wrapper with the\n`CALLOC_ARRAY()` macro instead.\n\nThe latter is preferred since it is more conventional in Git's codebase,\nbut also because it automatically picks the correct value for the record\nsize.\n\nSigned-off-by: Taylor Blau \n---\n midx.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/midx.c b/midx.c\nindex 05bcfc6f02..d703fc5a16 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1912,9 +1912,11 @@ static int fill_included_packs_batch(struct repository *r,\n {\n \tuint32_t i, packs_to_repack;\n \tsize_t total_size;\n-\tstruct repack_info *pack_info = xcalloc(m->num_packs, sizeof(struct repack_info));\n+\tstruct repack_info *pack_info;\n \tint pack_kept_objects = 0;\n \n+\tCALLOC_ARRAY(pack_info, m->num_packs);\n+\n \trepo_config_get_bool(r, \"repack.packkeptobjects\", &pack_kept_objects);\n \n \tfor (i = 0; i < m->num_packs; i++) {\n-- \n2.37.0.1.g1379af2e9d\n\n"},{"id":"463259","messageId":"4c9c18de2d3a36c0939d21027b73762b20298477.1663638929.git.me@ttaylorr.com","threadId":"58461","inReplyTo":"cover.1663638929.git.me@ttaylorr.com","subject":"[PATCH 6/7] midx.c: remove unnecessary loop condition","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-09-20T01:55:53Z","receivedAt":"2022-09-20T01:56:14Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The fill_included_packs_batch() routine is responsible for aggregating\nobjects in packs with a non-zero value for the `--batch-size` option of\nthe `git multi-pack-index repack` sub-command.\n\nSince this routine is explicitly called only when `--batch-size` is\nnon-zero, there is no point in checking that this is the case in our\nloop condition.\n\nRemove the unnecessary part of this condition to avoid confusion.\n\nSigned-off-by: Taylor Blau \n---\n midx.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/midx.c b/midx.c\nindex d703fc5a16..273704caed 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1928,7 +1928,7 @@ static int fill_included_packs_batch(struct repository *r,\n \t\tpack_info[i].mtime = m->packs[i]->mtime;\n \t}\n \n-\tfor (i = 0; batch_size && i < m->num_objects; i++) {\n+\tfor (i = 0; i < m->num_objects; i++) {\n \t\tuint32_t pack_int_id = nth_midxed_pack_int_id(m, i);\n \t\tpack_info[pack_int_id].referenced_objects++;\n \t}\n-- \n2.37.0.1.g1379af2e9d\n\n"},{"id":"463260","messageId":"5f06a2e244a4efb166267cd3a7c975d5f04c07d5.1663638929.git.me@ttaylorr.com","threadId":"58461","inReplyTo":"cover.1663638929.git.me@ttaylorr.com","subject":"[PATCH 7/7] midx.c: avoid cruft packs with non-zero `repack --batch-size`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-09-20T01:55:56Z","receivedAt":"2022-09-20T01:56:49Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Apply similar treatment with respect to cruft packs as in a few commits\nago to `repack` with a non-zero `--batch-size`.\n\nSince the case of a non-zero `--batch-size` is handled separately (in\n`fill_included_packs_batch()` instead of `fill_included_packs_all()`), a\nseparate fix must be applied for this case.\n\nSigned-off-by: Taylor Blau \n---\n midx.c | 2 ++\n t/t5319-multi-pack-index.sh | 41 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 43 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex 273704caed..3a8dcfe98e 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1946,6 +1946,8 @@ static int fill_included_packs_batch(struct repository *r,\n \t\t\tcontinue;\n \t\tif (!pack_kept_objects && p->pack_keep)\n \t\t\tcontinue;\n+\t\tif (p->is_cruft)\n+\t\t\tcontinue;\n \t\tif (open_pack_index(p) || !p->num_objects)\n \t\t\tcontinue;\n \ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex d967d92c20..b5f9b10922 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -807,6 +807,47 @@ test_expect_success 'repack (all) ignores cruft pack' '\n \t)\n '\n \n+test_expect_success 'repack (--batch-size) ignores cruft pack' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit_bulk 5 &&\n+\t\ttest_commit --no-tag unreachable &&\n+\n+\t\tgit reset --hard HEAD^ &&\n+\t\tgit reflog expire --all --expire=all &&\n+\t\tgit repack --cruft -d &&\n+\n+\t\ttest_commit four &&\n+\n+\t\tfind $objdir/pack -type f -name \"*.pack\" | sort >before &&\n+\t\tgit repack -d &&\n+\t\tfind $objdir/pack -type f -name \"*.pack\" | sort >after &&\n+\n+\t\tpack=\"$(comm -13 before after)\" &&\n+\t\ttest_file_size \"$pack\" >sz &&\n+\t\t# Set --batch-size to twice the size of the pack created\n+\t\t# in the previous step, since this is enough to\n+\t\t# accommodate it and the cruft pack.\n+\t\t#\n+\t\t# This means that the MIDX machinery *could* combine the\n+\t\t# new and cruft packs together.\n+\t\t#\n+\t\t# We ensure that it does not below.\n+\t\tbatch=\"$((($(cat sz) * 2)))\" &&\n+\n+\t\tgit multi-pack-index write &&\n+\n+\t\tfind $objdir/pack | sort >before &&\n+\t\tgit multi-pack-index repack --batch-size=$batch &&\n+\t\tfind $objdir/pack | sort >after &&\n+\n+\t\ttest_cmp before after\n+\t)\n+'\n+\n test_expect_success 'expire removes repacked packs' '\n \t(\n \t\tcd dup &&\n-- \n2.37.0.1.g1379af2e9d\n"},{"id":"463306","messageId":"CAP8UFD3AE_qcQ2jLodKjoUE_wfEmXMLm5B9VbG52e10+XnhkbA@mail.gmail.com","threadId":"58461","inReplyTo":"cover.1663638929.git.me@ttaylorr.com","subject":"Re: [PATCH 0/7] midx: ignore cruft pack with `repack`, `expire`","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-09-20T14:39:56Z","receivedAt":"2022-09-20T14:42:46Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Sep 20, 2022 at 4:59 AM Taylor Blau wrote:\n\n> This series teaches both sub-commands to avoid any cruft pack(s),\n> preserving their metadata.\n\nMaybe:\n\ns/to avoid any cruft pack(s)/to avoid removing any cruft pack(s)/\n\nor:\n\ns/to avoid any cruft pack(s)/to avoid absorbing any cruft pack(s)/\n"},{"id":"463444","messageId":"37ffe1a6-bed9-70ce-2c66-b7a2d8c45ba9@github.com","threadId":"58461","inReplyTo":"3ae9903d2df491e291ab975c56ec78aa13d95655.1663638929.git.me@ttaylorr.com","subject":"Re: [PATCH 3/7] midx.c: prevent `expire` from removing the cruft pack","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-09-22T14:08:05Z","receivedAt":"2022-09-22T14:08:10Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 9/19/2022 9:55 PM, Taylor Blau wrote:\n> The `expire` sub-command unlinks any packs that are (a) contained in the\n> MIDX, but (b) have no objects referenced by the MIDX.\n\nIt is important to note that this can only happen if all objects in\nthe pack have duplicates in other pack-files.\n \n> This sub-command ignores `.keep` packs, which remain on-disk even if\n> they have no objects referenced by the MIDX. Cruft packs, however,\n> aren't given the same treatment: if none of the objects contained in the\n> cruft pack are selected from the cruft pack by the MIDX, then the cruft\n> pack is eligible to be expired.\n> \n> This is less than desireable, since the cruft pack has important\n\ns/desireable/desirable/\n(according to my spell-checker)\n\n> metadata about the individual object mtimes, which is useful to\n> determine how quickly an object should age out of the repository when\n> pruning.\n>\n> Ordinarily, we wouldn't expect the contents of a cruft pack to\n> duplicated across non-cruft packs (and we'd expect to see the MIDX\n> select all cruft objects from other sources even less often). But\n> nonetheless, it is still possible to trick the `expire` sub-command into\n> removing the `.mtimes` file in this circumstance.\n\nI was initially unconvinced that this scenario was super-critical\nto keeping the .mtimes file, but I was able to think of cases where\nobjects are duplicated out of the cruft pack due to de-thinning or\notherwise inefficiently packing unreachable objects into these\nother pack-files.\n\nThanks,\n-Stolee\n\n"},{"id":"463446","messageId":"63a1c3d4-eff3-af10-4263-058c88e74594@github.com","threadId":"58461","inReplyTo":"cover.1663638929.git.me@ttaylorr.com","subject":"Re: [PATCH 0/7] midx: ignore cruft pack with `repack`, `expire`","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-09-22T14:14:26Z","receivedAt":"2022-09-22T14:14:38Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 9/19/2022 9:55 PM, Taylor Blau wrote:\n> This series fixes a pair of bugs that were originally pointed out by\n> Gregory Szorc in [1].\n> \n> Namely, that both `git multi-pack-index repack` and `git\n> multi-pack-index expire` can cause us to \"absorb\" the cruft pack,\n> distributing its objects into a new pack, and removing its metadata.\n> \n> This is worth avoiding, since even though it doesn't result in object\n> corruption, this bug removes semi-important metadata contained in the\n> .mtimes file, which controls how fast objects leave the repository\n> during a pruning GC.\n> \n> This series teaches both sub-commands to avoid any cruft pack(s),\n> preserving their metadata.\n\nI gave these patches a careful review. They all look correct and\nare closing possible gaps in this mixed mode of repacking with\ndifferent strategies.\n\nI do think the 'expire' situation is very rare, but it's best to\nbe safe here.\n\nOverall, the end result is that users could set up their background\nmaintenance as follows:\n\n 1. maintenance.incremental-repack.schedule=daily to do an\n incremental repack every night.\n\n 2. maintenance.gc.schedule=weekly to do a GC once a week.\n\n 3. With gc.cruftPacks=true and a gc.pruneExpire value, the cruft\n packs will be written on that weekly job, only expiring old\n enough objects.\n\nBefore this series, that weekly GC creating cruft packs ran the\n(very likely) risk of those cruft packs being rewritten with the\nrest of the new object data, losing the .mtimes data.\n\n(While the proposed schedule above is now possible, I don't\nrecommend it as a default. Not only will it delete objects\nautomatically, but the GC task is very expensive for very large\nrepositories and the incremental-repack task was explicitly\ncreated to avoid that huge cost in those cases.)\n\nThanks for this update!\n-Stolee\n"}]}