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

[PATCH v3 05/10] pack-objects: fix error when packing same pack twice

From
Patrick Steinhardt <ps@pks.im>
Date
Apr 13, 2023, 11:16 UTC
Message-ID
<ac528514e7e609adc00f417b30593722ef73105a.1681384405.git.ps@pks.im>
In-Reply-To
<cover.1681384405.git.ps@pks.im>

When passed the same packfile twice via `--stdin-packs` we return an error that the packfile supposedly was not found. This is because when reading packs into the list of included or excluded packfiles, we will happily re-add packfiles even if they are part of the lists already. And while the list can now contain duplicates, we will only set the `util` pointer of the first list entry to the `packed_git` structure. We notice that at a later point when checking that all list entries have their `util` pointer set and die with an error.

While this is kind of a nonsensical request, this scenario can be hit when doing geometric repacks. When a repository is connected to an alternate object directory and both have the exact same packfile then both would get added to the geometric sequence. And when we then decide to perform the repack, we will invoke git-pack-objects(1) with the same packfile twice.

Fix this bug by removing any duplicates from both the included and excluded packs.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/pack-objects.c        |  2 ++
 t/t5331-pack-objects-stdin.sh | 27 +++++++++++++++++++++++++++
 t/t7703-repack-geometric.sh   | 25 +++++++++++++++++++++++++
 3 files changed, 54 insertions(+)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 7e9e20172a..7d0e864c35 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3348,7 +3348,9 @@ static void read_packs_list_from_stdin(void)
 	}
 
 	string_list_sort(&include_packs);
+	string_list_remove_duplicates(&include_packs, 0);
 	string_list_sort(&exclude_packs);
