Re: [PATCH 2/3] diff_flush_patch_id: stop returning error result

[Jeff King <peff@peff.net>]

On Fri, Sep 09, 2016 at 02:58:25PM +0200, Johannes Schindelin wrote:

> > Yes, I agree that this is the opposite direction of libification. And I
> > agree that the current message is not very helpful.
> > 
> > But I am not sure that returning the error up the stack will actually
> > help somebody move forward. The reason these are all die() calls in the
> > rest of the diff code is that they are generally indicative of
> > unrecoverable repository corruption. So any advice does not really
> > depend on what operation you are performing; it is always "stop what you
> > are doing immediately, run fsck, and try to get the broken objects from
> > somebody else".
> > 
> > So IMHO, on balance this is not hurting anything.
> 
> Well, you make such a situation even worse than it already is.
> 
> It would be one thing to change the code to actually say "stop what you
> are doing immediately, run `git fsck` and try to get the broken objects
> from somewhere else", *before* saying how to proceed after that.
> 
> But that is not what your patch does.
> 
> What your patch does is to remove *even the possibility* of saying how to
> proceed after getting the repository corruption fixed. And instead of
> saying how the corruption could be fixed, it outputs a terse "cannot read
> files to diff".
> 
> I do not think that is a wise direction.

First, do not blame me for the terse "cannot read files to diff". That
is the current message. And my patch does not make changing that message
any more difficult. You are welcome to change it in its error() form.
You are welcome to change it in the resulting die().

The quality of that message is totally orthogonal to what the patch is
doing.

The _only_ thing it is losing is the ability to for the caller to then
additionally say "once you have finished uncorrupting the repository,
you can resume your operation with ...".

My point is that this is not useful advice. No callers give it, and I
don't foresee other callers giving it. My argument above was basically
that it is such an exceptional condition it is not worth worrying about.

-Peff