SIGPIPE in t9300-fast-import

SIGPIPE in t9300-fast-import

[Brian Gernhardt]

The new cat-blob test (9300.114) is failing for me on OS X, and has been since it's introduction in "85c6239 fast-import: let importers retrieve blobs":

---- 8< ----

ok 113 - setup: have pipes?

expecting success: 
	expect_id=$(git hash-object big) &&
	expect_len=$(wc -c <big) &&
	echo $expect_id blob $expect_len >expect.response &&

	rm -f blobs &&
	cat >frontend <<-\FRONTEND_END &&
	#!/bin/sh
	cat <<EOF &&
	feature cat-blob
	blob
	mark :1
	data <<BLOB
	EOF
	cat big
	cat <<EOF
	BLOB
	cat-blob :1
	EOF

	read blob_id type size <&3 &&
	echo "$blob_id $type $size" >response &&
	dd of=blob bs=$size count=1 <&3 &&
	read newline <&3 &&

	cat <<EOF &&
	commit refs/heads/copied
	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
	data <<COMMIT
	copy big file as file3
	COMMIT
	M 644 inline file3
	data <<BLOB
	EOF
	cat blob &&
	cat <<EOF
	BLOB
	EOF
	FRONTEND_END

	mkfifo blobs &&
	(
		export GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL GIT_COMMITTER_DATE &&
		sh frontend 3<blobs |
		git fast-import --cat-blob-fd=3 3>blobs
	) &&
	git show copied:file3 >actual &&
	test_cmp expect.response response &&
	test_cmp big actual

0+1 records in
0+1 records out
8139 bytes transferred in 0.000062 secs (131297847 bytes/sec)
error: git-fast-import died of signal 13
not ok - 114 R: copy using cat-file
---- 8< ----

I don't have the tuits right now to dig into this, but "trash directory.t9300-fast-input" has a good response (`cmp expect.response response` is true), but has no refs/heads/copied.  I can run help provide diagnostics, if anyone needs more data.

~~ Brian

PS:  Isn't t9300 getting a little crazily long?  Is there a good way to split it up by feature or something? It runs quickly, but finding were something is failing is getting a little difficult.

Re: SIGPIPE in t9300-fast-import

[Jonathan Nieder]

Brian Gernhardt wrote:

> 	read blob_id type size <&3 &&
> 	echo "$blob_id $type $size" >response &&
> 	dd of=blob bs=$size count=1 <&3 &&
[...]
> 0+1 records in
> 0+1 records out
> 8139 bytes transferred in 0.000062 secs (131297847 bytes/sec)
> error: git-fast-import died of signal 13
[...]
> I can run help provide diagnostics, if anyone needs more data.

Hmm.  Any idea why dd is ending early?  Does using

	head -c "$size" >blob <&3 &&

or

	dd of=blob bs=1 count=$size <&3 &&

in its place work?  What does the Mac OS X equivalent of

	strace dd of=blob bs=$size count=1 <&3

(dtrace or ktrace, I guess) tell?

Thanks for noticing.

Re: SIGPIPE in t9300-fast-import

[Brian Gernhardt]

On Dec 12, 2010, at 4:49 PM, Jonathan Nieder wrote:

> Hmm.  Any idea why dd is ending early?

No idea.

> Does using
> 
> 	head -c "$size" >blob <&3 &&

Yes.

> 	dd of=blob bs=1 count=$size <&3 &&
> 
> in its place work?

Yes.

> What does the Mac OS X equivalent of
> 
> 	strace dd of=blob bs=$size count=1 <&3
> 
> (dtrace or ktrace, I guess) tell?

The moral equivalent seems to be "dtruss" (which I think is a front end to DTrace) in modern OS X.

