Re: [PATCH 0/6] fix test failure with busybox
On 2020-03-19 11:51:36-0400, Jeff King <peff@peff.net> wrote:
Show 27 quoted lines
> On Thu, Mar 19, 2020 at 09:00:01PM +0700, Đoàn Trần Công Danh wrote:
>
> > Despite some non-compiance from busybox's sh(1), grep(1), diff(1),
> > Alpine Linux is still a popular choice for container these days.
> >
> > Fix false-positive failure in testsuite when run in Alpine Linux.
> >
> > t5703.{4,5,6,7} is still failing.
> > Despite git pack-objects behaves correctly,
> > upload-pack.c works incorrectly on this platform.
> >
> > I haven't dig deeper, yet.
>
> I was able to reproduce the problems on Debian (with busybox installed)
> with:
>
> mkdir /tmp/bb
> (cd /tmp/bb
> bb=$(which busybox)
> for i in $($bb --list); do
> ln -s $bb $i
> done)
> PATH=/tmp/bb:$PATH make test TEST_SHELL_PATH=/tmp/bb/sh
>
> 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.
> Our usual strategy here would be to switch to perl, which is more
> predictable about binary bytes.
> 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:
-------------8<--------------
Show changes to 2 files +9 −4
t/helper/test-pkt-line.c, t/t5703-upload-pack-ref-in-want.sh
diff --git a/t/helper/test-pkt-line.c b/t/helper/test-pkt-line.c
index 282d536384..1d62143dbc 100644
--- a/t/helper/test-pkt-line.c
+++ b/t/helper/test-pkt-line.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "test-tool.h"
#include "pkt-line.h"
+#include "strbuf.h"
static void pack_line(const char *line)
{
@@ -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);
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
----------------------->8-------------------
But, this doesn't work. as `packet_reader_read` reads directly from fd 0.
I think perl should be good for now.
--
Danh
Đoàn Trần Công Danh
