Re: [PATCH v2 0/4] Sideband the whole fetch v2 response

From: Jonathan Tan <jonathantanmy@google.com>
Date: Jan 16, 2019, 00:38 UTC
Message-ID: <20190116003828.64889-1-jonathantanmy@google.com>
In-Reply-To: <xmqqr2ddpl8q.fsf@gitster-ct.c.googlers.com>

> > Ah...yes, you're right. I forgot to build before running the tests. I'll
> > take a look.
> 
> Thanks.

Thanks once again for taking a look. It turns out that it's because progress messages are sometimes split across PKT-LINEs depending on your luck, and we need to retain the "leftover" on a \2 sideband in order to combine it with the next one if necessary. So, for example, the following fixup works:

	diff --git a/sideband.c b/sideband.c
	index c185c38637..d5da587d68 100644
	--- a/sideband.c
	+++ b/sideband.c
	@@ -117,7 +117,7 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
	 int demultiplex_sideband(const char *me, char *buf, int len, int die_on_error)
	 {
	        static const char *suffix;
	-       struct strbuf outbuf = STRBUF_INIT;
	+       static struct strbuf outbuf = STRBUF_INIT;
	        int retval = 0;
	        const char *b, *brk;
	        int band;
	@@ -187,8 +187,7 @@ int demultiplex_sideband(const char *me, char *buf, int len, int die_on_error)
	                                    "" : DISPLAY_PREFIX);
	                        maybe_colorize_sideband(&outbuf, b, strlen(b));
	                }
	-               retval = SIDEBAND_PROGRESS;
	-               break;
	+               return SIDEBAND_PROGRESS; /* skip cleanup */
	        case 1:
	                retval = SIDEBAND_PRIMARY;
	                break;

We could make the caller of demultiplex_sideband() store the outbuf, but at this point, it might be better to refactor packet_reader into its own file and have it depend on both pkt-line.h and sideband.h. If you (or anyone else) have any ideas, let me know what you think. I'll think further about this too.