{"thread":{"id":"6253","subject":"problem with git clone on cygwin","startedAt":"2007-01-06T17:03:30Z","lastAt":"2007-01-07T20:01:55Z","messageCount":12,"participants":["Stefan-W. Hahn","Juergen Ruehle","Shawn O. Pearce","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"30973","messageId":"20070106170330.GA8041@scotty.home","threadId":"6253","inReplyTo":null,"subject":"problem with git clone on cygwin","fromName":"Stefan-W. Hahn","fromEmail":"stefan.hahn@s-hahn.de","sentAt":"2007-01-06T17:03:30Z","receivedAt":"2007-01-06T17:03:30Z","isPatch":false,"sender":{"key":"stefan.hahn@s-hahn.de","avatar":null},"body":"Hi,\n\nrunning git on Cygwin I have a problem with git clone on local disk,\nwhile packing data. \n\nThe problem comes with v1.5.0-rc0. I bisected the problem down to\ncommit 6d2fa7 as the first bad commit.\n\nIt seems to be a problem with cygwin.dll prior v1.5.22 and pread(), if\nusing an offset!=0. (I'm running cygwin.dll v1.5.21 build date\n2006-07-27 and I can't update because of other compatibility problems).\n\nSo I tried:\n- not to set NO_MMAP to use real mmap\n- changing get_data_from_pack() from index-pack.c to used mmap() as\n  in 042aea8. (I did this because it directly uses pread().)\nThis solved the problem for my testcase.\n\nThe added testcase also succeeds on Linux but just with v1.4.4 or my\npatch on Cygwin.\n\nIs there anybody else having the same problem in git universe?\n\n-- \nStefan-W. Hahn                          It is easy to make things.\n/ mailto:stefan.hahn@s-hahn.de /        It is hard to make things simple.\t\t\t\n\n\n\ndiff --git a/Makefile b/Makefile\nindex 180e1e0..afa5d08 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -368,7 +368,7 @@ ifeq ($(uname_O),Cygwin)\n \t# There are conflicting reports about this.\n \t# On some boxes NO_MMAP is needed, and not so elsewhere.\n \t# Try commenting this out if you suspect MMAP is more efficient\n-\tNO_MMAP = YesPlease\n+\t#NO_MMAP = YesPlease\n \tNO_IPV6 = YesPlease\n \tX = .exe\n endif\ndiff --git a/index-pack.c b/index-pack.c\nindex 5f6d128..f1d11a0 100644\n--- a/index-pack.c\n+++ b/index-pack.c\n@@ -277,25 +277,27 @@ static void *get_data_from_pack(struct object_entry *obj)\n {\n \tunsigned long from = obj[0].offset + obj[0].hdr_size;\n \tunsigned long len = obj[1].offset - from;\n-\tunsigned char *src, *data;\n+\tunsigned pg_offset = from % getpagesize();\n+\tunsigned char *map, *data;\n \tz_stream stream;\n \tint st;\n \n-\tsrc = xmalloc(len);\n-\tif (pread(pack_fd, src, len, from) != len)\n-\t\tdie(\"cannot pread pack file: %s\", strerror(errno));\n+\tmap = mmap(NULL, len + pg_offset, PROT_READ, MAP_PRIVATE,\n+\t\t   pack_fd, from - pg_offset);\n+\tif (map == MAP_FAILED)\n+\t\tdie(\"cannot mmap pack file: %s\", strerror(errno));\n \tdata = xmalloc(obj->size);\n \tmemset(&stream, 0, sizeof(stream));\n \tstream.next_out = data;\n \tstream.avail_out = obj->size;\n-\tstream.next_in = src;\n+\tstream.next_in = map + pg_offset;\n \tstream.avail_in = len;\n \tinflateInit(&stream);\n \twhile ((st = inflate(&stream, Z_FINISH)) == Z_OK);\n \tinflateEnd(&stream);\n \tif (st != Z_STREAM_END || stream.total_out != obj->size)\n \t\tdie(\"serious inflate inconsistency\");\n-\tfree(src);\n+\tmunmap(map, len + pg_offset);\n \treturn data;\n }\n \ndiff --git a/t/t5610-clone-fail.sh b/t/t5610-clone-fail.sh\nnew file mode 100755\nindex 0000000..d46c255\n--- /dev/null\n+++ b/t/t5610-clone-fail.sh\n@@ -0,0 +1,24 @@\n+#!/bin/sh\n+#\n+\n+test_description='test git-clone failure on cygwin using pread()\n+'\n+\n+. ./test-lib.sh\n+\n+# Need a repo to clone\n+test_create_repo foo2\n+\n+GIT_AUTHOR_EMAIL=xxxxxxxx@yyyyyyyy.yyyyy.yyyyyyy.yyy\n+GIT_COMMITTER_EMAIL=xxxxxxxx@yyyyyyyy.yyyyy.yyyyyyy.yyy\n+export GIT_AUTHOR_EMAIL\n+export GIT_COMMITTER_EMAIL\n+\n+(cd foo2 && echo \"Hello\" > file && git add file && git commit -m 'add file' >/dev/null 2>&1)\n+(cd foo2 && echo \"Hello2\" >> file && git commit -a -m 'test' >/dev/null 2>&1)\n+\n+test_expect_success \\\n+    'clone with resolving' \\\n+    'git-clone foo2 bar2'\n+\n+test_done\n"},{"id":"30983","messageId":"17824.5988.344000.501073@lapjr.intranet.kiel.bmiag.de","threadId":"6253","inReplyTo":"20070106170330.GA8041@scotty.home","subject":"Re: problem with git clone on cygwin","fromName":"Juergen Ruehle","fromEmail":"j.ruehle@bmiag.de","sentAt":"2007-01-06T21:40:52Z","receivedAt":"2007-01-06T21:40:52Z","isPatch":false,"sender":{"key":"j.ruehle@bmiag.de","avatar":null},"body":"Stefan-W. Hahn writes:\n > It seems to be a problem with cygwin.dll prior v1.5.22 and pread(), if\n > using an offset!=0. (I'm running cygwin.dll v1.5.21 build date\n > 2006-07-27 and I can't update because of other compatibility problems).\n\nYou should compile a cygwin.dll from 1.5.21 sources with the pread\npatch applied.\n\nCould you enumerate the other compatability problems?\n"},{"id":"31010","messageId":"20070107060007.GA10351@spearce.org","threadId":"6253","inReplyTo":"20070106170330.GA8041@scotty.home","subject":"Re: problem with git clone on cygwin","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-01-07T06:00:08Z","receivedAt":"2007-01-07T06:00:08Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"\"Stefan-W. Hahn\" <stefan.hahn@s-hahn.de> wrote:\n> running git on Cygwin I have a problem with git clone on local disk,\n> while packing data. \n> \n> The problem comes with v1.5.0-rc0. I bisected the problem down to\n> commit 6d2fa7 as the first bad commit.\n\nThat was a performance improvement for Mac OS X to reduce the\nindex-pack time on linux.git from almost an hour to about 1 minute.\n \n> It seems to be a problem with cygwin.dll prior v1.5.22 and pread(), if\n> using an offset!=0. (I'm running cygwin.dll v1.5.21 build date\n> 2006-07-27 and I can't update because of other compatibility problems).\n\nWhy can't you upgrade Cygwin?\n \n> So I tried:\n> - not to set NO_MMAP to use real mmap\n\nFYI: you can \"unset\" NO_MMAP at compile time:\n\n  make NO_MMAP=\n\nor by putting it into your personal config.mak:\n\n  echo NO_MMAP= >config.mak\n  make\n\nthis way you always build with the real mmap, but don't need to\nlocally patch Makefile.\n\n> - changing get_data_from_pack() from index-pack.c to used mmap() as\n>   in 042aea8. (I did this because it directly uses pread().)\n> This solved the problem for my testcase.\n\nThis isn't ideal because of the performance problems that some\nOSes have with small but frequent mmap() calls.\n\nPerhaps we should add our own fake pread (aka git_pread) to\ngit-compat-util.h and provide a NO_PREAD option in the Makefile?\nThough I don't think its really worth it, as pretty much every\nOS we run on should have a functioning pread call.  Except some\nCygwins apparently.\n\n-- \nShawn.\n"},{"id":"31025","messageId":"20070107093015.GA9909@scotty.home","threadId":"6253","inReplyTo":"20070106215919.GB8041@scotty.home","subject":"Re: problem with git clone on cygwin","fromName":"Stefan-W. Hahn","fromEmail":"stefan.hahn@s-hahn.de","sentAt":"2007-01-07T09:30:15Z","receivedAt":"2007-01-07T09:30:15Z","isPatch":false,"sender":{"key":"stefan.hahn@s-hahn.de","avatar":null},"body":"Also sprach Stefan-W. Hahn am Sat, 06 Jan 2007 at 22:59:19 +0100:\n> Also sprach Juergen Ruehle am Sat, 06 Jan 2007 at 22:40:52 +0100:\n> > You should compile a cygwin.dll from 1.5.21 sources with the pread\n> > patch applied.\n> \n> Thanks for the info.\n> \n> > Could you enumerate the other compatability problems?\n> \n> The problem that one cannot use one version of cygwin.dll and\n> simultaniously another (incomatible) version. (Having a local\n> installed cygwin window open and trying to build in a branch where\n> another version is used.)\n> \n> -- \n> Stefan-W. Hahn                          It is easy to make things.\n> / mailto:stefan.hahn@s-hahn.de /        It is hard to make things simple.\t\t\t\n> \n\n-- \nStefan-W. Hahn                          It is easy to make things.\n/ mailto:stefan.hahn@s-hahn.de /        It is hard to make things simple.\t\t\t\n"},{"id":"31032","messageId":"17824.53141.207000.935133@lapjr.intranet.kiel.bmiag.de","threadId":"6253","inReplyTo":"20070107093015.GA9909@scotty.home","subject":"Re: problem with git clone on cygwin","fromName":"Juergen Ruehle","fromEmail":"j.ruehle@bmiag.de","sentAt":"2007-01-07T10:46:45Z","receivedAt":"2007-01-07T10:46:45Z","isPatch":false,"sender":{"key":"j.ruehle@bmiag.de","avatar":null},"body":"Stefan-W. Hahn writes:\n > Also sprach Juergen Ruehle am Sat, 06 Jan 2007 at 22:40:52 +0100:\n > > Could you enumerate the other compatability problems?\n > \n > The problem that one cannot use one version of cygwin.dll and\n > simultaniously another (incomatible) version. (Having a local\n > installed cygwin window open and trying to build in a branch where\n > another version is used.)\n\nYes that is an unfortunate (though probably unavoidable) downside of\ncygwin. But my question was meant more along the lines of 'could you\ngive some examples of applications that do not work with 1.5.22 or\nlater and prevent you from upgrading your installed cygwin version'.\nIf there are serious showstoppers it would probably be a good idea to\ngo the route Shawn outlined to retain compatibility with earlier\ncygwin releases. Otherwise we'll just keep recommending an upgrade:-)\n\nSorry for being unclear.\n"},{"id":"31038","messageId":"20070107111712.GB9909@scotty.home","threadId":"6253","inReplyTo":"20070107060007.GA10351@spearce.org","subject":"[PATCH] Replacing the system call pread() with real mmap().","fromName":"Stefan-W. Hahn","fromEmail":"stefan.hahn@s-hahn.de","sentAt":"2007-01-07T11:17:12Z","receivedAt":"2007-01-07T11:17:12Z","isPatch":true,"sender":{"key":"stefan.hahn@s-hahn.de","avatar":null},"body":"Using cygwin with cygwin.dll before 1.5.22 the system call pread() is buggy.\nThis patch introduces NO_PREAD. If NO_MMAP is not set and NO_PREAD is set\ngit uses real mmap instead of pread.\n\nSigned-off-by: Stefan-W. Hahn <stefan.hahn@s-hahn.de>\n---\n\nI made a patch for this problem replacing pread().\n\n Makefile          |    9 +++++++++\n compat/pread.c    |   16 ++++++++++++++++\n git-compat-util.h |    5 +++++\n 3 files changed, 30 insertions(+), 0 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 180e1e0..4f928fd 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -69,6 +69,10 @@ all:\n #\n # Define NO_MMAP if you want to avoid mmap.\n #\n+# Define NO_PREAD if you have a problem with pread() system call (i.e.\n+# cygwin.dll before v1.5.22). This needs NO_MMAP not to be set, because pread()\n+# is emulated using mmap.\n+#\n # Define NO_FAST_WORKING_DIRECTORY if accessing objects in pack files is\n # generally faster on your platform than accessing the working directory.\n #\n@@ -521,6 +525,11 @@ endif\n ifdef NO_MMAP\n \tCOMPAT_CFLAGS += -DNO_MMAP\n \tCOMPAT_OBJS += compat/mmap.o\n+else\n+ifdef NO_PREAD\n+\tCOMPAT_CFLAGS += -DNO_PREAD\n+\tCOMPAT_OBJS += compat/pread.o\n+endif\n endif\n ifdef NO_FAST_WORKING_DIRECTORY\n \tBASIC_CFLAGS += -DNO_FAST_WORKING_DIRECTORY\ndiff --git a/compat/pread.c b/compat/pread.c\nnew file mode 100644\nindex 0000000..23e775d\n--- /dev/null\n+++ b/compat/pread.c\n@@ -0,0 +1,16 @@\n+#include \"../git-compat-util.h\"\n+\n+ssize_t git_pread(int fd,void *buf,size_t count,off_t offset)\n+{\n+        unsigned pg_offset = offset % getpagesize();\n+        unsigned char *map;\n+\n+        map = mmap(NULL, count + pg_offset, PROT_READ, MAP_PRIVATE,\n+                   fd, offset - pg_offset);\n+        if (map == MAP_FAILED)\n+                return -1;\n+\n+        memcpy(buf, map + pg_offset, count);\n+        munmap(map, count + pg_offset);\n+        return count;\n+}\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 5d9eb26..413e7e8 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -96,6 +96,11 @@ extern int git_munmap(void *start, size_t length);\n \n #include <sys/mman.h>\n \n+#ifdef NO_PREAD\n+#define pread git_pread\n+extern ssize_t git_pread(int fd,void *buf,size_t count,off_t offset);\n+#endif\n+\n #endif /* NO_MMAP */\n \n #ifdef NO_SETENV\n-- \n1.5.0.rc0.g244a7\n"},{"id":"31039","messageId":"20070107111841.GC9909@scotty.home","threadId":"6253","inReplyTo":"20070107060007.GA10351@spearce.org","subject":"[PATCH] Test for failing pread() on cygwin.","fromName":"Stefan-W. Hahn","fromEmail":"stefan.hahn@s-hahn.de","sentAt":"2007-01-07T11:18:42Z","receivedAt":"2007-01-07T11:18:42Z","isPatch":true,"sender":{"key":"stefan.hahn@s-hahn.de","avatar":null},"body":"Test for failing pread() on cygwin.\n\nSigned-off-by: Stefan-W. Hahn <stefan.hahn@s-hahn.de>\n---\n\nAnd the test which fails on cygwin with 1.5.21 and original pread().\n\n t/t5610-clone-fail.sh |   29 +++++++++++++++++++++++++++++\n 1 files changed, 29 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5610-clone-fail.sh b/t/t5610-clone-fail.sh\nnew file mode 100644\nindex 0000000..d30b25e\n--- /dev/null\n+++ b/t/t5610-clone-fail.sh\n@@ -0,0 +1,29 @@\n+#!/bin/sh\n+#\n+# Copyright (C) 2007 Stefan-W. Hahn <stefan.hahn@s-hahn.de>\n+#\n+\n+test_description='test git-clone failure on cygwin using pread()\n+\n+This test covers the fact that git-clone fails, if pread() does not\n+work properly, because the size of objects makes it neccessary to\n+read data from a not zero offset.'\n+\n+. ./test-lib.sh\n+\n+# Need a repo to clone\n+test_create_repo foo2\n+\n+GIT_AUTHOR_EMAIL=xxxxxxxx@yyyyyyyy.yyyyy.yyyyyyy.yyy\n+GIT_COMMITTER_EMAIL=xxxxxxxx@yyyyyyyy.yyyyy.yyyyyyy.yyy\n+export GIT_AUTHOR_EMAIL\n+export GIT_COMMITTER_EMAIL\n+\n+(cd foo2 && echo \"Hello\" > file && git add file && git commit -m 'add file' >/dev/null 2>&1)\n+(cd foo2 && echo \"Hello2\" >> file && git commit -a -m 'test' >/dev/null 2>&1)\n+\n+test_expect_success \\\n+    'clone with resolving' \\\n+    'git-clone foo2 bar2'\n+\n+test_done\n-- \n1.5.0.rc0.g244a7\n"},{"id":"31040","messageId":"20070107112107.GD9909@scotty.home","threadId":"6253","inReplyTo":"17824.53141.207000.935133@lapjr.intranet.kiel.bmiag.de","subject":"Re: problem with git clone on cygwin","fromName":"Stefan-W. Hahn","fromEmail":"stefan.hahn@s-hahn.de","sentAt":"2007-01-07T11:21:07Z","receivedAt":"2007-01-07T11:21:07Z","isPatch":false,"sender":{"key":"stefan.hahn@s-hahn.de","avatar":null},"body":"Also sprach Juergen Ruehle am Sun, 07 Jan 2007 at 11:46:45 +0100:\n> Stefan-W. Hahn writes:\n\n> cygwin. But my question was meant more along the lines of 'could you\n> give some examples of applications that do not work with 1.5.22 or\n> later and prevent you from upgrading your installed cygwin version'.\n\nNon, its just the above metioned incompatibility between the cygwin\nversions.\n\n> If there are serious showstoppers it would probably be a good idea to\n> go the route Shawn outlined to retain compatibility with earlier\n> cygwin releases. Otherwise we'll just keep recommending an upgrade:-)\n\nI sent out a patch using mmap() instead of pread() if configured.\n\n-- \nStefan-W. Hahn                          It is easy to make things.\n/ mailto:stefan.hahn@s-hahn.de /        It is hard to make things simple.\t\t\t\n"},{"id":"31042","messageId":"20070107112445.GE10351@spearce.org","threadId":"6253","inReplyTo":"20070107111712.GB9909@scotty.home","subject":"Re: [PATCH] Replacing the system call pread() with real mmap().","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-01-07T11:24:45Z","receivedAt":"2007-01-07T11:24:45Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"\"Stefan-W. Hahn\" <stefan.hahn@s-hahn.de> wrote:\n> +# Define NO_PREAD if you have a problem with pread() system call (i.e.\n> +# cygwin.dll before v1.5.22). This needs NO_MMAP not to be set, because pread()\n> +# is emulated using mmap.\n\nBetter to emulate pread using lseek/xread/lseek:\n\n\toff_t p = lseek(fd, offset, SEEK_SET);\n\txread(fd, buf, count);\n\tlseek(fd, p, SEEK_SET);\n\nIf you look at the history for contrib/mmap.c you will see an\nimplementation like that before it was called git_mmap().\n\nThis works OK in Git as we only ever have one thread, therefore\nthere is no race condition during the seek/read/seek calls.  Its also\nusually faster on platforms like Mac OS X where mmap() is horrible.\nIf the platform is lacking a good pread() it may also be the case\nthat it lacks a good mmap(); in this case it would be desirable to\nset both NO_MMAP and NO_PREAD.\n\n-- \nShawn.\n"},{"id":"31058","messageId":"20070107163637.GE9909@scotty.home","threadId":"6253","inReplyTo":"20070107112445.GE10351@spearce.org","subject":"Re: [PATCH] Replacing the system call pread() with real mmap().","fromName":"Stefan-W. Hahn","fromEmail":"stefan.hahn@s-hahn.de","sentAt":"2007-01-07T16:36:37Z","receivedAt":"2007-01-07T16:36:37Z","isPatch":true,"sender":{"key":"stefan.hahn@s-hahn.de","avatar":null},"body":"Also sprach Shawn O. Pearce am Sun, 07 Jan 2007 at 06:24:45 -0500:\n> Better to emulate pread using lseek/xread/lseek:\n> \n> \toff_t p = lseek(fd, offset, SEEK_SET);\n> \txread(fd, buf, count);\n> \tlseek(fd, p, SEEK_SET);\n> \n> If you look at the history for contrib/mmap.c you will see an\n> implementation like that before it was called git_mmap().\n\nThis doesn't work;I tried it before.\n\nStefan\n-- \nStefan-W. Hahn                          It is easy to make things.\n/ mailto:stefan.hahn@s-hahn.de /        It is hard to make things simple.\t\t\t\n"},{"id":"31060","messageId":"Pine.LNX.4.63.0701071810220.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6253","inReplyTo":"20070107163637.GE9909@scotty.home","subject":"Re: [PATCH] Replacing the system call pread() with real mmap().","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-01-07T17:10:55Z","receivedAt":"2007-01-07T17:10:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 7 Jan 2007, Stefan-W. Hahn wrote:\n\n> Also sprach Shawn O. Pearce am Sun, 07 Jan 2007 at 06:24:45 -0500:\n> > Better to emulate pread using lseek/xread/lseek:\n> > \n> > \toff_t p = lseek(fd, offset, SEEK_SET);\n> > \txread(fd, buf, count);\n> > \tlseek(fd, p, SEEK_SET);\n> > \n> > If you look at the history for contrib/mmap.c you will see an\n> > implementation like that before it was called git_mmap().\n> \n> This doesn't work;I tried it before.\n\nCare to elaborate? It worked pretty well on _all_ cygwin setups I tested \nwith.\n\nCiao,\nDscho\n"},{"id":"31074","messageId":"20070107200155.GF9909@scotty.home","threadId":"6253","inReplyTo":"Pine.LNX.4.63.0701071810220.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Replacing the system call pread() with real mmap().","fromName":"Stefan-W. Hahn","fromEmail":"stefan.hahn@s-hahn.de","sentAt":"2007-01-07T20:01:55Z","receivedAt":"2007-01-07T20:01:55Z","isPatch":true,"sender":{"key":"stefan.hahn@s-hahn.de","avatar":null},"body":"Also sprach Johannes Schindelin am Sun, 07 Jan 2007 at 18:10:55 +0100:\n\n> Care to elaborate? It worked pretty well on _all_ cygwin setups I tested \n> with.\n\nYes, but not today.\n\nBut after a first look the received pack file is corrupt. It doesn't\nstart with the pack header.\n\nStefan\n\n-- \nStefan-W. Hahn                          It is easy to make things.\n/ mailto:stefan.hahn@s-hahn.de /        It is hard to make things simple.\t\t\t\n"}]}