Re: [PATCH 6/7] pack-objects: disable --full-name-hash when shallow

[Taylor Blau <me@ttaylorr.com>]

On Tue, Nov 05, 2024 at 03:05:06AM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
>
> As demonstrated in the previous change, the --full-name-hash option of
> 'git pack-objects' is less effective in a trunctated history. Thus, even
> when the option is selected via a command-line option or config, disable
> this option when the '--shallow' option is specified. This will help
> performance in servers that choose to enable the --full-name-hash option
> by default for a repository while not regressing their ability to serve
> shallow clones.
>
> This will not present a compatibility issue in the future when the full
> name hash values are stored in the reachability bitmaps, since shallow
> clones disable bitmaps.
>
> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
>  builtin/pack-objects.c       | 6 ++++++
>  t/perf/p5313-pack-objects.sh | 1 +
>  2 files changed, 7 insertions(+)

I appreciate demonstrating the value of declaring --shallow and
--full-name-hash incompatible by showing the performance numbers in the
previous patch.

But TBH I think that it would be equally fine or slightly better to say
up front "when combined with --shallow, this option produces larger
packs during testing, so the two are incompatible for now". You could
include some performance numbers there to illustrate that difference in
the commit log too if you wanted.

But I don't think it's worth introducing the pair as compatible only to
mark them incompatible later on in the same series.

Thanks,
Taylor