0+1 records in
0+1 records out
8139 bytes transferred in 0.000205 secs (39694698 bytes/sec)
SYSCALL(args) 		 = return
ioctl(0x6, 0x80086804, 0x7FFF5FBFD0A0)		 = 0 0
close(0x6)		 = 0 0
__sysctl(0x7FFF5FBFCF90, 0x2, 0x7FFF5FBFCF80)		 = 0 0
bsdthread_register(0x7FFF852F43DC, 0x7FFF852D4FF8, 0x2000)		 = 0 0
thread_selfid(0x7FFF852F43DC, 0x7FFF852D4FF8, 0x0)		 = 1850088 0
open_nocancel("/dev/urandom\0", 0x0, 0x7FFF70ABABE0)		 = 6 0
read_nocancel(0x6, "\350>6\026\310R\251_\227\213h\237\304\330&Q-\247\0313\020BnH\277\337z\276c\247\206\017\340/\302} \2644\273\003\252`\363d8\252\247)V2\323\021\320\\\214\001\331\226\020RY\024I\0", 0x40)		 = 64 0
close_nocancel(0x6)		 = 0 0
mmap(0x0, 0x3000, 0x3, 0x1002, 0x1000000, 0x7FFF00000001)		 = 0xD000 0
__sysctl(0x7FFF5FBFCE10, 0x2, 0x7FFF5FBFCDD0)		 = 0 0
__sysctl(0x7FFF5FBFCDD0, 0x2, 0x7FFF5FBFCE68)		 = 0 0
getpid(0x7FFF5FBFCD60, 0x7FFFFFE00050, 0x0)		 = 94596 0
open_nocancel("/dev/urandom\0", 0x0, 0x0)		 = 6 0
read_nocancel(0x6, "\253\331\354\357A)G\3256\222\024=\242E\261\301\314\351\017K\301;O\256I\270\301\312\263M\307\340+U\217\242\230\335\020\275\356T\323\334\312\313\331*\037\273K\332\a\247\323\324\377m\360\204\375c\026i\345\017x\2070n\026\266\027\021k\340( \245\337\3277H\214\0", 0x6C)		 = 108 0
close_nocancel(0x6)		 = 0 0
__sysctl(0x7FFF5FBFCE10, 0x2, 0x7FFF5FBFCE3C)		 = 0 0
mmap(0x0, 0x17000, 0x3, 0x1002, 0x1000000, 0x7FFF00000001)		 = 0x10000 0
mmap(0x0, 0x17000, 0x3, 0x1002, 0x1000000, 0x7FFF00000001)		 = 0x27000 0
mmap(0x0, 0x1000, 0x3, 0x1002, 0x1000000, 0x7FFF00000001)		 = 0x3E000 0
mmap(0x0, 0x200000, 0x3, 0x1002, 0x7000000, 0x7FFF00000001)		 = 0x3F000 0
munmap(0x10003F000, 0xC1000)		 = 0 0
munmap(0x100200000, 0x3F000)		 = 0 0
__sysctl(0x7FFF5FBFCF70, 0x2, 0x7FFF5FBFCF30)		 = 0 0
__sysctl(0x7FFF5FBFCF30, 0x2, 0x7FFF70AC7760)		 = 0 0
__sysctl(0x7FFF5FBFCF70, 0x2, 0x7FFF5FBFCF30)		 = 0 0
__sysctl(0x7FFF5FBFCF30, 0x2, 0x7FFF70AC7764)		 = 0 0
__sysctl(0x7FFF5FBFCF70, 0x2, 0x7FFF5FBFCF30)		 = 0 0
__sysctl(0x7FFF5FBFCF30, 0x2, 0x7FFF70AC7768)		 = 0 0
mmap(0x0, 0x3000, 0x3, 0x1002, 0x1000000, 0x7FFF00000001)		 = 0x3F000 0
__sysctl(0x7FFF5FBFCF40, 0x2, 0x7FFF5FBFCF00)		 = 0 0
__sysctl(0x7FFF5FBFCF00, 0x2, 0x7FFF5FBFCF98)		 = 0 0
__sysctl(0x7FFF5FBFCF40, 0x2, 0x7FFF5FBFCF6C)		 = 0 0
mmap(0x0, 0x17000, 0x3, 0x1002, 0x1000000, 0x7FFF00000001)		 = 0x42000 0
mmap(0x0, 0x17000, 0x3, 0x1002, 0x1000000, 0x7FFF00000001)		 = 0x59000 0
mmap(0x0, 0x200000, 0x3, 0x1002, 0x7000000, 0x7FFF00000001)		 = 0x200000 0
munmap(0x100300000, 0x100000)		 = 0 0
fstat64(0x0, 0x7FFF5FBFF320, 0x17FFF)		 = 0 0
lseek(0x0, 0x0, 0x1)		 = -1 Err#29
open("blob\0", 0x602, 0x1B6)		 = 6 0
fstat64(0x6, 0x7FFF5FBFF320, 0x0)		 = 0 0
lseek(0x6, 0x0, 0x1)		 = 0 0
mmap(0x0, 0x58000, 0x3, 0x1002, 0x3000000, 0x7FFF00000000)		 = 0x70000 0
mmap(0x0, 0x1000, 0x3, 0x1002, 0x3000000, 0x7FFF00000000)		 = 0xC8000 0
sigaction(0x1D, 0x7FFF5FBFF380, 0x7FFF5FBFF3B0)		 = 0 0
sigaction(0x2, 0x7FFF5FBFF380, 0x7FFF5FBFF3B0)		 = 0 0
read(0x0, "the quick brown fox jumps over the lazy dog\nthe quick brown fox jumps over the lazy dog\nthe quick brown fox jumps over the lazy dog\nthe quick brown fox jumps over the lazy dog\nthe quick brown fox jumps over the lazy dog\nthe quick brown fox jumps over the l", 0x2C000)		 = 8139 0
write(0x6, "the quick brown fox jumps over the lazy dog\nthe quick brown fox jumps over the lazy dog\nthe quick brown fox jumps over the lazy dog\nthe quick brown fox jumps over the lazy dog\nthe quick brown fox jumps over the lazy dog\nthe quick brown fox jumps over the l", 0x1FCB)		 = 8139 0
write(0x2, "0+1 records in\n0+1 records out\n quick brown fox jumps over the lazy dog\nthe quick brown fox jumps over the lazy dog\nthe quick brown fox jumps over the lazy dog\nthe quick brown fox jumps over the lazy dog\nthe quick brown fox jumps over the lazy dog\nthe quic", 0x1F)		 = 31 0
write(0x2, "8139 bytes transferred in 0.000205 secs (39694698 bytes/sec)\nuick brown fox jumps over the lazy dog\nthe quick brown fox jumps over the lazy dog\nthe quick brown fox jumps over the lazy dog\nthe quick brown fox jumps over the lazy dog\nthe quick brown fox jump", 0x3D)		 = 61 0
getpid(0x7FFF5FBFF250, 0x7FFFFFE00050, 0x0)		 = 94596 0
open_nocancel("/dev/urandom\0", 0x0, 0x0)		 = 6 0
read_nocancel(0x6, "\232/\226\373ld\353\r+l\236qV\317\370\271.0\353=\316\3464\t\263\262r\022\362w\360H\245LdE\3769\254\3309m\246I\361\374\307\342]6\n\337\303\310\333\332\036\327\313\224(\3443C\327\224\225\376\221\311-\323\272\344\275l\034\353LR<\236\t\202\250\353\202\276AS\363\023@\353\331[\037\bC\036)\336\363WZ\003\0", 0x6C)		 = 108 0
close_nocancel(0x6)		 = 0 0
issetugid(0x100000000, 0x7FFF5FBFF5B0, 0x7FFF5FC40530)		 = 0 0
geteuid(0x100000000, 0x7FFF5FBFF5B0, 0x0)		 = 0 0
__sysctl(0x7FFF5FBFD140, 0x2, 0x7FFF5FBFD100)		 = 0 0
__sysctl(0x7FFF5FBFD100, 0x2, 0x7FFF5FBFD19C)		 = 0 0
shared_region_check_np(0x7FFF5FBFD308, 0x0, 0x7FFF5FC1DC86)		 = 0 0
stat64("/usr/lib/dtrace/libdtrace_dyld.dylib\0", 0x7FFF5FBFC710, 0x7FFF5FBFCD50 = 0 0
open("/usr/lib/dtrace/libdtrace_dyld.dylib\0", 0x0, 0x0)		 = 6 0
pread(0x6, "\312\376\272\276\0", 0x1000, 0x0)		 = 4096 0
pread(0x6, "\317\372\355\376\a\0", 0x1000, 0x1000)		 = 4096 0
mmap(0x100008000, 0x2000, 0x5, 0x12, 0x6, 0x1FFFFFFFF)		 = 0x8000 0
mmap(0x10000A000, 0x1000, 0x3, 0x12, 0x6, 0x1FFFFFFFF)		 = 0xA000 0
mmap(0x10000B000, 0x1F10, 0x1, 0x12, 0x6, 0x1FFFFFFFF)		 = 0xB000 0
close(0x6)		 = 0 0
stat64("/usr/lib/libSystem.B.dylib\0", 0x7FFF5FBFC4C0, 0x7FFF5FBFCB00)		 = 0 0
stat64("/usr/lib/system/libmathCommon.A.dylib\0", 0x7FFF5FBFC370, 0x7FFF5FBFC9B0)		 = 0 0
madvise(0x7FFF89610000, 0x2000, 0x5)		 = 0 0
open("/dev/dtracehelper\0", 0x2, 0x7FFF5FC45258)		 = 6 0
ioctl(0x6, 0x80086804, 0x7FFF5FBFD0A0)		 = 0 0
close(0x6)		 = 0 0
stat64("/usr/lib/libstdc++.6.dylib\0", 0x7FFF5FBFC4B0, 0x7FFF5FBFCAF0)		 = 0 0
open("/dev/dtracehelper\0", 0x2, 0x7FFF5FC45320)		 = 6 0

error: git-fast-import died of signal 13
---- 8< ----

blob does exist, it just seems to end early:

--- big 2010-12-12 16:56:32.000000000 -0500
+++ blob        2010-12-12 16:56:33.000000000 -0500
@@ -182,3915 +182,4 @@

Followed by many lines of:

-the quick brown fox jumps over the lazy dog

and then:

+the quick brown fox jumps over the lazy dog
\ No newline at end of file

> Thanks for noticing.

I tend to update to latest next, compile, and test about once a day so exactly so I can catch things.  :-D

~~ Brian

[PATCH jn/fast-import-blob-access] t9300: avoid short reads from dd

[Jonathan Nieder]

dd is a thin wrapper around read(2).  As open group Issue 7 explains:

	It shall read the input one block at a time, using the specified
	input block size; it shall then process the block of data
	actually returned, which could be smaller than the requested
	block size.

Any short read --- for example from a pipe whose capacity cannot fill
a block --- results in that block being truncated.  As a result, the
first cat-blob test (9300.114) fails on Mac OS X, where the pipe
capacity is around 8 KiB.

Fix the test by using a block size of 1.  Each read will block until
the next byte of input is available.

It would be even nicer to use head -c which expresses the intention
more clearly.  Alas, IRIX "head" does not support the -c option.

Reported-by: Brian Gernhardt <brian@gernhardtsoftware.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t9300-fast-import.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 055ddc6..ed28d3c 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1794,7 +1794,7 @@ test_expect_success PIPE 'R: copy using cat-file' '
 
 	read blob_id type size <&3 &&
 	echo "$blob_id $type $size" >response &&
-	dd of=blob bs=$size count=1 <&3 &&
+	dd of=blob bs=1 count=$size <&3 &&
 	read newline <&3 &&
 
 	cat <<EOF &&
@@ -1845,7 +1845,7 @@ test_expect_success PIPE 'R: print blob mid-commit' '
 		EOF
 
 		read blob_id type size <&3 &&
-		dd of=actual bs=$size count=1 <&3 &&
+		dd of=actual bs=1 count=$size <&3 &&
 		read newline <&3 &&
 
 		echo
@@ -1880,7 +1880,7 @@ test_expect_success PIPE 'R: print staged blob within commit' '
 		echo "cat-blob $to_get" &&
 
 		read blob_id type size <&3 &&
-		dd of=actual bs=$size count=1 <&3 &&
+		dd of=actual bs=1 count=$size <&3 &&
 		read newline <&3 &&
 
 		echo deleteall
-- 
1.7.2.4

Re: [PATCH jn/fast-import-blob-access] t9300: avoid short reads from dd

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> dd is a thin wrapper around read(2).  As open group Issue 7 explains:
>
> 	It shall read the input one block at a time, using the specified
> 	input block size; it shall then process the block of data
> 	actually returned, which could be smaller than the requested
> 	block size.
>
> Any short read --- for example from a pipe whose capacity cannot fill
> a block --- results in that block being truncated.  As a result, the
> first cat-blob test (9300.114) fails on Mac OS X, where the pipe
> capacity is around 8 KiB.

I saw a similar breakage on my FBSD 8 bochs.  It is unfortunate and feels
yucky that we have to issue 8k+ read(2) of one byte, but I don't think of
a better way.  I thought it might be possible to specify cbs and/or conv
to have the input buffered to a size to defeat the short read issue, but
count specifies in terms of input blocks, so there doesn't seem to be a
way to do so...  Oh well...

Thanks.

[PATCH jn/fast-import-blob-access] t9300: use perl "head -c" clone in place of "dd bs=1 count=16000" kluge

[Jonathan Nieder]

It is unfortunate to have to issue thousands of one-byte read calls to
work around dd's refusal to buffer input that would fill a block after
a short read (a3a6f4, 2010-12-13).  We could do better by using
"head -c", if it were available on all platforms we cared about.
Replace it with some simple perl.

While doing so, restructure 9300.114 to use a subshell instead of a
script.  Subshells can inherit functions (like the new head_c) from
the parent shell while external scripts cannot.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Junio C Hamano wrote:

> It is unfortunate and feels
> yucky that we have to issue 8k+ read(2) of one byte

How about this?

 t/t9300-fast-import.sh |   84 ++++++++++++++++++++++++++++--------------------
 1 files changed, 49 insertions(+), 35 deletions(-)

diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index ed28d3c..924a833 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -7,6 +7,23 @@ test_description='test git fast-import utility'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
 
+# Print $1 bytes from stdin to stdout.
+#
+# This could be written as "head -c $1", but IRIX "head" does not
+# support the -c option.
+head_c () {
+	perl -e '
+		my $len = $ARGV[1];
+		while ($len > 0) {
+			my $s;
+			my $nread = sysread(STDIN, $s, $len);
+			die "cannot read: $!" unless defined($nread);
+			print $s;
+			$len -= $nread;
+		}
+	' - "$1"
+}
+
 file2_data='file2
 second line of EOF'
 
@@ -1780,44 +1797,41 @@ test_expect_success PIPE 'R: copy using cat-file' '
 	rm -f blobs &&
 	cat >frontend <<-\FRONTEND_END &&
 	#!/bin/sh
-	cat <<EOF &&
-	feature cat-blob
-	blob
-	mark :1
-	data <<BLOB
-	EOF
-	cat big
-	cat <<EOF
-	BLOB
-	cat-blob :1
-	EOF
-
-	read blob_id type size <&3 &&
-	echo "$blob_id $type $size" >response &&
-	dd of=blob bs=1 count=$size <&3 &&
-	read newline <&3 &&
-
-	cat <<EOF &&
-	commit refs/heads/copied
-	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
-	data <<COMMIT
-	copy big file as file3
-	COMMIT
-	M 644 inline file3
-	data <<BLOB
-	EOF
-	cat blob &&
-	cat <<EOF
-	BLOB
-	EOF
 	FRONTEND_END
 
 	mkfifo blobs &&
 	(
 		export GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL GIT_COMMITTER_DATE &&
-		sh frontend 3<blobs |
-		git fast-import --cat-blob-fd=3 3>blobs
-	) &&
+		cat <<-\EOF &&
+		feature cat-blob
+		blob
+		mark :1
+		data <<BLOB
+		EOF
+		cat big &&
+		cat <<-\EOF &&
+		BLOB
+		cat-blob :1
+		EOF
+
+		read blob_id type size <&3 &&
+		echo "$blob_id $type $size" >response &&
+		head_c $size >blob <&3 &&
+		read newline <&3 &&
+
+		cat <<-EOF &&
+		commit refs/heads/copied
+		committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+		data <<COMMIT
+		copy big file as file3
+		COMMIT
+		M 644 inline file3
+		data <<BLOB
+		EOF
+		cat blob &&
+		echo BLOB
+	) 3<blobs |
+	git fast-import --cat-blob-fd=3 3>blobs &&
 	git show copied:file3 >actual &&
 	test_cmp expect.response response &&
 	test_cmp big actual
@@ -1845,7 +1859,7 @@ test_expect_success PIPE 'R: print blob mid-commit' '
 		EOF
 
 		read blob_id type size <&3 &&
-		dd of=actual bs=1 count=$size <&3 &&
+		head_c $size >actual <&3 &&
 		read newline <&3 &&
 
 		echo
@@ -1880,7 +1894,7 @@ test_expect_success PIPE 'R: print staged blob within commit' '
 		echo "cat-blob $to_get" &&
 
 		read blob_id type size <&3 &&
-		dd of=actual bs=1 count=$size <&3 &&
+		head_c $size >actual <&3 &&
 		read newline <&3 &&
 
 		echo deleteall
-- 
1.7.2.4.568.g3733c.dirty