git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 18, 2025, 01:09 UTC
Message-ID
<xmqqikqd3p3m.fsf@gitster.g>
In-Reply-To
<xmqqr056bv8v.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
Show 33 quoted lines
>> 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)) (<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 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.
Previous: Junio C HamanoNext: Jan Palus
Message 4 of 6 in “t8002-blame: simplify padding generation in blank boundary tests”
  1. t8002-blame: simplify padding generation in blank boundary testsJan Palus, Jan 11, 2025
  2. Patrick SteinhardtJan 13, 2025
  3. Junio C HamanoJan 13, 2025
  4. Junio C HamanoJan 18, 2025
  5. Jan PalusJan 20, 2025
  6. Junio C HamanoJan 21, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.