git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] builtin/repack.c: invalidate MIDX only when necessary

From
Taylor Blau <me@ttaylorr.com>
Date
Aug 25, 2020, 02:01 UTC
Message-ID
<ef9186a8df0d712c2ecccbe62cb43a7abadb9c96.1598320716.git.me@ttaylorr.com>

In 525e18c04b (midx: clear midx on repack, 2018-07-12), 'git repack' learned to remove a multi-pack-index file if it added or removed a pack from the object store.

This mechanism is a little over-eager, since it is only necessary to drop a MIDX if 'git repack' removes a pack that the MIDX references. Adding a pack outside of the MIDX does not require invalidating the MIDX, and likewise for removing a pack the MIDX does not know about.

Teach 'git repack' to check for this by loading the MIDX, and checking whether the to-be-removed pack is known to the MIDX. This requires a slightly odd alternation to a test in t5319, which is explained with a comment.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
Something I noticed while working on a MIDX-related topic. Not critical
that we take this now since it's only dropping the MIDX too
aggressively, but not otherwise doing anything destructive. But, this
will make my somewhat large other series a little smaller.
 builtin/repack.c            | 12 +++++-------
 t/t5319-multi-pack-index.sh | 21 +++++++++++++++++++--
 2 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index 04c5ceaf7e..98fac03946 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -133,7 +133,11 @@ static void get_non_kept_pack_filenames(struct string_list *fname_list,
 static void remove_redundant_pack(const char *dir_name, const char *base_name)
 {
 	struct strbuf buf = STRBUF_INIT;
-	strbuf_addf(&buf, "%s/%s.pack", dir_name, base_name);
+	struct multi_pack_index *m = get_multi_pack_index(the_repository);
+	strbuf_addf(&buf, "%s.pack", base_name);
+	if (m && midx_contains_pack(m, buf.buf))
+		clear_midx_file(the_repository);
+	strbuf_insertf(&buf, 0, "%s/", dir_name);
 	unlink_pack_path(buf.buf, 1);
 	strbuf_release(&buf);
 }
@@ -286,7 +290,6 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 	int keep_unreachable = 0;
 	struct string_list keep_pack_list = STRING_LIST_INIT_NODUP;
 	int no_update_server_info = 0;
-	int midx_cleared = 0;
 	struct pack_objects_args po_args = {NULL};

 	struct option builtin_repack_options[] = {
@@ -439,11 +442,6 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 		for (ext = 0; ext < ARRAY_SIZE(exts); ext++) {
 			char *fname, *fname_old;

-			if (!midx_cleared) {
-				clear_midx_file(the_repository);
-				midx_cleared = 1;
-			}
-
 			fname = mkpathdup("%s/pack-%s%s", packdir,
 						item->string, exts[ext].name);
 			if (!file_exists(fname)) {
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 43b1b5b2af..16a1ad040e 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -382,12 +382,29 @@ test_expect_success 'repack with the --no-progress option' '
 	test_line_count = 0 err
 '

-test_expect_success 'repack removes multi-pack-index' '
+test_expect_success 'repack removes multi-pack-index when deleting packs' '
 	test_path_is_file $objdir/pack/multi-pack-index &&
-	GIT_TEST_MULTI_PACK_INDEX=0 git repack -adf &&
+	# Set GIT_TEST_MULTI_PACK_INDEX to 0 to avoid writing a new
+	# multi-pack-index after repacking, but set "core.multiPackIndex" to
+	# true so that "git repack" can read the existing MIDX.
+	GIT_TEST_MULTI_PACK_INDEX=0 git -c core.multiPackIndex repack -adf &&
 	test_path_is_missing $objdir/pack/multi-pack-index
 '

+test_expect_success 'repack preserves multi-pack-index when deleting unknown packs' '
+	git multi-pack-index write &&
+	cp $objdir/pack/multi-pack-index $objdir/pack/multi-pack-index.bak &&
+	test_when_finished "rm -f $objdir/pack/multi-pack-index.bak" &&
+
+	# Write a new pack that is unknown to the multi-pack-index.
+	git hash-object -w </dev/null >blob &&
+	git pack-objects $objdir/pack/pack <blob &&
+
+	GIT_TEST_MULTI_PACK_INDEX=0 git -c core.multiPackIndex repack -d &&
+	test_cmp_bin $objdir/pack/multi-pack-index \
+		$objdir/pack/multi-pack-index.bak
+'
+
 compare_results_with_midx "after repack"

 test_expect_success 'multi-pack-index and pack-bitmap' '
--
2.28.0.rc1.13.ge78abce653
Next: Jeff King
Message 1 of 78 in “builtin/repack.c: invalidate MIDX only when necessary”
  1. builtin/repack.c: invalidate MIDX only when necessaryTaylor Blau, Aug 25, 2020
  2. Jeff KingAug 25, 2020
  3. Taylor BlauAug 25, 2020
  4. Derrick StoleeAug 25, 2020
  5. Taylor BlauAug 25, 2020
  6. Derrick StoleeAug 25, 2020
  7. Taylor BlauAug 25, 2020
  8. Jeff KingAug 25, 2020
  9. Junio C HamanoAug 25, 2020
  10. Taylor BlauAug 25, 2020
  11. Derrick StoleeAug 25, 2020
  12. Jeff KingAug 25, 2020
  13. Jeff KingAug 25, 2020
  14. Junio C HamanoAug 25, 2020
  15. Jeff KingAug 25, 2020
  16. pack-redundant: gauge the usage before proposing its removalJunio C Hamano, Aug 25, 2020
  17. Taylor BlauAug 25, 2020
  18. Junio C HamanoAug 25, 2020
  19. 0/3 War on dashed-gitJunio C Hamano, Aug 26, 2020
  20. 1/3 transport-helper: do not run git-remote-ext etc. in dashed formJunio C Hamano, Aug 26, 2020
  21. Eric SunshineAug 26, 2020
  22. Johannes SchindelinAug 26, 2020
  23. Junio C HamanoAug 26, 2020
  24. 2/3 cvsexportcommit: do not run git programs in dashed formJunio C Hamano, Aug 26, 2020
  25. Eric SunshineAug 26, 2020
  26. Junio C HamanoAug 26, 2020
  27. Junio C HamanoAug 26, 2020
  28. Junio C HamanoAug 26, 2020
  29. Johannes SchindelinAug 26, 2020
  30. 3/3 git: catch an attempt to run "git-foo"Junio C Hamano, Aug 26, 2020
  31. Junio C HamanoAug 26, 2020
  32. Johannes SchindelinAug 26, 2020
  33. Junio C HamanoAug 26, 2020
  34. Johannes SchindelinAug 28, 2020
  35. Junio C HamanoAug 28, 2020
  36. Johannes SchindelinAug 31, 2020
  37. Junio C HamanoAug 31, 2020
  38. Johannes SchindelinDec 20, 2020
  39. Junio C HamanoDec 21, 2020
  40. Johannes SchindelinDec 30, 2020
  41. Johannes SchindelinAug 26, 2020
  42. Junio C HamanoAug 26, 2020
  43. 0/2 avoid running "git-subcmd" in the dashed formJunio C Hamano, Aug 26, 2020
  44. 1/2 transport-helper: do not run git-remote-ext etc. in dashed formJunio C Hamano, Aug 26, 2020
  45. 2/2 cvsexportcommit: do not run git programs in dashed formJunio C Hamano, Aug 26, 2020
  46. 3/2 credential-cache: use child_process.argsJunio C Hamano, Aug 26, 2020
  47. run_command: teach API users to use embedded 'args' moreJunio C Hamano, Aug 26, 2020
  48. Jeff KingAug 27, 2020
  49. Junio C HamanoAug 27, 2020
  50. Eric SunshineAug 27, 2020
  51. Jeff KingAug 27, 2020
  52. Eric SunshineAug 27, 2020
  53. worktree: fix leak in check_clean_worktree()Jeff King, Aug 27, 2020
  54. Eric SunshineAug 27, 2020
  55. Junio C HamanoAug 27, 2020
  56. Jeff KingAug 27, 2020
  57. Jeff KingAug 27, 2020
  58. Junio C HamanoAug 27, 2020
  59. Jeff KingAug 27, 2020
  60. Junio C HamanoAug 27, 2020
  61. Junio C HamanoAug 31, 2020
  62. Jeff KingSep 1, 2020
  63. Junio C HamanoSep 1, 2020
  64. Derrick StoleeAug 27, 2020
  65. Junio C HamanoAug 27, 2020
  66. Jeff KingAug 28, 2020
  67. Junio C HamanoAug 28, 2020
  68. Son Luong NgocAug 25, 2020
  69. Derrick StoleeAug 25, 2020
  70. Taylor BlauAug 25, 2020
  71. builtin/repack.c: invalidate MIDX only when necessaryTaylor Blau, Aug 25, 2020
  72. Derrick StoleeAug 26, 2020
  73. Junio C HamanoAug 26, 2020
  74. Jeff KingAug 25, 2020
  75. Derrick StoleeAug 25, 2020
  76. Jeff KingAug 25, 2020
  77. Taylor BlauAug 25, 2020
  78. Jeff KingAug 25, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.