+	string_list_remove_duplicates(&exclude_packs, 0);
 
 	for (p = get_all_packs(the_repository); p; p = p->next) {
 		const char *pack_name = pack_basename(p);
diff --git a/t/t5331-pack-objects-stdin.sh b/t/t5331-pack-objects-stdin.sh
index d5eece5899..def7640c91 100755
--- a/t/t5331-pack-objects-stdin.sh
+++ b/t/t5331-pack-objects-stdin.sh
@@ -7,6 +7,12 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
+packed_objects() {
+	git show-index <"$1" >tmp-object-list &&
+	cut -d' ' -f2 tmp-object-list | sort &&
+	rm tmp-object-list
+ }
+
 test_expect_success 'setup for --stdin-packs tests' '
 	git init stdin-packs &&
 	(
@@ -142,4 +148,25 @@ test_expect_success '--stdin-packs with broken links' '
 	)
 '
 
+test_expect_success 'pack-objects --stdin with duplicate packfile' '
+	test_when_finished "rm -fr repo" &&
+
+	git init repo &&
+	(
+		cd repo &&
+		test_commit "commit" &&
+		git repack -ad &&
+
+		{
+			basename .git/objects/pack/pack-*.pack &&
+			basename .git/objects/pack/pack-*.pack
+		} >packfiles &&
+
+		git pack-objects --stdin-packs generated-pack <packfiles &&
+		packed_objects .git/objects/pack/pack-*.idx >expect &&
+		packed_objects generated-pack-*.idx >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_done
diff --git a/t/t7703-repack-geometric.sh b/t/t7703-repack-geometric.sh
index 7027676977..d0823f2eb2 100755
--- a/t/t7703-repack-geometric.sh
+++ b/t/t7703-repack-geometric.sh
@@ -336,4 +336,29 @@ test_expect_success '--geometric --with-midx with no local objects' '
 	test_dir_is_empty member/$packdir
 '
 
+test_expect_success '--geometric with same pack in main and alternate ODB' '
+	test_when_finished "rm -fr shared member" &&
+
+	# Create a repository with a single packfile that acts as alternate
+	# object database.
+	git init shared &&
+	test_commit -C shared "shared-objects" &&
+	git -C shared repack -ad &&
+
+	# We create the member repository as an exact copy so that it has the
+	# same packfile.
+	cp -r shared member &&
+	test-tool path-utils real_path shared/.git/objects >member/.git/objects/info/alternates &&
+	find shared/.git/objects -type f >expected-files &&
+
+	# Verify that we can repack objects as expected without observing any
+	# error. Having the same packfile in both ODBs used to cause an error
+	# in git-pack-objects(1).
+	git -C member repack --geometric 2 2>err &&
+	test_must_be_empty err &&
+	# Nothing should have changed.
+	find shared/.git/objects -type f >actual-files &&
+	test_cmp expected-files actual-files
+'
+
 test_done
-- 
2.40.0
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 43 of 67 in “repack: fix geometric repacking with gitalternates”
  1. repack: fix geometric repacking with gitalternatesPatrick Steinhardt, Apr 4, 2023
  2. Taylor BlauApr 4, 2023
  3. Taylor BlauApr 4, 2023
  4. Patrick SteinhardtApr 5, 2023
  5. Derrick StoleeApr 10, 2023
  6. Taylor BlauApr 10, 2023
  7. Patrick SteinhardtApr 11, 2023
  8. Taylor BlauApr 11, 2023
  9. Patrick SteinhardtApr 12, 2023
  10. Patrick SteinhardtApr 11, 2023
  11. Patrick SteinhardtApr 11, 2023
  12. Taylor BlauApr 11, 2023
  13. Taylor BlauApr 10, 2023
  14. 0/8 repack: fix geometric repacking with gitalternatesPatrick Steinhardt, Apr 12, 2023
  15. 1/8 midx: fix segfault with no packs and invalid preferred packPatrick Steinhardt, Apr 12, 2023
  16. Taylor BlauApr 12, 2023
  17. Patrick SteinhardtApr 13, 2023
  18. 2/8 repack: fix trying to use preferred pack in alternatesPatrick Steinhardt, Apr 12, 2023
  19. Taylor BlauApr 12, 2023
  20. Patrick SteinhardtApr 13, 2023
  21. 3/8 repack: fix generating multi-pack-index with only non-local packsPatrick Steinhardt, Apr 12, 2023
  22. Taylor BlauApr 12, 2023
  23. 4/8 pack-objects: fix error when packing same pack twicePatrick Steinhardt, Apr 12, 2023
  24. Taylor BlauApr 12, 2023
  25. 5/8 pack-objects: fix error when same packfile is included and excludedPatrick Steinhardt, Apr 12, 2023
  26. Taylor BlauApr 12, 2023
  27. 6/8 pack-objects: extend test coverage of `--stdin-packs` with alternatesPatrick Steinhardt, Apr 12, 2023
  28. 7/8 repack: honor `-l` when calculating pack geometryPatrick Steinhardt, Apr 12, 2023
  29. Junio C HamanoApr 12, 2023
  30. Junio C HamanoApr 13, 2023
  31. Patrick SteinhardtApr 13, 2023
  32. 8/8 repack: disable writing bitmaps when doing a local geometric repackPatrick Steinhardt, Apr 12, 2023
  33. Taylor BlauApr 12, 2023
  34. Patrick SteinhardtApr 13, 2023
  35. Patrick SteinhardtApr 13, 2023
  36. Taylor BlauApr 12, 2023
  37. 00/10 repack: fix geometric repacking with gitalternatesPatrick Steinhardt, Apr 13, 2023
  38. 01/10 midx: fix segfault with no packs and invalid preferred packPatrick Steinhardt, Apr 13, 2023
  39. Derrick StoleeApr 13, 2023
  40. 02/10 repack: fix trying to use preferred pack in alternatesPatrick Steinhardt, Apr 13, 2023
  41. 03/10 repack: fix generating multi-pack-index with only non-local packsPatrick Steinhardt, Apr 13, 2023
  42. 04/10 pack-objects: split out `--stdin-packs` tests into separate filePatrick Steinhardt, Apr 13, 2023
  43. 05/10 pack-objects: fix error when packing same pack twicePatrick Steinhardt, Apr 13, 2023
  44. 06/10 pack-objects: fix error when same packfile is included and excludedPatrick Steinhardt, Apr 13, 2023
  45. 07/10 pack-objects: extend test coverage of `--stdin-packs` with alternatesPatrick Steinhardt, Apr 13, 2023
  46. 08/10 t/helper: allow chmtime to print verbosely without modifying mtimePatrick Steinhardt, Apr 13, 2023
  47. 09/10 repack: honor `-l` when calculating pack geometryPatrick Steinhardt, Apr 13, 2023
  48. Derrick StoleeApr 13, 2023
  49. Patrick SteinhardtApr 13, 2023
  50. Junio C HamanoApr 13, 2023
  51. 10/10 repack: disable writing bitmaps when doing a local repackPatrick Steinhardt, Apr 13, 2023
  52. Derrick StoleeApr 13, 2023
  53. Junio C HamanoApr 14, 2023
  54. Patrick SteinhardtApr 14, 2023
  55. 00/10 repack: fix geometric repacking with gitalternatesPatrick Steinhardt, Apr 14, 2023
  56. 01/10 midx: fix segfault with no packs and invalid preferred packPatrick Steinhardt, Apr 14, 2023
  57. 03/10 repack: fix generating multi-pack-index with only non-local packsPatrick Steinhardt, Apr 14, 2023
  58. 02/10 repack: fix trying to use preferred pack in alternatesPatrick Steinhardt, Apr 14, 2023
  59. 04/10 pack-objects: split out `--stdin-packs` tests into separate filePatrick Steinhardt, Apr 14, 2023
  60. 05/10 pack-objects: fix error when packing same pack twicePatrick Steinhardt, Apr 14, 2023
  61. 06/10 pack-objects: fix error when same packfile is included and excludedPatrick Steinhardt, Apr 14, 2023
  62. 07/10 pack-objects: extend test coverage of `--stdin-packs` with alternatesPatrick Steinhardt, Apr 14, 2023
  63. 08/10 t/helper: allow chmtime to print verbosely without modifying mtimePatrick Steinhardt, Apr 14, 2023
  64. 09/10 repack: honor `-l` when calculating pack geometryPatrick Steinhardt, Apr 14, 2023
  65. 10/10 repack: disable writing bitmaps when doing a local repackPatrick Steinhardt, Apr 14, 2023
  66. Derrick StoleeApr 14, 2023
  67. Junio C HamanoApr 14, 2023

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.