* [BUG] git cat-file does not terminate
@ 2011-03-04 13:04 Robert Wruck
2011-03-04 15:40 ` Peter Baumann
0 siblings, 1 reply; 12+ messages in thread
From: Robert Wruck @ 2011-03-04 13:04 UTC (permalink / raw
To: git
Hi,
this is some strange behaviour of cat-file:
On a certain file, `git cat-file blob <objectname>` writes an endless
stream repeating the first 4096 byte of the original file.
cat-file -s and cat-file -t produce correct results.
Even stranger: This only happens with cygwin-git (1.7.4.1).
msysgit (same machine, same repository): works
linux-git (same machine, same repository): works
Even more strange: This only happens with cygwin on a particular machine
(recent cygwin1.dll 1.7.8) under WinXP/32bit. On another machine, recent
cygwin, Windows7/64bit it works...
Debugging a bit, I found that the following happens:
In xwrite (wrapper.c), write() is called with the total file size - in
my case about 87 MB. This call returns -1 and EAGAIN but nevertheless
writes 4096 byte to the output fd. I don't think that's expected
behaviour...
I "fixed" it by limiting each write to 64k (thus looping in
write_in_full) but maybe somebody knows about that cygwin behaviour?
This seems to be the cause of the dreaded "No newline found after blob"
when running `git svn clone` under cygwin on a repository with large files.
You could argue that this is a cygwin bug but maybe limiting each write
to a maximum size is a simple workaround.
-Robert
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] git cat-file does not terminate
2011-03-04 13:04 [BUG] git cat-file does not terminate Robert Wruck
@ 2011-03-04 15:40 ` Peter Baumann
2011-03-04 16:00 ` Jeff King
0 siblings, 1 reply; 12+ messages in thread
From: Peter Baumann @ 2011-03-04 15:40 UTC (permalink / raw
To: Robert Wruck; +Cc: git
On Fri, Mar 04, 2011 at 02:04:00PM +0100, Robert Wruck wrote:
> Hi,
>
> this is some strange behaviour of cat-file:
> On a certain file, `git cat-file blob <objectname>` writes an
> endless stream repeating the first 4096 byte of the original file.
> cat-file -s and cat-file -t produce correct results.
>
> Even stranger: This only happens with cygwin-git (1.7.4.1).
> msysgit (same machine, same repository): works
> linux-git (same machine, same repository): works
>
> Even more strange: This only happens with cygwin on a particular
> machine (recent cygwin1.dll 1.7.8) under WinXP/32bit. On another
> machine, recent cygwin, Windows7/64bit it works...
>
> Debugging a bit, I found that the following happens:
> In xwrite (wrapper.c), write() is called with the total file size -
> in my case about 87 MB. This call returns -1 and EAGAIN but
> nevertheless writes 4096 byte to the output fd. I don't think that's
> expected behaviour...
>
> I "fixed" it by limiting each write to 64k (thus looping in
> write_in_full) but maybe somebody knows about that cygwin behaviour?
>
> This seems to be the cause of the dreaded "No newline found after
> blob" when running `git svn clone` under cygwin on a repository with
> large files.
>
> You could argue that this is a cygwin bug but maybe limiting each
> write to a maximum size is a simple workaround.
>
Maybe you could post a patch, so everyone can see the technical implications
and discuss the fix?
-Peter
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] git cat-file does not terminate
2011-03-04 15:40 ` Peter Baumann
@ 2011-03-04 16:00 ` Jeff King
2011-03-04 17:16 ` Junio C Hamano
0 siblings, 1 reply; 12+ messages in thread
From: Jeff King @ 2011-03-04 16:00 UTC (permalink / raw
To: Peter Baumann; +Cc: Robert Wruck, git
On Fri, Mar 04, 2011 at 04:40:14PM +0100, Peter Baumann wrote:
> > I "fixed" it by limiting each write to 64k (thus looping in
> > write_in_full) but maybe somebody knows about that cygwin behaviour?
> >
> > This seems to be the cause of the dreaded "No newline found after
> > blob" when running `git svn clone` under cygwin on a repository with
> > large files.
> >
> > You could argue that this is a cygwin bug but maybe limiting each
> > write to a maximum size is a simple workaround.
> >
> Maybe you could post a patch, so everyone can see the technical implications
> and discuss the fix?
It would probably look like the patch below, though it really feels like
the right solution is to fix the cygwin bug.
-Peff
---
diff --git a/Makefile b/Makefile
index 4c31d1a..e7d3285 100644
--- a/Makefile
+++ b/Makefile
@@ -167,6 +167,9 @@ all::
# Define NO_ST_BLOCKS_IN_STRUCT_STAT if your platform does not have st_blocks
# field that counts the on-disk footprint in 512-byte blocks.
#
+# Define MAX_WRITE_SIZE to N if your platform has unpredictable results for
+# write() calls larger than N (e.g., cygwin).
+#
# Define ASCIIDOC7 if you want to format documentation with AsciiDoc 7
#
# Define DOCBOOK_XSL_172 if you want to format man pages with DocBook XSL v1.72
@@ -928,6 +931,7 @@ ifeq ($(uname_O),Cygwin)
NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
+ MAX_WRITE_SIZE=65536
# There are conflicting reports about this.
# On some boxes NO_MMAP is needed, and not so elsewhere.
# Try commenting this out if you suspect MMAP is more efficient
@@ -1495,6 +1499,10 @@ ifdef NO_POSIX_GOODIES
BASIC_CFLAGS += -DNO_POSIX_GOODIES
endif
+ifdef MAX_WRITE_SIZE
+ BASIC_CFLAGS += -DMAX_WRITE_SIZE=$(MAX_WRITE_SIZE)
+endif
+
ifdef BLK_SHA1
SHA1_HEADER = "block-sha1/sha1.h"
LIB_OBJS += block-sha1/sha1.o
diff --git a/wrapper.c b/wrapper.c
index 056e9d6..a7a2437 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -133,6 +133,10 @@ ssize_t xread(int fd, void *buf, size_t len)
ssize_t xwrite(int fd, const void *buf, size_t len)
{
ssize_t nr;
+#ifdef MAX_WRITE_SIZE
+ if (len > MAX_WRITE_SIZE)
+ len = MAX_WRITE_SIZE;
+#endif
while (1) {
nr = write(fd, buf, len);
if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [BUG] git cat-file does not terminate
2011-03-04 16:00 ` Jeff King
@ 2011-03-04 17:16 ` Junio C Hamano
2011-03-04 17:29 ` Robert Wruck
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Junio C Hamano @ 2011-03-04 17:16 UTC (permalink / raw
To: Jeff King; +Cc: Peter Baumann, Robert Wruck, git
Jeff King <peff@peff.net> writes:
> It would probably look like the patch below, though it really feels like
> the right solution is to fix the cygwin bug.
Thanks for a quick analysis and fix, folks.
We would need to assume certain things that the platform would give its
users are reliable, and system calls are fundamental part; otherwise we
would end up tying our hands behind our back saying "we cannot use this
and that as these are unreliable in certain places" and litter our
codebase with full of ifdefs and workarounds.
If this were merely of a breakage in a test release or a nightly build, I
would be happier to see us ignore this, but the problematic one is widely
in the wild, a workaround would be necessary, and more importantly, if it
is harder for a casual end-user to tell if the platform is affected (I am
assuming that most releases of Cygwin is without this bug, and most users
who build git themselves wouldn't bother reading README or Makefile even
if we said a MAX_WRITE_SIZE definition is necessary only for this and that
version), I would rather see a change that covers the problem a bit more
widely than necessary.
How prevalent is the problematic cygwin1.dll 1.7.8? Also for how long did
this bug exist, in other words, if we were to make a table of problematic
versions, would we have only just a handful entries in it? Also can we at
runtime find out what version we are running?
The reason I am asking these questions is because I think, assuming that
this would affect many unsuspecting Cygwin users, the best fix would be to
add a hook in the compat/ layer that decides if MAX_WRITE_SIZE workaround
is necessary at runtime, and do something like this:
ssize_t xwrite(int fd, const void *buf, size_t len)
{
ssize_t nr;
static size_t max_write_size = platform_max_write_size();
if (max_write_size && max_write_size < len)
len = max_write_size;
...
}
And then we would have in git-compat-util.h something like:
#define platform_max_write_size() 0
on sane platforms, so that the fix will be optimized away by the compiler.
By the way, does the same version of Cygwin have similar issue on the read
side?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] git cat-file does not terminate
2011-03-04 17:16 ` Junio C Hamano
@ 2011-03-04 17:29 ` Robert Wruck
2011-03-04 18:26 ` Robert Wruck
2011-03-08 21:14 ` Jeff King
2 siblings, 0 replies; 12+ messages in thread
From: Robert Wruck @ 2011-03-04 17:29 UTC (permalink / raw
To: Junio C Hamano; +Cc: Jeff King, Peter Baumann, git
> By the way, does the same version of Cygwin have similar issue on the read
> side?
Actually I've written a small test program on the problematic cygwin
machine that seems to show that the EAGAIN is somehow related to pipes
(e.g. stdout & stuff) and does not occur when fd refers to a file.
I will test this for read() as well but I don't know enough cygwin
internals to tell what other versions may be affected.
It might even be related to 32/64 bit or different Windows versions
since I didn't test more constellations.
-Robert
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] git cat-file does not terminate
2011-03-04 17:16 ` Junio C Hamano
2011-03-04 17:29 ` Robert Wruck
@ 2011-03-04 18:26 ` Robert Wruck
2011-03-08 21:14 ` Jeff King
2 siblings, 0 replies; 12+ messages in thread
From: Robert Wruck @ 2011-03-04 18:26 UTC (permalink / raw
To: Junio C Hamano; +Cc: Jeff King, Peter Baumann, git
> By the way, does the same version of Cygwin have similar issue on the read
> side?
Here some quick results:
read(fd, buffer, 90000000)
Returns total file size for a file and 65536 (errno=0) for pipes (cat
file | readtest). When repeating the read, the whole file is read until
read() returns 0. No problem here on any cygwin I tested.
write(fd, buffer, 90000000)
Returns 90000000 for a file.
Returns 90000000 for redirection (writetest > outfile)
Returns 90000000 for pipes (writetest | cat > outfile) on sane cygwins
and -1 / EAGAIN on the machine with the original problem.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] git cat-file does not terminate
2011-03-04 17:16 ` Junio C Hamano
2011-03-04 17:29 ` Robert Wruck
2011-03-04 18:26 ` Robert Wruck
@ 2011-03-08 21:14 ` Jeff King
2011-03-08 22:52 ` Junio C Hamano
2 siblings, 1 reply; 12+ messages in thread
From: Jeff King @ 2011-03-08 21:14 UTC (permalink / raw
To: Junio C Hamano; +Cc: Peter Baumann, Robert Wruck, git
On Fri, Mar 04, 2011 at 09:16:30AM -0800, Junio C Hamano wrote:
> How prevalent is the problematic cygwin1.dll 1.7.8? Also for how long did
> this bug exist, in other words, if we were to make a table of problematic
> versions, would we have only just a handful entries in it? Also can we at
> runtime find out what version we are running?
>
> The reason I am asking these questions is because I think, assuming that
> this would affect many unsuspecting Cygwin users, the best fix would be to
> add a hook in the compat/ layer that decides if MAX_WRITE_SIZE workaround
> is necessary at runtime, and do something like this:
>
> ssize_t xwrite(int fd, const void *buf, size_t len)
> {
> ssize_t nr;
> static size_t max_write_size = platform_max_write_size();
>
> if (max_write_size && max_write_size < len)
> len = max_write_size;
> ...
> }
How are we doing the runtime test for platform max write?
If I read the original bug report correctly, the problem was that write
would actually write some bytes _and_ return -1, which is terrible. We
can detect "seems to be returning -1 over and over", but we can't handle
a misbehavior like writing and claiming not to have done so.
So I think the test needs to be "is our version of cygwin in the broken
list" and not "let's try a few different writes and see what works".
But it is still not clear to me how many versions have this bug. I think
the next stop is to show the cygwin developers a clear test-case and see
whether it's already fixed, and which versions show the behavior. They
should be able to get that information much more easily than us. I
really don't want to get involved in bisecting bugs in cygwin (according
to cygwin.com, it's kept in CVS. Blech).
Robert, can you try (or have you already tried) submitting a bug report
to Cygwin?
-Peff
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] git cat-file does not terminate
2011-03-08 21:14 ` Jeff King
@ 2011-03-08 22:52 ` Junio C Hamano
2011-03-09 14:49 ` Robert Wruck
0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2011-03-08 22:52 UTC (permalink / raw
To: Jeff King; +Cc: Peter Baumann, Robert Wruck, git
Jeff King <peff@peff.net> writes:
> On Fri, Mar 04, 2011 at 09:16:30AM -0800, Junio C Hamano wrote:
>
>> How prevalent is the problematic cygwin1.dll 1.7.8? Also for how long did
>> this bug exist, in other words, if we were to make a table of problematic
>> versions, would we have only just a handful entries in it? Also can we at
>> runtime find out what version we are running?
>>
>> The reason I am asking these questions is because I think, assuming that
>> ...
> How are we doing the runtime test for platform max write?
I asked (1) if we can find out at runtime if we are on which version of
cygwin1.dll, and (2) if we can have a small list of "bad" versions of
cygwin1.dll. If both are true, the runtime test should be trivial, no?
> So I think the test needs to be "is our version of cygwin in the broken
> list" and not "let's try a few different writes and see what works".
Yes, that is exactly what I had in mind.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] git cat-file does not terminate
2011-03-08 22:52 ` Junio C Hamano
@ 2011-03-09 14:49 ` Robert Wruck
2011-03-09 19:32 ` Junio C Hamano
0 siblings, 1 reply; 12+ messages in thread
From: Robert Wruck @ 2011-03-09 14:49 UTC (permalink / raw
To: Junio C Hamano; +Cc: Jeff King, Peter Baumann, git
> I asked (1) if we can find out at runtime if we are on which version of
> cygwin1.dll, and (2) if we can have a small list of "bad" versions of
> cygwin1.dll. If both are true, the runtime test should be trivial, no?
Currently I don't know of a programmatic way to get the cygwin version
except `cygcheck -c cygwin` or `uname -r` but these utilities seem to
know where to find it. I'll take a look at the source.
Unfortunately, the same cygwin version works on most platforms except
WinXP, so its rather a platform issue and I fear that in this case all
cygwin versions up to a currently unknown fixed version will be subject.
Depending on the machine, the "limit" at which write() fails seems to
vary as well. In my initial report, it was about 80MB, on another
machine it was around 200MB...
I submitted a bug report to cygwin over the weekend and tried to debug
what's going on in cygwin1.dll but haven't gone very far yet.
-Robert
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] git cat-file does not terminate
2011-03-09 14:49 ` Robert Wruck
@ 2011-03-09 19:32 ` Junio C Hamano
2011-03-09 19:51 ` Robert Wruck
0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2011-03-09 19:32 UTC (permalink / raw
To: Robert Wruck; +Cc: Jeff King, Peter Baumann, git
Robert Wruck <wruck@tweerlei.de> writes:
>> I asked (1) if we can find out at runtime if we are on which version of
>> cygwin1.dll, and (2) if we can have a small list of "bad" versions of
>> cygwin1.dll. If both are true, the runtime test should be trivial, no?
>
> Currently I don't know of a programmatic way to get the cygwin version
> except `cygcheck -c cygwin` or `uname -r` but these utilities seem to
> know where to find it. I'll take a look at the source.
Thanks; it is not very critical so don't spend too much effort trying to
find a way to do so at runtime. We can always use the approach Jeff's
Makefile patch took to make it safe (and potentially slow) by default on
all Cygwin while still allowing people on an unaffected version to turn
the workaround off while building.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] git cat-file does not terminate
2011-03-09 19:32 ` Junio C Hamano
@ 2011-03-09 19:51 ` Robert Wruck
2011-03-09 21:43 ` Jeff King
0 siblings, 1 reply; 12+ messages in thread
From: Robert Wruck @ 2011-03-09 19:51 UTC (permalink / raw
To: Junio C Hamano; +Cc: Jeff King, Peter Baumann, git
> Thanks; it is not very critical so don't spend too much effort trying to
> find a way to do so at runtime. We can always use the approach Jeff's
> Makefile patch took to make it safe (and potentially slow) by default on
> all Cygwin while still allowing people on an unaffected version to turn
> the workaround off while building.
The write() issue will presumably be fixed in the next cygwin release. I
already have a version that works on WinXP where it used to fail.
-Robert
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [BUG] git cat-file does not terminate
2011-03-09 19:51 ` Robert Wruck
@ 2011-03-09 21:43 ` Jeff King
0 siblings, 0 replies; 12+ messages in thread
From: Jeff King @ 2011-03-09 21:43 UTC (permalink / raw
To: Robert Wruck; +Cc: Junio C Hamano, Peter Baumann, git
On Wed, Mar 09, 2011 at 08:51:46PM +0100, Robert Wruck wrote:
> >Thanks; it is not very critical so don't spend too much effort trying to
> >find a way to do so at runtime. We can always use the approach Jeff's
> >Makefile patch took to make it safe (and potentially slow) by default on
> >all Cygwin while still allowing people on an unaffected version to turn
> >the workaround off while building.
>
> The write() issue will presumably be fixed in the next cygwin
> release. I already have a version that works on WinXP where it used
> to fail.
Cool. Does our Makefile know the cygwin version number via "uname -r"?
We could at least tweak the build-time patch to turn it on for the right
versions (though we should perhaps wait for the fixed version to be
released so we know what it is. :) ).
Personally I wouldn't bother much with run-time detection. But maybe
cygwin people tend to download binary packages and run them on top of
arbitrary versions cygwin? I don't know what's normal.
-Peff
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2011-03-09 21:43 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-04 13:04 [BUG] git cat-file does not terminate Robert Wruck
2011-03-04 15:40 ` Peter Baumann
2011-03-04 16:00 ` Jeff King
2011-03-04 17:16 ` Junio C Hamano
2011-03-04 17:29 ` Robert Wruck
2011-03-04 18:26 ` Robert Wruck
2011-03-08 21:14 ` Jeff King
2011-03-08 22:52 ` Junio C Hamano
2011-03-09 14:49 ` Robert Wruck
2011-03-09 19:32 ` Junio C Hamano
2011-03-09 19:51 ` Robert Wruck
2011-03-09 21:43 ` Jeff King
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).