Re: [PATCH 0/6] fix test failure with busybox

[Jeff King <peff@peff.net>]

On Fri, Mar 20, 2020 at 07:37:15AM +0700, Danh Doan wrote:

> > The issue in t5703 is the sed call in get_actual_commits(). It's trying
> > to cut off the early (text) part of the file, and pass through the
> > binary goo of the packfile. Presumably busybox's sed isn't binary-clean.
> 
> I've checked, busybox's sed think every input is text file,
> and in POSIX, every line in every text file must be terminated by
> <newline>.
> 
> Thus, busybox's sed append a <newline> after `0000` marker, render the
> pack file invalid.

Ah, thanks for tracking it down.

> > We're also feeding this into a test-tool helper. It's possible that
> > helper could be made smart enough to replace both this sed invocation
> > and the one in get_actual_refs().
> 
> I looked into this direction, I guess you meant something like this:
> [...]
> @@ -53,6 +54,13 @@ static void unpack(void)
>  static void unpack_sideband(void)
>  {
>  	struct packet_reader reader;
> +	struct strbuf buf = STRBUF_INIT;
> +
> +	while (strbuf_getline(&buf, stdin) == 0)
> +		if (strstr(buf.buf, "packfile"))
> +		    break;
> +	strbuf_release(&buf);
> +
>  	packet_reader_init(&reader, 0, NULL, 0,
>  			   PACKET_READ_GENTLE_ON_EOF |
>  			   PACKET_READ_CHOMP_NEWLINE);
> [...]
> But, this doesn't work. as `packet_reader_read` reads directly from fd 0.

You'd want to decode the packets first first, and then skip until after
you see the packet with "packfile" in it. Or to be more generic, just
wait until you see packets with actual sideband data. Something like the
patch below works (though I'm also fine with just switching to perl).

diff --git a/t/helper/test-pkt-line.c b/t/helper/test-pkt-line.c
index 282d536384..12ca698e17 100644
--- a/t/helper/test-pkt-line.c
+++ b/t/helper/test-pkt-line.c
@@ -67,7 +67,7 @@ static void unpack_sideband(void)
 		case PACKET_READ_NORMAL:
 			band = reader.line[0] & 0xff;
 			if (band < 1 || band > 2)
-				die("unexpected side band %d", band);
+				continue; /* skip non-sideband packets */
 			fd = band;
 
 			write_or_die(fd, reader.line + 1, reader.pktlen - 1);
diff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh
index 7fba3063bf..a34460f7d8 100755
--- a/t/t5703-upload-pack-ref-in-want.sh
+++ b/t/t5703-upload-pack-ref-in-want.sh
@@ -13,10 +13,7 @@ get_actual_refs () {
 }
 
 get_actual_commits () {
-	sed -n -e '/packfile/,/0000/{
-		/packfile/d
-		p
-		}' <out | test-tool pkt-line unpack-sideband >o.pack &&
+	test-tool pkt-line unpack-sideband <out >o.pack &&
 	git index-pack o.pack &&
 	git verify-pack -v o.idx >objs &&
 	grep commit objs | cut -d" " -f1 | sort >actual_commits