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

[PATCH 1/6] repack: do not accidentally pack kept objects by default

From
Jeff King <peff@peff.net>
Date
Jun 10, 2014, 20:08 UTC
Message-ID
<20140610200837.GA14974@sigill.intra.peff.net>
In-Reply-To
<20140610200741.GA11248@sigill.intra.peff.net>

Commit ee34a2b (repack: add `repack.packKeptObjects` config var, 2014-03-03) added a flag which could duplicate kept objects, but did not mean to turn it on by default. Instead, the option is tied by default to the decision to write bitmaps, like:

  if (pack_kept_objects < 0)
	  pack_kept_objects = write_bitmap;
after which we expect pack_kept_objects to be a boolean 0 or
1.  However, that assignment neglects that write_bitmap is
_also_ a tri-state with "-1" as the default, and with
neither option given, we accidentally turn the option on.

This patch is the minimal fix to restore the desired behavior for the default state. Further patches will fix the more complicated cases.

Note the update to t7700. It failed to turn on bitmaps, meaning we were actually confirming the wrong behavior!

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/repack.c  | 2 +-
 t/t7700-repack.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index 6b0b62d..17bc8da 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -191,7 +191,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 				git_repack_usage, 0);
 
 	if (pack_kept_objects < 0)
-		pack_kept_objects = write_bitmap;
+		pack_kept_objects = write_bitmap > 0;
 
 	packdir = mkpathdup("%s/pack", get_object_directory());
 	packtmp = mkpathdup("%s/.tmp-%d-pack", packdir, (int)getpid());
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 284018e..f054434 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -37,7 +37,7 @@ test_expect_success 'objects in packs marked .keep are not repacked' '
 
 test_expect_success 'writing bitmaps can duplicate .keep objects' '
 	# build on $objsha1, $packsha1, and .keep state from previous
-	git repack -Adl &&
+	git repack -Adbl &&
 	test_when_finished "found_duplicate_object=" &&
 	for p in .git/objects/pack/*.idx; do
 		idx=$(basename $p)
-- 
2.0.0.729.g520999f
Previous: Jeff KingNext: Jeff King
Message 5 of 13 in “Disk waste with packs and .keep files”
  1. Matthieu MoyJun 10, 2014
  2. Jeff KingJun 10, 2014
  3. Jeff KingJun 10, 2014
  4. 0/6 fix repack.packKeptObjects regression in v2.0Jeff King, Jun 10, 2014
  5. 1/6 repack: do not accidentally pack kept objects by defaultJeff King, Jun 10, 2014
  6. Jeff KingJun 11, 2014
  7. 2/6 repack: respect pack.writebitmapsJeff King, Jun 10, 2014
  8. 3/6 repack: s/write_bitmap/&s/ in codeJeff King, Jun 10, 2014
  9. 4/6 pack-objects: stop respecting pack.writebitmapsJeff King, Jun 10, 2014
  10. Junio C HamanoJun 10, 2014
  11. Jeff KingJun 10, 2014
  12. 5/6 repack: simplify handling of --write-bitmap-indexJeff King, Jun 10, 2014
  13. 6/6 repack: introduce repack.writeBitmaps config optionJeff King, Jun 10, 2014

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.