Re: [GSoC PATCH v2] backfill: error out when HEAD cannot be parsed

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Trieu Huynh <vikingtc4@gmail.com> writes:
> >
> >> From: Trieu Huynh <vikingtc4@gmail.com>
> >>
> >> handle_revision_arg() returns non-zero on failure, but do_backfill()
> >> ignores the return value. On an empty repo with no commits, HEAD is
> >> unborn and handle_revision_arg() fails, but backfill silently
> >> continues with an empty revision walk and exists with a zero return
> >> code.
> >
> > "exists" -> "exits", I think.
> >
> > But more importantly (with Devil's advocate hat on), what's the
> > downside of the current behaviour?
> >
> > You tell the command to backfill, the machinery does not find
> > anything necessary to fetch to backfill, and successfully, quickly,
> > and quietly exits.  That sounds like a graceful exit to me.
> >
> > Is there anything wrong with that?
> >
> >> +test_expect_success 'backfill on empty repo fails gracefully' '
> >> +	git init empty-repo &&
> >> +	test_must_fail git -C empty-repo backfill 2>err &&
> >> +	test_grep "unable to parse HEAD" err
> >> +'
> 
> By the way, a more relevant thing to mention is that this change
> will probably become totally unnecessary in the presence of the
> ds/backfill-revs topic that is already in 'next'.
> 

> It does the usual "if you do not get revision range, fall back to
> HEAD", so
> 
>     git backfill<RET>
> 
> in an empty repository gracefully does nothing, while giving
> revision ranges explicitly, like,
> 
>     git backfill master..next
>     git backfill HEAD
> 
> in such a repository will be greeted with a more explicit "bad
> revision" error.
> 
> Another lesson to pay closer attention to what others are doing in
> the same project.  This would have been easily discoverable if you
> attempted trial merges to 'next' and to 'seen' after you tested your
> change standalone (well, that is how I recalled the other topic
> anyway).
>