Re: [RFC PATCH] Modify fetch-pack to no longer die on error?

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 25, 2020, 23:01 UTC
Message-ID: <xmqqft9fi4hm.fsf@gitster.c.googlers.com>
In-Reply-To: <20200725214118.GA4508@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 10 quoted lines

> I think it was this one:
>
>   https://lore.kernel.org/git/20160927191955.mympqgylrxhkp24n@sigill.intra.peff.net/
>
> I still think it's a good idea, but the hard part is lib-ifying all of
> the functions that call die() to instead return an error up the stack
> (and handle cleanup, etc). Which is why I never really pushed it
> further. But if we're going to be lib-ifying such calls anyway, I think
> it's nice to do this flexible thing (from their perspective it's no more
> work to trigger the callback than it is to call error() and return).

Yeah, I almost 80%+ agree.

The remainder of 20% is that I am not so convinced that (fmt_string
+ va_list) that isn't pretty much usable for anything but generating
human readable error messages is enough.  It is certainly a good
enough replacement for (and much better than) the approach to
prepare an error string in a "struct strbuf err" that was passeed by
the caller, but I am not sure if that is a good bar to aim to pass
to begin with ;-).

> That said, I do think for this particular case, your "just run it in a
> sub-process" suggestion may be simpler and more robust.

For this particular case, with the performance and all, I agree that the stupid and robust approach would be the best.