[PATCH] fix pack.packSizeLimit and --max-pack-size handling
First, pack.packSizeLimit and --max-pack-size didn't use the same base unit which was confusing. They both use suffixable value arguments 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>
---
On Wed, 12 Nov 2008, Junio C Hamano wrote:
Show 26 quoted lines
> 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.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=")) {
