Re: [PATCH 1/1] upload-pack: buffer ref advertisement writes

[Jeff King <peff@peff.net>]

On Tue, Aug 24, 2021 at 02:42:20PM -0700, Junio C Hamano wrote:

> > None of this looks wrong to me, but it might be nice to teach the
> > packet writer a buffered mode that would handle this for us. It would be
> > especially nice to bolt the final flush onto packet_flush(), since it is
> > otherwise easy to forget to do.
> 
> FWIW, the packet-line part of the system was from the beginning
> written with an eye to allow buffering until _flush() comes; we may
> have added some buggy conversation path that deadlocks if we make
> the non-flush packets fully buffered, so there may need some fixes,
> but I do not expect the fallout would be too hard to diagnose.
> 
> It may be worth trying that avenue first before piling on the user
> level buffering like this patch does.

Yeah, I had the same thought. It also feels like this is a problem
already solved by stdio. I.e., a lot of the packet_* functions can
handle descriptors or strbufs. Why not "FILE *" handles?

It would probably involve using the original descriptor _and_ the
filehandle in some cases (e.g., ref advertisement over the handle, and
then muxing pack-objects output straight to the descriptor). But that's
OK as long we are sensible about flushing at the right moments.

It may not be much less complex than just implementing buffering in the
packet_* interfaces, though. The tricky part is likely to be the
interface (not itself, but avoiding repetition between all the
fd/strbuf/buffered variants).

-Peff