Re: [PATCH] t8002-blame: simplify padding generation in blank boundary tests

From: Patrick Steinhardt <ps@pks.im>
Date: Jan 13, 2025, 12:36 UTC
Message-ID: <Z4UIWId7ExLB2gWJ@pks.im>
In-Reply-To: <20250111231107.2190448-1-jpalus@fastmail.com>

On Sun, Jan 12, 2025 at 12:11:07AM +0100, Jan Palus wrote:

Show 5 quoted lines

> Fixes compatibility with mksh as well:
> $ mksh -c 'printf "%0.s" ""'
> printf: %0.s: invalid conversion specification
> 
> Fixes: e7fb2ca945 ("builtin/blame: fix out-of-bounds write with blank boundary commits")

We don't typically use Fixes tags in our project, but instead embed the commit into the commit message with `git log --format=reference -1` together with a description.

The subject can also be adjusted a bit: we use to just write the test number, and the important aspect is not that we simplify the padding generation, but that we make it more portable.

So, my suggestion would be:

    t8002: fix unportable printf formatting directives

    In e7fb2ca945 (builtin/blame: fix out-of-bounds write with blank
    boundary commits, 2025-01-10), we have introduced two new tests that
    expect a certain amount of padding. This padding is generated via
    printf using the "%0.s" formatting directive. That directive is
    non-portable and not understood by for example mksh, breaking these
    tests on platforms using that shell.

    Fix this issue by using "%${N}s" instead, which is already being
    used in t5300 and thus portable enough for us.

Show 18 quoted lines

> Signed-off-by: Jan Palus <jpalus@fastmail.com>
> ---
>  t/t8002-blame.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh
> index 1ad039e123..e98993276a 100755
> --- a/t/t8002-blame.sh
> +++ b/t/t8002-blame.sh
> @@ -138,7 +138,7 @@ test_expect_success 'blame --abbrev -b truncates the blank boundary' '
>  	# Note that `--abbrev=` always gets incremented by 1, which is why we
>  	# expect 11 leading spaces and not 10.
>  	cat >expect <<-EOF &&
> -	$(printf "%0.s " $(test_seq 11)) (<author@example.com> 2005-04-07 15:45:13 -0700 1) abbrev
> +	$(printf "%11s" "") (<author@example.com> 2005-04-07 15:45:13 -0700 1) abbrev
>  	EOF
>  	git blame -b --abbrev=10 ^HEAD -- abbrev.t >actual &&
>  	test_cmp expect actual

Okay, makes sense. And as mentioned, we already have such a use of printf in t5300, so it should be portable enough for our use case.

Thanks!

Patrick