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

From: Junio C Hamano <[email-masked]>
Date: Fri, 17 Jan 2025 17:09:01 -0800

Junio C Hamano <[email-masked]> writes:

>> 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.
>
> Is "%.0s" really not portable, or is it just mksh
> being a bit lacking?
>
> "That directive non-portable ..." -> "Some implementations (e.g.
> one that is built into mksh) does not support the precision to be 0
> (i.e. "%.0" before the "s" conversion)"
>
> Other than that, your version is easy to read and understand.
>
>>> -	$(printf "%0.s " $(test_seq 11)) (<[email-masked]> 2005-04-07 15:45:13 -0700 1) abbrev
>>> +	$(printf "%11s" "") (<[email-masked]> 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 for reviewing, and thanks, Jan, for noticing and fixing.

Sorry, as Jan is not a list regular, perhaps I should have
communicated more carefully when I said "Thanks".

The above message from me with "Thanks" does not mean that the patch
is now settled.  There are suggested improvements pending that needs
to be incorporated before the patch becomes acceptable to our tree.

Anybody can help that "further polishing as suggested" step, and
when the patch is left in limbo for too long, I might step in to do
it myself (when I have no other better things to do), but it is
customary around here that the original patch submitter does so.

Thanks.