Re: [PATCH v2] index-pack: clarify the breached limit

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Matt Cooper via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, gitster@pobox.com, me@ttaylorr.com,
	derrickstolee@github.com, Matt Cooper <vtbassmatt@gmail.com>
Subject: Re: [PATCH v2] index-pack: clarify the breached limit
Date: Thu, 24 Feb 2022 01:14:25 +0100	[thread overview]
Message-ID: <220224.86wnhl9ky8.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <pull.1158.v2.git.1645661240356.gitgitgadget@gmail.com>

On Thu, Feb 24 2022, Matt Cooper via GitGitGadget wrote:

> From: Matt Cooper <vtbassmatt@gmail.com>
> [...]
> +test_expect_success 'too-large packs report the breach' '
> +	pack=$(git pack-objects --all pack </dev/null) &&
> +	sz="$(test_file_size pack-$pack.pack)" &&

I don't think this needs a re-roll, but FWIW I have an old-ish local
patch I haven't submitted yet to s/test_file_size/test-tool path-utils
file-size/g in the test suite.

I.e. we've been moving more in the direction of using test-tool
directly, which some tests already do in that case, with some using
test_file_size.

> +	test "$sz" -gt 20 &&
> +	test_must_fail git index-pack --max-input-size=20 pack-$pack.pack 2>err &&
> +	grep "maximum allowed size (20 bytes)" err
> +'

Maybe this is covered by another test, but in case we don't have a test
for this error already: This doesn't cover that we don't error on packs
smaller than the limit, so in terms of black-box testing it's
indistinguishable from us erroring out about this all the time.

But that's probably too paranoid in the first place, and maybe we do
have that other test...