Re: [RFC PATCH] pack-refs: fail on falsely sorted packed-refs

[Max Kirillov <max@max630.net>]

On Wed, Feb 13, 2019 at 11:08:01AM +0100, Ævar Arnfjörð Bjarmason wrote:
> You have an extra two whitespaces after "&&" there.

Thanks, will check it.

>> +	git commit --allow-empty -m commit &&
> Looks like just "test_commit A" would do here.

About this I'm not sure. AFAIK test_commit does lots of stuff,
so can it be considered "just" compared to "commit
--allow-empty" or the opposite? I could replace it with
test_commit for uniformity reason though.

> We can fail in these sorts of loops. There's a few ways to deal with
> that. Doing it like this with "break" will still silently hide errors:

Thanks, this was pointed point

>> +	printf "$head_object refs/heads/b00\\n" >>.git/packed-refs &&
> 
> Looks like just "echo" here would be simpler since we only use printf to
> add a newline.

Could it happen so that "echo" adds '\r\n' at Windows? I
could use echo.

> Instead of "! git ..." use "test_must_fail git ...". See t/README. This
> will hide e.g. segfaults.

Thanks, this was pointed point

> Also, perhaps:
> 
>     test_must_fail git ... 2>stderr &&
>     grep "broken sorting in packed-refs" stderr
> 
> Would make this more obvious/self-documenting so we know we failed due
> to that issue in particular.

Thanks, will change it