From: Ben Knoble <ben.knoble@gmail.com>
Date: Tue, 29 Jul 2025 00:15:14 GMT
Subject: Re: [GSoC PATCH v5 5/5] repo: add the --format flag
Message-ID: <CF7744F0-394B-415D-BECC-9D7CD477B78C@gmail.com>
In-Reply-To: <CAPig+cQn7c5+k06yHOD2jxYTGnny7is=fbo4tOw26eD+4zX-Jw@mail.gmail.com>


> Le 27 juil. 2025 à 18:02, Eric Sunshine <sunshine@sunshineco.com> a écrit :
> 
> On Sun, Jul 27, 2025 at 2:02 PM Lucas Seiki Oshiro
> <lucasseikioshiro@gmail.com> wrote:
> 
>> diff --git a/t/t1900-repo.sh b/t/t1900-repo.sh
>> @@ -20,11 +20,20 @@ test_repo_info () {
>> +       test_expect_success "null-terminated: $label" '
>> +               test_when_finished "rm -rf repo" &&
>> +               eval "$init_command" &&
>> +               echo "$expected_value" | lf_to_nul >expected &&
> 
> Simpler:
> 
>    printf "$expected_value\0" >expected &&

(Below as well): the shell linter I use does prefer to see

    printf %s\\0 "$var"

to avoid issues with the variable containing format specifiers. 

(Backslash has to be quoted in double-quotes, too, I think? So I left out the quotes here.)

> 
>> +               git -C repo repo info --format=nul "$key" >output &&
>> +               tail -n 1 output >actual &&
>> +               test_cmp expected actual
>> +       '
> 
> How confident are we that `tail -n 1 output >actual` is going to
> perform as expected across platforms and versions of those platforms?
> It feels awfully fragile to me. Why slice and dice the output anyhow
> rather than merely crafting the correct expected output in the first
> place and comparing that directly against the actual output? In other
> words, something like this:
> 
>    printf "$key\n$expected_value\0" >expect &&
>    git -C repo repo info --format=nul "$key" >actual &&
>    test_cmp_bin expect actual