Subject: Re: [PATCH 4/6] t5616: use rev-parse instead to get HEAD's object_id

From: Danh Doan <[email-masked]>
Date: Fri, 20 Mar 2020 07:57:51 +0700

On 2020-03-19 12:07:07-0400, Jeff King <[email-masked]> wrote:
> On Thu, Mar 19, 2020 at 09:00:05PM +0700, Đoàn Trần Công Danh wrote:
> 
> > Only HEAD's object_id is necessary, rev-list is an overkill.
> > 
> > Despite POSIX requires grep(1) treat single pattern with <newline>
> > as multiple patterns.
> > busybox's grep(1) (as of v1.31.1) haven't implemented it yet.
> > 
> > Use rev-parse to simplify the test and avoid busybox unimplemented
> > features.
> 
> That makes sense. It would also future-proof us against the test
> changing the graph such that HEAD actually has ancestors.
> 
> > diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
> > index 77bb91e976..135187c5b5 100755
> > --- a/t/t5616-partial-clone.sh
> > +++ b/t/t5616-partial-clone.sh
> > @@ -49,7 +49,7 @@ test_expect_success 'do partial clone 1' '
> >  test_expect_success 'verify that .promisor file contains refs fetched' '
> >  	ls pc1/.git/objects/pack/pack-*.promisor >promisorlist &&
> >  	test_line_count = 1 promisorlist &&
> > -	git -C srv.bare rev-list HEAD >headhash &&
> > +	git -C srv.bare rev-parse HEAD >headhash &&
> 
> Maybe worth using "rev-parse --verify" which would double check that we
> produced a useful hash (it seems like an unlikely failure mode, but it's
> easy enough to cover).

Yeah, I also thought it wasn't expected to be a failure in this case.
Using `--verify` doesn't cost anything in this case, though.
I will add it in the reroll.

-- 
Danh