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

[Jeff King <peff@peff.net>]

On Tue, Jul 28, 2020 at 12:23:50PM -0700, Jonathan Tan wrote:

> > For this particular case, with the performance and all, I agree that
> > the stupid and robust approach would be the best.
> 
> I'm concerned that we will be painting ourselves into a corner here - I
> have been appreciating the richer interface that a C call provides us,
> compared to sub-processes where we have to communicate through a single
> input stream and a single output stream. For example, "wanted-refs" and
> [...]

Yeah, that's a compelling reason. I'd have thought for this use case you
could just say "hey, make sure these objects exist", which doesn't
require a lot of communication. But often when I think things like that
and start coding, it turns out to be much more complicated. So I am
perfectly willing to believe you that it doesn't apply here. And even
if it did, you're right that we may run into other spots that do need to
pass back more information, but benefit from more lib-ified code that
doesn't die().

Just to play devil's advocate for a moment...

> (Also, I think that debugging within a process is easier than debugging
> across processes, but that might not be a concern that other people
> share.)

This is definitely true sometimes, but I think is sometimes the
opposite. When we push things out to a sub-process, then the interface
between the two processes has to be well-defined (e.g., writing results
to a file with a particular format). And that can make debugging easier,
because you can pick up from that intermediate state (munging it in the
middle, or even generating it from scratch for testing).

Likewise, that can result in a more flexible and robust system from the
perspective of users. If we had invented "git log" first, we probably
wouldn't have "git rev-list | git diff-tree --stdin" at all. But having
that as two separate tools is sometimes useful for people doing things
_besides_ log, since it gives different entry points to the code.

That said, I think I could buy the argument that "fetch" works pretty
well as a basic building block for users. It's pretty rare to actually
use fetch-pack as a distinct operation. This is all a monolith vs module
tradeoff question, and the tradeoff around modularity for fetch isn't
that compelling.

-Peff