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

From: Max Kirillov <max@max630.net>
Date: Feb 23, 2019, 07:09 UTC
Message-ID: <20190223070914.GC2354@jessie.local>
In-Reply-To: <87lg2kj91a.fsf@evledraar.gmail.com>

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

Show 7 quoted lines

> 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