login
Header Space

 
 

Re: First cut at git port to Cygwin

Previous thread: Multi-head fetch proposal by Johannes Schindelin on Wednesday, September 28, 2005 - 8:25 pm. (2 messages)

Next thread: Edit log message after commit by Kevin Leung on Wednesday, September 28, 2005 - 10:28 pm. (9 messages)
To: Git Mailing List <git@...>
Date: Wednesday, September 28, 2005 - 8:53 pm

I have made a first cut at a git port to Cygwin.  It looks like the 
"git-diff-tree -p" problem has been resolved independently, or at least 
I can't reproduce it on a fresh Cygwin install (running on XP Home), but 
I have added support for running without the IPv6 and the getaddrinfo() API.

There are still funnies.  In particular, Cygwin and Samba handle 
symlinks differently, so you can't trivially share a repository via 
Samba.  Linus' "symbolic refs" changes should eventually take care of that.

Another funny which I haven't been able to figure out yet is that 'gitk' 
scrunches all its output up into a few pixels at the top of the window. 
  If I maximize the window, I can manually resize most of the panes and 
the output looks correct, but the highlighted text in the top panes show 
up in black on a really really dark blue background and is thus illegible.

I have set up a git-on-Cygwin temporary tree at:

http://www.kernel.org/pub/scm/git/git-cygwin.git

	-hpa
-
To: H. Peter Anvin <hpa@...>
Cc: Git Mailing List <git@...>
Date: Wednesday, October 5, 2005 - 9:16 am

I have a few things I experienced with the merged cygwin stuff. Sorry I
haven't investigated it further, but there should be enough for a few
fixes.

When I ...

  user@machine /usr/local/dev/git/git
  $ make prefix=/usr/local install
  install -d -m755 /usr/local/bin
  install git-apply.exe [...]
  sh ./cmd-rename.sh /usr/local/bin
  ln: creating symbolic link `/usr/local/bin/git-http-pull.exe' to `git-http-fetch.exe': File exists
  make: *** [install] Error 1

Can be fixed by the patch below. I don't know if it would be cleaner to
pass cmd-rename.sh "$X" as a second argument from the Makefile.

--- cmd-rename.sh	2005-10-05 14:42:00.000000000 +0200
+++ cmd-rename.sh-orig	2005-10-05 14:43:48.000000000 +0200
@@ -3,7 +3,7 @@
 test -d "$d" || exit
 while read old new
 do
-	rm -f "$d/$old" "$d/$old.exe" 
+	rm -f "$d/$old"
 	if test -f "$d/$new"
 	then
 		ln -s "$new" "$d/$old" || exit


Some other obscurities ...

  user@machine /usr/local/dev/git/git
  $ git-log
  fatal: Not a git repository

  user@machine /usr/local/dev/git/git
  $ GIT_DIR=.git git-log | wc -l
  26094

and I cannot rebuild the index file with git-reset. First time I run it
it creates the index.lock file and errors out when writing. The second
time it errors out because the lock file was not removed in the first
case.

  user@machine /usr/local/dev/git/git
  $ GIT_DIR=.git git-reset
  fatal: unable to write new index file
  
  user@machine /usr/local/dev/git/git
  $ GIT_DIR=.git git-reset
  fatal: unable to create new cachefile
  
  user@machine /usr/local/dev/git/git
  $ uname -a
  CYGWIN_NT-5.1 antimatter 1.5.18(0.132/4/2) 2005-07-02 20:30 i686 unknown unknown Cygwin

-- 
Jonas Fonseca
-
To: Jonas Fonseca <fonseca@...>
Cc: H. Peter Anvin <hpa@...>, Git Mailing List <git@...>
Date: Wednesday, October 5, 2005 - 9:58 am

Hi,


That could have its cause in your .git/HEAD being no symlink. That happen=
s=20
when rsync=B4ing the .git directory.

The other errors could also stem from the fact that quite a few places=20
expect HEAD to be a symlink.

Ciao,
Dscho
To: Junio C Hamano <junkio@...>
Cc: H. Peter Anvin <hpa@...>, Johannes Schindelin <Johannes.Schindelin@...>, Git Mailing List <git@...>
Date: Wednesday, October 5, 2005 - 11:52 am

&gt; when rsync
To: Jonas Fonseca <fonseca@...>
Cc: <git@...>
Date: Wednesday, October 5, 2005 - 12:54 pm

Thanks.

One request, not just to Jonas.  Please do not use '^---$' to
separate the introductory discussion and the real commit log
message.

Linus style (recently the kernel list had a thread on this as
well) is to have the commit log upfront with signoff, three-dash
line, optional discussion and diffstat, and then diff.

I do not mind seeing discussion upfront personally [*1*], but
the thing is the tool treats everything after the first '^---$' 
something to be fed to patch, and does not treat it as the
commit log message.


