Re: [PATCH v4 06/16] merge_recursive: abort properly upon errors

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > There are a couple of places where return values indicating errors
> > are ignored. Let's teach them manners.
> 
> That is because the return value never indicated errors before this
> series, isn't it?  A true error used to be expressed by dying, and
> the return value indicating "cleanliness" of the merge were
> deliberately ignored.
> 
> The world order changed by previous patches in this series and the
> callers need to be updated to take the new kind of return values
> into account.  That is not teaching them manners ;-)

> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  merge-recursive.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/merge-recursive.c b/merge-recursive.c
> > index dc3182b..2d4cb80 100644
> > --- a/merge-recursive.c
> > +++ b/merge-recursive.c
> > @@ -1949,8 +1949,9 @@ int merge_recursive(struct merge_options *o,
> >  		saved_b2 = o->branch2;
> >  		o->branch1 = "Temporary merge branch 1";
> >  		o->branch2 = "Temporary merge branch 2";
> > -		merge_recursive(o, merged_common_ancestors, iter->item,
> > -				NULL, &merged_common_ancestors);
> > +		if (merge_recursive(o, merged_common_ancestors, iter->item,
> > +				    NULL, &merged_common_ancestors) < 0)
> > +			return -1;
> >  		o->branch1 = saved_b1;
> >  		o->branch2 = saved_b2;
> >  		o->call_depth--;
> 
> This hunk feels somewhat wrong as-is.
> 
> There is a comment before the pre-context explaining why cleanness
> flag is ignored.  It needs to be updated.  We still do not care
> about cleanliness, i.e. 0=clean, 1=merged with conflict, but we now
> can get negative values so we need to reject and return early if
> this call indicates an error.