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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 29, 2020, 19:02 UTC
Message-ID: <xmqqr1su9mbf.fsf@gitster.c.googlers.com>
In-Reply-To: <20200728200847.GA1019822@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 10 quoted lines

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

I agree that it forces us to be more disciplined to separate them into two processes and clearly define the way they communicate with each other. When done poorly, it tends to make a bigger mess.

Show 5 quoted lines

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

Yup.

The "more naive and stupid way to run them as subprocesses, just like we do for cURL based transport" was primarily because I thought it would be easier to get right (as the transport API and interface has long been established) than hunting down all the callers of die() and touch all the functions involved in the callchain. I won't complain or block if a harder-but-more-correct approach is taken ;-)