[PATCH] pack-objects: warn on split packs disabling bitmaps

[Eric Wong <normalperson@yhbt.net>]

It can be tempting for a server admin to want a stable set of
long-lived packs for dumb clients; but also want to enable
bitmaps to serve smart clients more quickly.

Unfortunately, such a configuration is impossible;
so at least warn users of this incompatibility since
commit 21134714787a02a37da15424d72c0119b2b8ed71
("pack-objects: turn off bitmaps when we split packs").

Tested the warning by inspecting the output of:

	make -C t t5310-pack-bitmaps.sh GIT_TEST_OPTS=-v

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 Fwiw, I'm hoping to publish an ~800MB git-clone-able repo of
 our ML archives, soonish.  I can serve terabytes of dumb HTTP
 traffic all day long without breaking a sweat; but smart
 packing of big repos worries me; especially when feeding
 slow clients and having to leave processes running
 (or buffering pack output to disk).  So perhaps I'll teach
 my HTTP server play dumb whenever CPU/memory usage is high.

 Documentation/config.txt           | 11 +++++++----
 Documentation/git-pack-objects.txt |  3 ++-
 Documentation/git-repack.txt       |  9 ++++++---
 builtin/pack-objects.c             |  9 ++++++++-
 4 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 42d2b50..ec31170 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2156,8 +2156,10 @@ pack.packSizeLimit::
 	The maximum size of a pack.  This setting only affects
 	packing to a file when repacking, i.e. the git:// protocol
 	is unaffected.  It can be overridden by the `--max-pack-size`
-	option of linkgit:git-repack[1]. The minimum size allowed is
-	limited to 1 MiB. The default is unlimited.
+	option of linkgit:git-repack[1].  Reaching this limit prevents
+	pack bitmaps from being written when `repack.writeBitmaps`
+	is configured.  The minimum size allowed is limited to 1 MiB.
+	The default is unlimited.
 	Common unit suffixes of 'k', 'm', or 'g' are
 	supported.
 
@@ -2557,8 +2559,9 @@ repack.writeBitmaps::
 	objects to disk (e.g., when `git repack -a` is run).  This
 	index can speed up the "counting objects" phase of subsequent
 	packs created for clones and fetches, at the cost of some disk
-	space and extra time spent on the initial repack.  Defaults to
-	false.
+	space and extra time spent on the initial repack.  This has
+	no effect if `pack.packSizeLimit` is configured and reached.
+	Defaults to false.
 
 rerere.autoUpdate::
 	When set to true, `git-rerere` updates the index with the
diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt
index bbea529..19cdcd0 100644
--- a/Documentation/git-pack-objects.txt
+++ b/Documentation/git-pack-objects.txt
@@ -110,7 +110,8 @@ base-name::
 --max-pack-size=<n>::
 	Maximum size of each output pack file. The size can be suffixed with
 	"k", "m", or "g". The minimum size allowed is limited to 1 MiB.
-	If specified,  multiple packfiles may be created.
+	If specified, multiple packfiles may be created, which also
+	prevents the creation of a bitmap index.
 	The default is unlimited, unless the config variable
 	`pack.packSizeLimit` is set.
 
diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
index af230d0..2065812 100644
--- a/Documentation/git-repack.txt
+++ b/Documentation/git-repack.txt
@@ -106,7 +106,8 @@ other objects in that pack they already have locally.
 --max-pack-size=<n>::
 	Maximum size of each output pack file. The size can be suffixed with
 	"k", "m", or "g". The minimum size allowed is limited to 1 MiB.
-	If specified,  multiple packfiles may be created.
+	If specified, multiple packfiles may be created, causing
+	`--write-bitmap-index` and `repack.writeBitmaps` to be ignored.
 	The default is unlimited, unless the config variable
 	`pack.packSizeLimit` is set.
 
@@ -115,7 +116,9 @@ other objects in that pack they already have locally.
 	Write a reachability bitmap index as part of the repack. This
 	only makes sense when used with `-a` or `-A`, as the bitmaps
 	must be able to refer to all reachable objects. This option
-	overrides the setting of `pack.writeBitmaps`.
+	overrides the setting of `repack.writeBitmaps`.  This option
+	has no effect if a multiple packfiles are created due to
+	reaching `pack.packSizeLimit` or `--max-pack-size`.
 
 --pack-kept-objects::
 	Include objects in `.keep` files when repacking.  Note that we
@@ -123,7 +126,7 @@ other objects in that pack they already have locally.
 	This means that we may duplicate objects, but this makes the
 	option safe to use when there are concurrent pushes or fetches.
 	This option is generally only useful if you are writing bitmaps
-	with `-b` or `pack.writeBitmaps`, as it ensures that the
+	with `-b` or `repack.writeBitmaps`, as it ensures that the
 	bitmapped packfile has the necessary objects.
 
 Configuration
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index a27de5b..b6664ce 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -759,6 +759,10 @@ static off_t write_reused_pack(struct sha1file *f)
 	return reuse_packfile_offset - sizeof(struct pack_header);
 }
 
+static const char no_split_warning[] = N_(
+"disabling bitmap writing, packs are split due to pack.packSizeLimit"
+);
+
 static void write_pack_file(void)
 {
 	uint32_t i = 0, j;
@@ -813,7 +817,10 @@ static void write_pack_file(void)
 			fixup_pack_header_footer(fd, sha1, pack_tmp_name,
 						 nr_written, sha1, offset);
 			close(fd);
-			write_bitmap_index = 0;
+			if (write_bitmap_index) {
+				warning(_(no_split_warning));
+				write_bitmap_index = 0;
+			}
 		}
 
 		if (!pack_to_stdout) {
-- 
EW