[PATCH] fix pack.packSizeLimit and --max-pack-size handling

From

Nicolas Pitre <nico@cam.org>

Date

Nov 12, 2008, 18:23 UTC

Message-ID

<alpine.LFD.2.00.0811121255330.27509@xanadu.home>

In-Reply-To

<7vk5b8q1l4.fsf@gitster.siamese.dyndns.org>

Signed-off-by: Nicolas Pitre <nico@cam.org>
---

On Wed, 12 Nov 2008, Junio C Hamano wrote:

> Nicolas Pitre <nico@cam.org> writes:
> 
> > First, pack.packSizeLimit and --max-pack-size didn't use the same base
> > unit which was confusing.  They both use MiB now.
> >
> > Also, if the limit was sufficiently low, having a single object written
> > could bust the limit (by design), but caused the remaining allowed size 
> > to go negative for subsequent objects, which for an unsigned variable is 
> > a rather huge limit.
> >
> > Signed-off-by: Nicolas Pitre <nico@cam.org>
> > ---
> 
> > @@ -1844,7 +1848,7 @@ static int git_pack_config(const char *k, const char *v, void *cb)
> >  		return 0;
> >  	}
> >  	if (!strcmp(k, "pack.packsizelimit")) {
> > -		pack_size_limit_cfg = git_config_ulong(k, v);
> > +		pack_size_limit_cfg = git_config_ulong(k, v) * 1024 * 1024;
> 
> The fix to tweak the limit for subsequent split pack is a good thing to
> have, but this change would break existing repositories where people
> specified 20971520 (or worse yet "20m") to limit the size to 20MB.
> 
> I think --max-pack-size is what should be fixed to use git_parse_ulong()
> to match the configuration, if you find the discrepancy disturbing.

Fair enough.

diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt
index 1c4fb43..d60409a 100644
--- a/Documentation/git-pack-objects.txt
+++ b/Documentation/git-pack-objects.txt
@@ -104,10 +104,11 @@ base-name::
 	default.
 
 --max-pack-size=<n>::
-	Maximum size of each output packfile, expressed in MiB.
+	Maximum size of each output packfile.
 	If specified,  multiple packfiles may be created.
 	The default is unlimited, unless the config variable
-	`pack.packSizeLimit` is set.
+	`pack.packSizeLimit` is set. The size can be suffixed with
+	"k", "m", or "g".
 
 --incremental::
 	This flag causes an object already in a pack ignored
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 0c4649c..9c4333b 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -75,7 +75,7 @@ static int allow_ofs_delta;
 static const char *base_name;
 static int progress = 1;
 static int window = 10;
-static uint32_t pack_size_limit, pack_size_limit_cfg;
+static unsigned long pack_size_limit, pack_size_limit_cfg;
 static int depth = 50;
 static int delta_search_threads = 1;
 static int pack_to_stdout;
@@ -245,8 +245,12 @@ static unsigned long write_object(struct sha1file *f,
 	type = entry->type;
 
 	/* write limit if limited packsize and not first object */
-	limit = pack_size_limit && nr_written ?
-			pack_size_limit - write_offset : 0;
+	if (!pack_size_limit || !nr_written)
+		limit = 0;
+	else if (pack_size_limit <= write_offset)
+		limit = 1;
+	else
+		limit = pack_size_limit - write_offset;
 
 	if (!entry->delta)
 		usable_delta = 0;	/* no delta */
@@ -2103,11 +2107,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 		if (!prefixcmp(arg, "--max-pack-size=")) {
-			char *end;
-			pack_size_limit_cfg = 0;
-			pack_size_limit = strtoul(arg+16, &end, 0) * 1024 * 1024;
-			if (!arg[16] || *end)
+			if (!git_parse_ulong(arg+16, &pack_size_limit))
 				usage(pack_usage);
+			else
+				pack_size_limit_cfg = 0;
 			continue;
 		}
 		if (!prefixcmp(arg, "--window=")) {