Thomas Rast <trast@student.ethz.ch> writes:

quoted text
> Interestingly enough, this call survived with slight changes here and
> there from all the way back in cf48454 (Teach git-rev-list to follow
> just a specified set of files, 2005-10-20), where it was added in
> rev-list.c.  Even back then diff_tree() would always return 0.

The idea always have been that diff_tree() may some day start returning
non-fatal errors (otherwise it calls die() so that the caller does not
even have a chance to worry about the return value), and the particular
codepath the patch touches would treat such "one of the trees is unreadble
so we cannot say if/how different they are" case as "there may be a change
worth showing (even if the actual change couldn't be shown)".

So I'm a bit reluctant to accept this patch under discussion.  It doesn't
change the behaviour, it doesn't make the _interface_ any simpler, and it
removes the documentation value of what _should_ happen if diff_tree()
were to be updated in such a way.

It is entirely a different matter if the patch were to change the function
signature of diff_tree() to return "void" and adjust all the callers
involved.  That will make the _interface_ simpler without changing the
behaviour, and it makes it absolutely clear that we would _never_ enhance
git to say "some necessary data for comparison is missing---we report
error and allow the caller to err on the safe side and say 'there could be
difference but the details we cannot say'".

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html