Re: [PATCH] Make rev_compare_tree less confusing.

Thomas Rast [off-list ref] writes:

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'".