Re: [PATCH 8/8] git-repack --max-pack-size: add option parsing to enable feature

From: Shawn O. Pearce <spearce@spearce.org>
Date: May 1, 2007, 05:45 UTC
Message-ID: <20070501054528.GC5942@spearce.org>
In-Reply-To: <46367ADC.2090704@gmail.com>

Dana How <danahow@gmail.com> wrote:

> Add --max-pack-size parsing and usage messages.
> Upgrade git-repack.sh to handle multiple packfile names.

...

> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c

...

Show 6 quoted lines

> +		if (!prefixcmp(arg, "--max-pack-size=")) {
> +			char *end;
> +			pack_size_limit = strtoul(arg+16, &end, 0) * 1024 * 1024;
> +			if (!arg[16] || *end)
> +				usage(pack_usage);
> +			no_reuse_delta = 1;

Wow, are you serious? This entire change is about making repack automatically split large projects into multiple packfiles. If that happens are you intending that the caller will mark all of those packfiles with .keep files immediately after repacking them? If you want users to create .keep files, can git-repack.sh do that for them when more than one packfile was output?

Because otherwise a "quick" git-gc will not be quick because the reuse delta feature (which is a massive performance improvement for repack/gc) will always be disabled. But odds are a future repack of the same project will generally keep things that are in the same packfile already in the same packfile again, so delta reuse is actually possible for most objects. I think you should find a way to make this change work without needing to force no_reuse_delta just because this limit was added.

> diff --git a/git-repack.sh b/git-repack.sh

...

Show 8 quoted lines

> +names=$(git-pack-objects --non-empty --all --reflog $args </dev/null "$PACKTMP") ||
>  	exit 1
> -if [ -z "$name" ]; then
> +if [ -z "$names" ]; then
>  	echo Nothing new to pack.
> -else
> +fi
> +for name in $names ; do

I think this particular change needs to either preceed the prior commit, or be part of it. If someone tries to bisect this patch series with a large enough project that multiple packfiles are being produced then you run into some ugly problems in this repack script.

-- 
Shawn.