[PATCH] Introduce the config variable pack.packSizeLimit

[PATCH] Introduce the config variable pack.packSizeLimit

[Johannes Schindelin]

"git pack-objects" has the option --max-pack-size to limit the file
size of the packs to a certain amount of bytes.  On platforms where
the pack file size is limited by filesystem constraints, it is easy
to forget this option, and this option does not exist for "git gc"
to begin with.

So introduce a config variable to set the default maximum, but make
this overrideable by the command line.

Suggested by Tor Arvid Lund.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/config.txt           |    6 ++++++
 Documentation/git-pack-objects.txt |    3 ++-
 builtin-pack-objects.c             |   10 +++++++++-
 t/t5300-pack-object.sh             |    6 ++++++
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 4e222f1..3e10feb 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -766,6 +766,12 @@ pack.indexVersion::
 	whenever the corresponding pack is larger than 2 GB.  Otherwise
 	the default is 1.
 
+pack.packSizeLimit:
+	The default maximum size of a pack.  This setting only affects
+	packing to a file, i.e. the git:// protocol is unaffected.  It
+	can be overridden by the `\--max-pack-size` option of
+	linkgit:git-repack[1].
+
 pull.octopus::
 	The default merge strategy to use when pulling multiple branches
 	at once.
diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt
index 74cc7c1..8353be1 100644
--- a/Documentation/git-pack-objects.txt
+++ b/Documentation/git-pack-objects.txt
@@ -99,7 +99,8 @@ base-name::
 --max-pack-size=<n>::
 	Maximum size of each output packfile, expressed in MiB.
 	If specified,  multiple packfiles may be created.
-	The default is unlimited.
+	The default is unlimited, unless the config variable
+	`pack.packSizeLimit` is set.
 
 --incremental::
 	This flag causes an object already in a pack ignored
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index d3efeff..692a761 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -68,7 +68,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;
+static uint32_t pack_size_limit, pack_size_limit_cfg;
 static int depth = 50;
 static int delta_search_threads = 1;
 static int pack_to_stdout;
@@ -1867,6 +1867,10 @@ static int git_pack_config(const char *k, const char *v)
 			die("bad pack.indexversion=%d", pack_idx_default_version);
 		return 0;
 	}
+	if (!strcmp(k, "pack.packsizelimit")) {
+		pack_size_limit_cfg = git_config_ulong(k, v);
+		return 0;
+	}
 	return git_default_config(k, v);
 }
 
@@ -2096,6 +2100,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 		}
 		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)
 				usage(pack_usage);
@@ -2220,6 +2225,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	if (pack_to_stdout != !base_name)
 		usage(pack_usage);
 
+	if (!pack_to_stdout && !pack_size_limit)
+		pack_size_limit = pack_size_limit_cfg;
+
 	if (pack_to_stdout && pack_size_limit)
 		die("--max-pack-size cannot be used to build a pack for transfer.");
 
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 4f350dd..cd3c149 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -268,4 +268,10 @@ test_expect_success \
     'make sure index-pack detects the SHA1 collision' \
     '! git-index-pack -o bad.idx test-3.pack'
 
+test_expect_success \
+    'honor pack.packSizeLimit' \
+    'git config pack.packSizeLimit 200 &&
+     packname_4=$(git pack-objects test-4 <obj-list) &&
+     test 3 = $(ls test-4-*.pack | wc -l)'
+
 test_done
-- 
1.5.4.1178.gd491a5

Re: [PATCH] Introduce the config variable pack.packSizeLimit

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> "git pack-objects" has the option --max-pack-size to limit the file
> size of the packs to a certain amount of bytes.  On platforms where
> the pack file size is limited by filesystem constraints, it is easy
> to forget this option, and this option does not exist for "git gc"
> to begin with.
>
> So introduce a config variable to set the default maximum, but make
> this overrideable by the command line.

The intention is good and all makes sense.

> +test_expect_success \
> +    'honor pack.packSizeLimit' \
> +    'git config pack.packSizeLimit 200 &&
> +     packname_4=$(git pack-objects test-4 <obj-list) &&
> +     test 3 = $(ls test-4-*.pack | wc -l)'
> +

Where does "3" come from, and what could break it?

Re: [PATCH] Introduce the config variable pack.packSizeLimit

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> +test_expect_success \
>> +    'honor pack.packSizeLimit' \
>> +    'git config pack.packSizeLimit 200 &&
>> +     packname_4=$(git pack-objects test-4 <obj-list) &&
>> +     test 3 = $(ls test-4-*.pack | wc -l)'
>> +
>
> Where does "3" come from, and what could break it?

How about doing it like this, instead?

diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 4f350dd..3eea478 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -268,4 +268,12 @@ test_expect_success \
     'make sure index-pack detects the SHA1 collision' \
     '! git-index-pack -o bad.idx test-3.pack'
 
+test_expect_success 'honor pack.packSizeLimit' '
+     limit=200 &&
+     git config pack.packSizeLimit $limit &&
+     packname_4=$(git pack-objects test-4 <obj-list) &&
+     bigger=$(find test-4-*.pack -size +${limit}c) &&
+     test -z "$bigger"
+'
+
 test_done

Re: [PATCH] Introduce the config variable pack.packSizeLimit

[Johannes Schindelin]

Hi,

On Fri, 8 Feb 2008, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> >> +test_expect_success \
> >> +    'honor pack.packSizeLimit' \
> >> +    'git config pack.packSizeLimit 200 &&
> >> +     packname_4=$(git pack-objects test-4 <obj-list) &&
> >> +     test 3 = $(ls test-4-*.pack | wc -l)'
> >> +
> >
> > Where does "3" come from, and what could break it?

It was in my test, and as I do not expect the obj-list (as well as the 
contents it references to change), I just fixed it to 3.

> How about doing it like this, instead?
> 
> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
> index 4f350dd..3eea478 100755
> --- a/t/t5300-pack-object.sh
> +++ b/t/t5300-pack-object.sh
> @@ -268,4 +268,12 @@ test_expect_success \
>      'make sure index-pack detects the SHA1 collision' \
>      '! git-index-pack -o bad.idx test-3.pack'
>  
> +test_expect_success 'honor pack.packSizeLimit' '
> +     limit=200 &&
> +     git config pack.packSizeLimit $limit &&
> +     packname_4=$(git pack-objects test-4 <obj-list) &&
> +     bigger=$(find test-4-*.pack -size +${limit}c) &&
> +     test -z "$bigger"
> +'

Sure (with minor modifications).  I was not certain if I could rely on 
find being available everywhere, and understanding -size.

Minor modification that is necessary: MacOSX' find does not accept a find 
call like that.  It must be

	bigger=$(find . -name test-4-*.pack -size +${limit}c) &&

Ciao,
Dscho

Re: [PATCH] Introduce the config variable pack.packSizeLimit

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Minor modification that is necessary: MacOSX' find does not accept a find 
> call like that.  It must be
>
> 	bigger=$(find . -name test-4-*.pack -size +${limit}c) &&

That looks more traditional, but now you mention it, I agree
that it might not be so portable.  I've used "-size +$size" in
Ancient Unices, but 'c' suffix is a more recent invention (V7
did not have it, neither did 4BSD, IIRC).