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

From: Jeff King <peff@peff.net>
Date: Jul 28, 2020, 20:08 UTC
Message-ID: <20200728200847.GA1019822@coredump.intra.peff.net>
In-Reply-To: <20200728192350.352978-1-jonathantanmy@google.com>

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

Show 8 quoted lines

> > 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