[Footnote]

*1* ...but remember, Linus does.

-
To: H. Peter Anvin <hpa@...>
Cc: Git Mailing List <git@...>
Date: Tuesday, October 4, 2005 - 8:31 am

I noticed that rename(2) in my copy of cygwin (1.5.18-1) does not remove the
target and returns an error (probably EPERM, but I have reasons not to trust
strerror on that thing).
The repository was on FAT.
Taking "rename(2)" from cygwin's libiberty solved this (they unlink if link(2)
returns EEXIST).

PS: Does broken rename(2) qualify a system "not worthy to support"?
-
To: Alex Riesen <raa.lkml@...>
Cc: Git Mailing List <git@...>
Date: Tuesday, October 4, 2005 - 11:03 am

I just tried this with Cygwin 1.5.18-1 and didn't have any such 
problems.  I tried it on NTFS, FAT and Samba, using WinXP.

	-hpa
-
To: H. Peter Anvin <hpa@...>
Cc: Git Mailing List <git@...>
Date: Wednesday, October 5, 2005 - 7:24 am

It's on Win2k, there was multiple cygwin installations in path, the other one
supposedly is 1.5.5 (it's from QNX Momentics installation).
I had that old "cygwin1.dll" renamed into "cygwin1.dll-disabled" long
ago, though...
I can't reproduce this out of GIT context, and the error is not
reproducable after
I removed the other cygwin installation out of PATH.
Anyway, sorry, I should have tried this before posting.
-
To: H. Peter Anvin <hpa@...>
Cc: Git Mailing List <git@...>
Date: Wednesday, October 5, 2005 - 11:46 am

Still does not work for me. I cannot isolate the problem out of git,
but at the moment the only way for me to make commit_index_file to work
is to put unlink(indexfile) before rename(cf-&gt;lockfile, indexfile).

For everyone interested, I attach cygwin's strace output here.
To: Git Mailing List <git@...>
Date: Wednesday, October 5, 2005 - 11:54 am

I'm sorry that I missed this thread.  I'm usually pretty alert to the word
"cygwin" showing up in a subject.

I'll go back and read the archives to catch up but, at the risk of
making an observation that has already been made, under windows you
can't always rename a file that is open.  Is that what's happening here?

--
Christopher Faylor			spammer? -&gt;	aaaspam@sourceware.org
Cygwin Co-Project Leader				aaaspam@duffek.com
TimeSys, Inc.
-
To: Christopher Faylor <me@...>
Cc: Git Mailing List <git@...>
Date: Wednesday, October 5, 2005 - 3:17 pm

Don't think so, but will check in about 10 hrs. The code in question
is in index.c, commit_index_file.

-
To: Git Mailing List <git@...>
Date: Wednesday, October 5, 2005 - 4:29 pm

Ok.  Looks pretty simple.  FWIW, I've just built git on windows and I
don't see this behavior.  For the most part, it "just works".

I do see the gitk behavior but I'm tk illiterate too, unfortunately, so
I can't help there.

Cygwin's tcl/tk is a funny beast.  It is primarily included (by me) to
allow for the operation of the insight debugger and is more windows than
POSIX.  As was noted, it doesn't interact with Cygwin/X but just draws
windows using the Windows API.

So, tcl/tk are just barely supported in Cygwin currently.  I'm actually
a little surprised that it works as well as it does with gitk.
--
Christopher Faylor			spammer? -&gt;	aaaspam@sourceware.org
Cygwin Co-Project Leader				aaaspam@duffek.com
TimeSys, Inc.
-
To: Git Mailing List <git@...>
Cc: Christopher Faylor <me@...>, Junio C Hamano <junkio@...>, H. Peter Anvin <hpa@...>
Date: Thursday, October 6, 2005 - 5:05 am

Thanks for the hint. There are open files involved (both index.lock and ind=
ex).
I attach the patch which closes index.lock (this is not really needed, btw:
rename works even without closing index.lock) and unmaps the index
(a bit too intrusive). The patch fixes only update-index.c (the one I had
problems with), there probably are other places were the situation is alike=
.

I don't like the patch (and win32 at all; hence the offending comment),
so use it only unless there is no other possibility to workaround.
I specifically do not request its inclusion into official branch
(even though Junio is cc'ed).
To: Git Mailing List <git@...>
Cc: Christopher Faylor <me@...>, Junio C Hamano <junkio@...>, H. Peter Anvin <hpa@...>
Date: Thursday, October 6, 2005 - 6:07 am

ke.

of course there are "other places". Please, try the attached patch instead.

For the record: the patch is supposed to help people with
"Unable to write new cachefile" kind of errors.
To: Git Mailing List <git@...>
Cc: Christopher Faylor <me@...>, Junio C Hamano <junkio@...>, H. Peter Anvin <hpa@...>
Date: Friday, October 7, 2005 - 8:44 am

Just as I thought the situation improved (by closing index.lock and
unmapping index),
it suddenly get worse: now I'm stuck on git-pull.

git-merge-index (called at some point by git-pull) maps the index in, and starts
git-merge-one-file for each (or the given) entry in the index.
git-merge-one-file
calls git-update-index, which wants to update the index. Which doesn't work,
because it's locked by that piece of s$%^.

The only working walkaround for me atm is unlinking indexfile before rename in
commit_index_file :(
-
To: Alex Riesen <raa.lkml@...>
Cc: Git Mailing List <git@...>, Christopher Faylor <me@...>, Junio C Hamano <junkio@...>, H. Peter Anvin <hpa@...>
Date: Friday, October 7, 2005 - 11:34 am

NOTE! git doesn't use mmap() because it _needs_ to use mmap(), but because 
it was simple to do that way, and it's a total idiosyncracy of mine that I 
often try to mmap the data. I often also tend to do my own allocators 
instead of using malloc() (see my "sparse" project in case you're 
interested in other idiosyncracies of mine - macros to do list traversal 
etc).

The fact is, "mmap()" isn't really any better than "read()": it has some 
advantages wrt memory management for the kernel, which is probably one big 
reason why I do it, but quite frankly, if you were to change every single 
mmap() to be a "map_file()" instead, and made it optional whether it used 
mmap() or "malloc + read()", I personally don't think it would be 
horrible.

And it might make things much simpler for portability. The "use mmap" 
approach is very much a unixism, particularly the way unix people do it 
(mmap followed by close, making the file descriptor "go away"). Sure, 
other OS's have mmap too, but I think on them it tends to be less commonly 
used.

			Linus
-
To: Git Mailing List <git@...>
Cc: Linus Torvalds <torvalds@...>, Christopher Faylor <me@...>, Junio C Hamano <junkio@...>, H. Peter Anvin <hpa@...>
Date: Friday, October 7, 2005 - 4:54 pm

"Sounds like a thinly veiled threat or a very effective prodding" 8)

---

Make read_cache copy the index into memory, to improve portability on
other OS's which have mmap too, tend to use it less commonly.

Signed-off-by: Alex Riesen &lt;raa.lkml@gmail.com&gt;

diff --git a/read-cache.c b/read-cache.c
--- a/read-cache.c
+++ b/read-cache.c
@@ -497,9 +497,11 @@ int read_cache(void)
 	offset = sizeof(*hdr);
 	for (i = 0; i &lt; active_nr; i++) {
 		struct cache_entry *ce = map + offset;
-		offset = offset + ce_size(ce);
-		active_cache[i] = ce;
+		size_t size = ce_size(ce);
+		offset = offset + size;
+		active_cache[i] = malloc(ce, size);
 	}
+	munmap(map, size);
 	return active_nr;
 
 unmap:
-
To: Git Mailing List <git@...>
Cc: Junio C Hamano <junkio@...>, Linus Torvalds <torvalds@...>, Christopher Faylor <me@...>, H. Peter Anvin <hpa@...>
Date: Friday, October 7, 2005 - 5:22 pm

Unbelievable... I actually tested the change! But not _the_ patch.
Thanks. Next time, hit me :)

---

Make read_cache copy the index into memory, to improve portability on
other OS's which have mmap too, tend to use it less commonly.

Signed-off-by: Alex Riesen &lt;raa.lkml@gmail.com&gt;

diff --git a/read-cache.c b/read-cache.c
--- a/read-cache.c
+++ b/read-cache.c
@@ -497,9 +497,12 @@ int read_cache(void)
 	offset = sizeof(*hdr);
 	for (i = 0; i &lt; active_nr; i++) {
 		struct cache_entry *ce = map + offset;
-		offset = offset + ce_size(ce);
-		active_cache[i] = ce;
+		size_t size = ce_size(ce);
+		struct cache_entry *newce = malloc(size);
+		offset = offset + size;
+		active_cache[i] = memcpy(newce, ce, size);
 	}
+	munmap(map, size);
 	return active_nr;
 
 unmap:
-
To: Alex Riesen <fork0@...>
Cc: Git Mailing List <git@...>, Junio C Hamano <junkio@...>, Linus Torvalds <torvalds@...>, Christopher Faylor <me@...>, H. Peter Anvin <hpa@...>
Date: Friday, October 7, 2005 - 5:29 pm

s/malloc/xmalloc/
To: Chuck Lever <cel@...>
Cc: Git Mailing List <git@...>, Junio C Hamano <junkio@...>, Linus Torvalds <torvalds@...>, Christopher Faylor <me@...>, H. Peter Anvin <hpa@...>
Date: Friday, October 7, 2005 - 5:39 pm

It's not that funny after second repost...

---

Make read_cache copy the index into memory, to improve portability on
other OS's which have mmap too, tend to use it less commonly.

Signed-off-by: Alex Riesen &lt;raa.lkml@gmail.com&gt;

diff --git a/read-cache.c b/read-cache.c
--- a/read-cache.c
+++ b/read-cache.c
@@ -497,9 +497,12 @@ int read_cache(void)
 	offset = sizeof(*hdr);
 	for (i = 0; i &lt; active_nr; i++) {
 		struct cache_entry *ce = map + offset;
-		offset = offset + ce_size(ce);
-		active_cache[i] = ce;
+		size_t size = ce_size(ce);
+		struct cache_entry *newce = xmalloc(size);
+		offset = offset + size;
+		active_cache[i] = memcpy(newce, ce, size);
 	}
+	munmap(map, size);
 	return active_nr;
 
 unmap:
-
To: Alex Riesen <fork0@...>
Cc: Chuck Lever <cel@...>, Git Mailing List <git@...>, Junio C Hamano <junkio@...>, Christopher Faylor <me@...>, H. Peter Anvin <hpa@...>
Date: Saturday, October 8, 2005 - 12:11 pm

I really think that you should just get rid of the mmap.

As it is, you're just slowing the code down on sane architectures. That's 
not good.

So I'd suggest something like this instead.

Totally untested, of course.

		Linus

----
diff --git a/read-cache.c b/read-cache.c
--- a/read-cache.c
+++ b/read-cache.c
@@ -454,13 +454,39 @@ static int verify_hdr(struct cache_heade
 	return 0;
 }
 
+static void *map_index_file(int fd, size_t size)
+{
+	void *map;
+#ifdef NO_MMAP
+	map = malloc(size);
+	if (!map)
+		die("Unable to allocate index file mapping");
+	if (read(fd, map, size) != size)
+		die("Unable to read %z bytes from inde
+#else
+	map = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
+	if (map == MAP_FAILED)
+		die("index file mmap failed (%s)", strerror(errno));
+#endif
+	return map;
+}
+
+static void unmap_index_file(void *map, size_t size)
+{
+#ifdef NO_MMAP
+	free(map);
+#else
+	munmap(map, size);
+#endif
+}
+
 int read_cache(void)
 {
 	int fd, i;
 	struct stat st;
 	unsigned long size, offset;
-	void *map;
 	struct cache_header *hdr;
+	void *map;
 
 	errno = EBUSY;
 	if (active_cache)
@@ -475,16 +501,15 @@ int read_cache(void)
 	}
 
 	size = 0; // avoid gcc warning
-	map = MAP_FAILED;
-	if (!fstat(fd, &amp;st)) {
-		size = st.st_size;
-		errno = EINVAL;
-		if (size &gt;= sizeof(struct cache_header) + 20)
-			map = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
-	}
+	if (fstat(fd, &amp;st))
+		die("unable to fstat index file");
+
+	size = st.st_size;
+	errno = EINVAL;
+	if (size &lt; sizeof(struct cache_header) + 20)
+		goto corrupt;
+	map = map_index_file(fd, size);
 	close(fd);
-	if (map == MAP_FAILED)
-		die("index file mmap failed (%s)", strerror(errno));
 
 	hdr = map;
 	if (verify_hdr(hdr, size) &lt; 0)
@@ -503,8 +528,9 @@ int read_cache(void)
 	return active_nr;
 
 unmap:
-	munmap(map, size);
+	unmap_index_file(map, size);
 	errno = EINVAL;
+corrupt:
 	die("index file corrupt"...
To: Linus Torvalds <torvalds@...>
Cc: Alex Riesen <fork0@...>, Chuck Lever <cel@...>, Git Mailing List <git@...>, Junio C Hamano <junkio@...>, Christopher Faylor <me@...>, H. Peter Anvin <hpa@...>
Date: Saturday, October 8, 2005 - 2:27 pm

Hi,


Am I missing something? I don't see where the changes are written back to 
the fd. After all, mmap() is called with PROT_WRITE...

*shameless plug* Of course, this problem does not come up with my NO_MMAP 
patch.

Ciao,
Dscho

-
To: Johannes Schindelin <Johannes.Schindelin@...>
Cc: Linus Torvalds <torvalds@...>, Chuck Lever <cel@...>, Git Mailing List <git@...>, Junio C Hamano <junkio@...>, Christopher Faylor <me@...>, H. Peter Anvin <hpa@...>
Date: Saturday, October 8, 2005 - 2:49 pm

It's just becase the file is open for reading only.
Also, it is not an mmap/unmap implementation. Just reading cache in.

-
To: Johannes Schindelin <Johannes.Schindelin@...>
Cc: <git@...>, Linus Torvalds <torvalds@...>
Date: Saturday, October 8, 2005 - 2:44 pm

PROT_WRITE is true, but we do MAP_PRIVATE, and if I recall
correctly we do not write file via mmap -- at least we do not
intend to.

 - index file is mapped for reading, long ago it was mapped
   read-only but these days we do PROT_WRITE, but updates are
   done via opening a new file and writing afresh.

 - objects are mapped for reading, but, never updated once
   created.  Creation side is regular open - write - close.

 - diff reads original by mapping, but obviously has no business
   writing.


Yes.  It might have been overkill that you supported writing
changes back, though.
.  

-
To: Junio C Hamano <junkio@...>
Cc: Johannes Schindelin <Johannes.Schindelin@...>, <git@...>, Linus Torvalds <torvalds@...>
Date: Monday, October 10, 2005 - 2:43 pm

Not just overkill; if we do MAP_PRIVATE it's actively WRONG.

	-hpa
-
To: H. Peter Anvin <hpa@...>
Cc: Junio C Hamano <junkio@...>, <git@...>, Linus Torvalds <torvalds@...>
Date: Monday, October 10, 2005 - 3:01 pm

Hi,


Not necessarily. Sometimes you need to annotate the data from the index, 

See above.

BTW, is there a mechanism to make sure that the index file is locked 
between reading and writing?

Ciao,
Dscho
-
To: Johannes Schindelin <Johannes.Schindelin@...>
Cc: H. Peter Anvin <hpa@...>, Junio C Hamano <junkio@...>, <git@...>, Linus Torvalds <torvalds@...>
Date: Monday, October 10, 2005 - 4:27 pm

In fact, it is intentional that we open the file O_RDONLY, and mmap it 
PROT_READ | PROT_WRITE, MAP_PRIVATE. We prepare the next index in the 
memory where we mapped the old index, but we don't want to change what's 
on the disk using the mapping; we write that later to a different file 

There's definitely locking; the new file is written to "(filename).lock", 
which is openned O_CREAT | O_EXCL, and is moved to the destination when 
it's complete. I believe everything that intends to write a new index gets 
the lock before reading the old index, although I haven't actually 
checked.

	-Daniel
*This .sig left intentionally blank*
-
To: Johannes Schindelin <Johannes.Schindelin@...>
Cc: Junio C Hamano <junkio@...>, <git@...>, Linus Torvalds <torvalds@...>
Date: Monday, October 10, 2005 - 3:26 pm

In the above sentence, emphasis on "at least we do not intend to."  If 
writes are done legitimately then that's fine, but we shouldn't have 

Eh?  If we MAP_PRIVATE, *and* we (intentionally) write to it, we 
*BETTER* not write anything back.

	-hpa
-
To: H. Peter Anvin <hpa@...>
Cc: Johannes Schindelin <Johannes.Schindelin@...>, <git@...>, Linus Torvalds <torvalds@...>
Date: Monday, October 10, 2005 - 4:34 pm

What I meant to say was "we do not intend to write back the
changes by expecting the modification on mapped area are written
back by mmap() mechanism -- the updates to index file is done by
creat - write - close - rename".  So your saying "the overkill
being actively wrong" was technically correct, but that wrongly
written data was renamed out anyway and no real harm was done.

-
To: Junio C Hamano <junkio@...>
Cc: Johannes Schindelin <Johannes.Schindelin@...>, <git@...>, Linus Torvalds <torvalds@...>
Date: Monday, October 10, 2005 - 4:52 pm

Well, it broke the atomicity of an operation, which *is* a real problem.

Anyway, malloc+read is a dead ringer for MAP_PRIVATE with PROT_WRITE, so 
that makes it even easier to mimic.

	-hpa
-
To: H. Peter Anvin <hpa@...>
Cc: Johannes Schindelin <Johannes.Schindelin@...>, <git@...>, Linus Torvalds <torvalds@...>
Date: Monday, October 10, 2005 - 4:21 pm

Correct.

It has already been fixed by Johannes last week and I merged it
over the weekend if not earlier if I recall correctly.

-
To: H. Peter Anvin <hpa@...>
Cc: Junio C Hamano <junkio@...>, <git@...>, Linus Torvalds <torvalds@...>
Date: Monday, October 10, 2005 - 3:42 pm

Hi,


Yes, those would be bugs. However, if I understood the man page for mmap() 
correctly, then PROT_WRITE &amp;&amp; MAP_PRIVATE makes the data copy-on-write, 
which means that those bugs would have been found (because the changes 
would no longer be present when git was called the next time). And I 

Yes. That was *my* mistake.

Ciao,
Dscho
-
To: Junio C Hamano <junkio@...>
Cc: <git@...>, Linus Torvalds <torvalds@...>
Date: Saturday, October 8, 2005 - 3:04 pm

Hi,



Sure. Something like this?

diff --git a/compat/mmap.c b/compat/mmap.c
index fca6321..fda39fc 100644
--- a/compat/mmap.c
+++ b/compat/mmap.c
@@ -49,7 +49,7 @@ void *gitfakemmap(void *start, size_t le
 		n += count;
 	}
 
-	if(prot &amp; PROT_WRITE) {
+	if((prot &amp; PROT_WRITE) &amp;&amp; !(flags &amp; MAP_PRIVATE)) {
 		fakemmapwritable *next = xmalloc(sizeof(fakemmapwritable));
 		next-&gt;start = start;
 		next-&gt;length = length;
-
To: Johannes Schindelin <Johannes.Schindelin@...>
Cc: <git@...>, Linus Torvalds <torvalds@...>
Date: Saturday, October 8, 2005 - 5:10 pm

Not, really.  What I meant was to rip out the writing out
altogether, and perhaps making sure that the caller never calls
us without MAP_PRIVATE.

-
To: Junio C Hamano <junkio@...>
Cc: <git@...>, Linus Torvalds <torvalds@...>
Date: Saturday, October 8, 2005 - 6:06 pm

Hi,


How about this, then?

[PATCH] If NO_MMAP is defined, fake mmap() and munmap()

Since some platforms do not support mmap() at all, and others do only just so,
this patch introduces the option to fake mmap() and munmap() by malloc()ing the
region explicitely.

Signed-off-by: Johannes Schindelin &lt;Johannes.Schindelin@gmx.de&gt;

---

 Makefile      |    6 ++++++
 cache.h       |   16 ++++++++++++++++
 compat/mmap.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 mailsplit.c   |    1 -
 4 files changed, 73 insertions(+), 1 deletions(-)
 create mode 100644 compat/mmap.c

applies-to: 274542bcbc891cca353c2728ac4075df3d1d2c0d
ed334e3e2276fe9d41ed78917544ef6a3fa87eb7
diff --git a/Makefile b/Makefile
index 1bdf4de..7ca77cf 100644
--- a/Makefile
+++ b/Makefile
@@ -27,6 +27,8 @@
 # Define NEEDS_SOCKET if linking with libc is not enough (SunOS,
 # Patrick Mauritz).
 #
+# Define NO_MMAP if you want to avoid mmap.
+#
 # Define WITH_OWN_SUBPROCESS_PY if you want to use with python 2.3.
 #
 # Define NO_IPV6 if you lack IPv6 support and getaddrinfo().
@@ -258,6 +260,10 @@ ifdef NO_STRCASESTR
 	DEFINES += -Dstrcasestr=gitstrcasestr
 	LIB_OBJS += compat/strcasestr.o
 endif
+ifdef NO_MMAP
+	DEFINES += -Dmmap=gitfakemmap -Dmunmap=gitfakemunmap -DNO_MMAP
+	LIB_OBJS += compat/mmap.o
+endif
 ifdef NO_IPV6
 	DEFINES += -DNO_IPV6 -Dsockaddr_storage=sockaddr_in
 endif
diff --git a/cache.h b/cache.h
index 514adb8..5987d4c 100644
--- a/cache.h
+++ b/cache.h
@@ -11,7 +11,9 @@
 #include &lt;string.h&gt;
 #include &lt;errno.h&gt;
 #include &lt;limits.h&gt;
+#ifndef NO_MMAP
 #include &lt;sys/mman.h&gt;
+#endif
 #include &lt;sys/param.h&gt;
 #include &lt;netinet/in.h&gt;
 #include &lt;sys/types.h&gt;
@@ -356,4 +358,18 @@ extern void packed_object_info_detail(st
 /* Dumb servers support */
 extern int update_server_info(int);
 
+#ifdef NO_MMAP
+
+#ifndef PROT_READ
+#define PROT_READ 1
+#define PROT_WRITE 2
+#define MAP_PRIVATE 1
+#define MAP_FAILED ((void*)...
To: Git Mailing List <git@...>
Date: Saturday, October 8, 2005 - 1:43 pm

Er, apologies for the dups - postfix crapped itself :/

*goes and stands in the corner donning the 'D' hat*

--=20
Elfyn McBratney
Gentoo Developer/Perl Team Lead
beu/irc.freenode.net                            http://dev.gentoo.org/~beu/
+------------O.o--------------------- http://dev.gentoo.org/~beu/pubkey.asc

PGP Key ID: 0x69DF17AD
PGP Key Fingerprint:
  DBD3 B756 ED58 B1B4 47B9  B3BD 8D41 E597 69DF 17AD
To: Git Mailing List <git@...>
Cc: Linus Torvalds <torvalds@...>, Alex Riesen <fork0@...>, Chuck Lever <cel@...>, Git Mailing List <git@...>, Junio C Hamano <junkio@...>, Christopher Faylor <me@...>, H. Peter Anvin <hpa@...>
Date: Saturday, October 8, 2005 - 1:38 pm

On Sat, Oct 08, 2005 at 09:11:03AM -0700, Linus Torvalds wrote:
 &gt;=20
 &gt; On Fri, 7 Oct 2005, Alex Riesen wrote:
 &gt; &gt;=20
 &gt; &gt; Make read_cache copy the index into memory, to improve portability on
 &gt; &gt; other OS's which have mmap too, tend to use it less commonly.
 &gt;=20
 &gt; I really think that you should just get rid of the mmap.
 &gt;=20
 &gt; As it is, you're just slowing the code down on sane architectures. That'=
s=20
 &gt; not good.
 &gt;=20
 &gt; So I'd suggest something like this instead.
 &gt;=20
 &gt; Totally untested, of course.
 &gt;=20
 &gt; 		Linus

Slightly adjusted diff below so it compiles ;)  (Note: only the second
die() un hunk #1 was changed.)

Best,
Elfyn

----
diff --git a/read-cache.c b/read-cache.c
--- a/read-cache.c
+++ b/read-cache.c
@@ -454,13 +454,39 @@ static int verify_hdr(struct cache_heade
 	return 0;
 }
=20
+static void *map_index_file(int fd, size_t size)
+{
+	void *map;
+#ifdef NO_MMAP
+	map =3D malloc(size);
+	if (!map)
+		die("Unable to allocate index file mapping");
+	if (read(fd, map, size) !=3D size)
+		die("Unable to read %z bytes from index", size);
+#else
+	map =3D mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
+	if (map =3D=3D MAP_FAILED)
+		die("index file mmap failed (%s)", strerror(errno));
+#endif
+	return map;
+}
+
+static void unmap_index_file(void *map, size_t size)
+{
+#ifdef NO_MMAP
+	free(map);
+#else
+	munmap(map, size);
+#endif
+}
+
 int read_cache(void)
 {
 	int fd, i;
 	struct stat st;
 	unsigned long size, offset;
-	void *map;
 	struct cache_header *hdr;
+	void *map;
=20
 	errno =3D EBUSY;
 	if (active_cache)
@@ -475,16 +501,15 @@ int read_cache(void)
 	}
=20
 	size =3D 0; // avoid gcc warning
-	map =3D MAP_FAILED;
-	if (!fstat(fd, &amp;st)) {
-		size =3D st.st_size;
-		errno =3D EINVAL;
-		if (size &gt;=3D sizeof(struct cache_header) + 20)
-			map =3D mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
-	}
+	if (fstat(fd, &am...
To: Git Mailing List <git@...>
Date: Wednesday, October 5, 2005 - 12:09 pm

I don't know how Cygwin implemented rename(), but if they used MoveFile() 
they broke the POSIX rename() since MoveFile() fails if destination 
already exists. They should have used MoveFileEx(MOVEFILE_REPLACE_EXISTING)
instead, to guarantee POSIX semantics. The symptoms you're experiencing 
make me think this might be the case (even if it is strange, since other 
Unix software would fail the same way).


- Davide


-
To: Davide Libenzi <davidel@...>, Git Mailing List <git@...>
Date: Wednesday, October 5, 2005 - 12:15 pm

Cygwin's rename is much more than just a simple wrapper around MoveFile
or MoveFileEx.  It tries hard to guarantee POSIX semantics within the
strictures imposed by Windows.
--
Christopher Faylor			spammer? -&gt;	aaaspam@sourceware.org
Cygwin Co-Project Leader				aaaspam@duffek.com
TimeSys, Inc.
-
To: Christopher Faylor <me@...>
Cc: Git Mailing List <git@...>
Date: Wednesday, October 5, 2005 - 1:29 pm

To: Christopher Faylor <me@...>
Cc: Davide Libenzi <davidel@...>, Git Mailing List <git@...>
Date: Wednesday, October 5, 2005 - 12:23 pm

Certainly, but presumably different versions of Win32 have different 
limitations, no?

	-hpa
-
To: Git Mailing List <git@...>
Date: Wednesday, October 5, 2005 - 12:28 pm

Yes, definitely.  That's why the Cygwin rename code (and unlink code for
that matter) is so headache-inducing.

FWIW, I just looked at the source and, AFAICT, if rename() is setting
EPERM on Windows NT/2000/XP, it should be the result of a failing
MoveFileEx (old, new, MOVEFILE_REPLACE_EXISTING).
--
Christopher Faylor			spammer? -&gt;	aaaspam@sourceware.org
Cygwin Co-Project Leader				aaaspam@duffek.com
TimeSys, Inc.
-
To: H. Peter Anvin <hpa@...>, Alex Riesen <raa.lkml@...>, Git Mailing List <git@...>
Date: Tuesday, October 4, 2005 - 11:16 pm

That's a relief.  Btw, AFAIK, strerror is working correctly under
Cygwin also.
--
Christopher Faylor			spammer? -&gt;	aaaspam@sourceware.org
Cygwin Co-Project Leader				aaaspam@duffek.com
TimeSys, Inc.
-
To: Christopher Faylor <me@...>
Cc: Alex Riesen <raa.lkml@...>, Git Mailing List <git@...>
Date: Wednesday, October 5, 2005 - 1:25 am

Now if we can only figure out why gitk is messed up...

	-hpa
-
To: Alex Riesen <raa.lkml@...>
Cc: Git Mailing List <git@...>
Date: Tuesday, October 4, 2005 - 10:06 am

In this case a better way would be to just add -liberty to all link 
lines if necessary, but I would expect the core cygwin code to do this.

	-hpa
-
To: H. Peter Anvin <hpa@...>, Alex Riesen <raa.lkml@...>, Git Mailing List <git@...>
Date: Tuesday, October 4, 2005 - 11:15 pm

AFAIK, cygwin has a working rename().  Many packages rely on it.

If rename() is not working then a bug report with a test case would be
appreciated.
--
Christopher Faylor			spammer? -&gt;	aaaspam@sourceware.org
Cygwin Co-Project Leader				aaaspam@duffek.com
TimeSys, Inc.
-
To: H. Peter Anvin <hpa@...>
Cc: Git Mailing List <git@...>
Date: Tuesday, October 4, 2005 - 9:06 am

I think I have to clarify: I copied the function (like in strcasestr
case) into compat/
-
To: H. Peter Anvin <hpa@...>
Cc: Git Mailing List <git@...>
Date: Thursday, September 29, 2005 - 4:46 am

Hi,


See my mail about rootless X11. I went about working around that 
particular Tk bug by specifying the dimensions of the panes explicitely. 
However, I was not especially happy with my workaround, since it did not 
reproduce the layout exactly after a restart. Maybe you can figure it out 
how to do that.

Ciao,
Dscho

-
To: Johannes Schindelin <Johannes.Schindelin@...>
Cc: Git Mailing List <git@...>
Date: Thursday, September 29, 2005 - 1:25 pm

It looks like this isn't a rootless *X* thing; it looks like the wish 
that is included with Cygwin actually opens native Win32 windows; even 
when run from inside a rooted X session it still opens an external 
window.  I also tried using the wish from the latest ActiveState 
distribution; it exhibits the same problem although with slightly 
different geometries.

	-hpa
-
To: Johannes Schindelin <Johannes.Schindelin@...>
Cc: Git Mailing List <git@...>
Date: Thursday, September 29, 2005 - 12:11 pm

Unlikely, since I'm a complete Tcl/Tk illiterate.

	-hpa
-
To: H. Peter Anvin <hpa@...>
Cc: Git Mailing List <git@...>
Date: Thursday, September 29, 2005 - 12:46 am

This is visible on OSX too, and someone's mentioned it's a Tk oddity

Getting a 404 on that. Doesn't show up on gitweb either. I guess I
have to wait...

Is there a way to get gitweb to "compare" branches using git-cherry? I
often look at branches via git-web and it's impossible to tell what
makes them unique...

cheers,


martin
-
To: Martin Langhoff <martin.langhoff@...>
Cc: Git Mailing List <git@...>
Date: Thursday, September 29, 2005 - 2:19 am

Well, it's up there now.

	-hpa
-
To: Martin Langhoff <martin.langhoff@...>
Cc: <git@...>
Date: Thursday, September 29, 2005 - 1:13 am

Now that you mention it, I felt that too.  Maybe git-show-branch
output could help somehow?

-
To: H. Peter Anvin <hpa@...>
Cc: <git@...>
Date: Thursday, September 29, 2005 - 12:30 am

I just sent out "The other side of Linus' symbolic refs" patch,
saying that Cygwin capable of doing symlink would probably made
it irrelevant.  But it may not be a waste after all, considering
what you said above.



-
To: Junio C Hamano <junkio@...>
Cc: <git@...>
Date: Thursday, September 29, 2005 - 1:07 am

After looking at it some more, what Samba does when talking to a host 
that doesn't support Unix extensions is that it simply resolves the 
symlink, in effect turning it into a hard link.  That might be all git 
needs.  The reverse still doesn't work, though.

	-hpa
-
Previous thread: Multi-head fetch proposal by Johannes Schindelin on Wednesday, September 28, 2005 - 8:25 pm. (2 messages)

Next thread: Edit log message after commit by Kevin Leung on Wednesday, September 28, 2005 - 10:28 pm. (9 messages)
speck-geostationary