First cut at git port to Cygwin

PATCH In review H. Peter Anvin 14 participants 62 messages Jun 15, 2016 1 message not in the archive Kbuild

Share & tools

H. Peter Anvin

author

2016-06-15 22:42 UTC #0 lore raw

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

Martin Langhoff

2016-06-15 22:42 UTC #1 lore raw

Quoted 4 lines · H. Peter Anvin

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

This is visible on OSX too, and someone's mentioned it's a Tk oddity with rootless X. Does it do the same if you run X with a root window?

Quoted 3 lines

I have set up a git-on-Cygwin temporary tree at:
http://www.kernel.org/pub/scm/git/git-cygwin.git

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

H. Peter Anvin

author

2016-06-15 22:42 UTC #2 lore raw

Quoted 8 lines · Martin Langhoff

Quoted 4 lines
I have set up a git-on-Cygwin temporary tree at:
http://www.kernel.org/pub/scm/git/git-cygwin.git
Getting a 404 on that. Doesn't show up on gitweb either. I guess I have to wait...

Well, it's up there now.

-hpa

Junio C Hamano

2016-06-15 22:42 UTC #3 lore raw

In reply to Martin Langhoff

Quoted 3 lines · Martin Langhoff

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...

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

Junio C Hamano

2016-06-15 22:42 UTC #4 lore raw

In reply to H. Peter Anvin

Quoted 3 lines · H. Peter Anvin

I have set up a git-on-Cygwin temporary tree at:
http://www.kernel.org/pub/scm/git/git-cygwin.git

: siamese; git clone http://kernel.org/pub/scm/git/git-cygwin.git/ git-cygwin defaulting to local storage area
Cannot get remote repository information.
Perhaps git-update-server-info needs to be run there?

Could you do update-server-info there, please?

hera$ cd /pub/scm/git/git-cygwin.git
hera$ GIT_DIR=. git-update-server-info

Knowing nothing about Cygwin environment, here are some comments.

+# Define NO_IPV6 if you lack IPv6 support and getaddrinfo().

This part probably is applicable outside Cygwin. At some point, can we have it in the mainline please?

         # The ones that do not have to link with lcrypto nor lz.
         SIMPLE_PROGRAMS = \
        -	git-get-tar-commit-id git-mailinfo git-mailsplit git-stripspace \
        -	git-daemon git-var
        +	git-get-tar-commit-id$(X) git-mailinfo$(X) git-mailsplit$(X) \
        +	git-stripspace$(X) git-var$(X) git-daemon$(X)

I have seen these $(X) in other programs' ports and found them quite distasteful. Since I do not have immediate suggestions for improvements, I do not have rights to complain, though.

Spelling it $X is a bit less distracting but not that much better. Maybe "SIMPLE_PROGRAM_NAMES = git-foo git-bar" and "SIMPLE_PROGRAMS = $(patsubst %,%$X,$(SIMPLE_PROGRAM_NAMES))"... but that would not help bits like this:

        -	PROGRAMS += git-http-fetch
        +	PROGRAMS += git-http-fetch$(X)

or this:

        -git-%: %.o $(LIB_FILE)
        +git-%$(X): %.o $(LIB_FILE)

... so I'd shut up about this part.

        diff --git a/daemon.c b/daemon.c
        --- a/daemon.c
        +++ b/daemon.c
        @@ -1,9 +1,11 @@
         #include "cache.h"
         #include "pkt-line.h"
        +#include <alloca.h>

Why? I do not see any use of alloca in the added code...

        +#include <sys/poll.h>

Is poll preferrable over select in general? Some may have only select available and others may have only poll available, perhaps? In any case, this is probably relevant to wider audience than just Cygwin; please give it to mainline at some point, perhaps conditionally allowing either/both.

        +	*socklist_p = malloc(sizeof(int));
        +	pfd = calloc(socknum, sizeof(struct pollfd));

Please use xmalloc and xcalloc just for consistency.

                test -x $path/git-$cmd && exec $path/git-$cmd "$@" ;;
        +
        +	# In case we're running on Cygwin...
        +	test -x $path/git-$cmd.exe && exec $path/git-$cmd.exe "$@" ;;
         esac

Hmph, I think you forgot to drop double semicolon there.

The git.sh script is munged by Makefile so presumably we could fix this part up there, like:

        git: git.sh Makefile
                rm -f $@+ $@
                sed -e '1s|#!.*/sh|#!$(SHELL_PATH)|' \
                    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
                    -e 's/@@X@@/$X/g' <[email protected] >$@+
                chmod +x $@+
                mv $@+ $@

And then (a patch on top of your "master"):

git.sh +7 -3

@@ -12,10 +12,14 @@ case "$#" in
 		exit 0 ;;
 	esac
 
-	test -x $path/git-$cmd && exec $path/git-$cmd "$@" ;;
+	test -x $path/git-$cmd && exec $path/git-$cmd "$@"
 
-	# In case we're running on Cygwin...
-	test -x $path/git-$cmd.exe && exec $path/git-$cmd.exe "$@" ;;
+	case '@@X@@' in
+	'')
+		;;
+	*)
+		test -x $path/git-$cmd@@X@@ && exec $path/git-$cmd@@X@@ "$@" ;;
+	esac		
 esac
 
 echo "Usage: git COMMAND [OPTIONS] [TARGET]"

H. Peter Anvin

author

2016-06-15 22:42 UTC #5 lore raw

Quoted 3 lines · Junio C Hamano

Could you do update-server-info there, please?

Done...

Quoted 9 lines

Knowing nothing about Cygwin environment, here are some comments.
+# Define NO_IPV6 if you lack IPv6 support and getaddrinfo().
This part probably is applicable outside Cygwin. At some point, can we have it in the mainline please?

Well, I would hope that all the changes could eventually be merged.

Quoted 19 lines

         # The ones that do not have to link with lcrypto nor lz.
         SIMPLE_PROGRAMS = \
        -	git-get-tar-commit-id git-mailinfo git-mailsplit git-stripspace \
        -	git-daemon git-var
        +	git-get-tar-commit-id$(X) git-mailinfo$(X) git-mailsplit$(X) \
        +	git-stripspace$(X) git-var$(X) git-daemon$(X)
I have seen these $(X) in other programs' ports and found them quite distasteful. Since I not have immediate suggestions for improvements, I do not have rights to complain, though.
Spelling it $X is a bit less distracting but not that much better. Maybe "SIMPLE_PROGRAM_NAMES = git-foo git-bar" and "SIMPLE_PROGRAMS = $(patsubst %,%$X,$(SIMPLE_PROGRAM_NAMES))"... but that would not help bits like this:
        -	PROGRAMS += git-http-fetch
        +	PROGRAMS += git-http-fetch$(X)

Quoted 6 lines

or this:
        -git-%: %.o $(LIB_FILE)
        +git-%$(X): %.o $(LIB_FILE)
... so I'd shut up about this part.

My first cut had PROGRAMS_X and SIMPLE_PROGRAMS_X being patsubst of the original versions, but in the end I decided it was even uglier, because these patterns were needed elsewhere. I'll change them to $X except where the parens are needed.

Quoted 9 lines

        diff --git a/daemon.c b/daemon.c
        --- a/daemon.c
        +++ b/daemon.c
        @@ -1,9 +1,11 @@
         #include "cache.h"
         #include "pkt-line.h"
        +#include <alloca.h>

Why? I do not see any use of alloca in the added code...

I originally used alloca() before changing my mind and using calloc(); I think there might be platforms without alloca out there.

Quoted 7 lines

        +#include <sys/poll.h>
Is poll preferrable over select in general? Some may have only select available and others may have only poll available, perhaps? In any case, this is probably relevant to wider audience than just Cygwin; please give it to mainline at some point, perhaps conditionally allowing either/both.

The main reason I switched to poll() is that I believe all platforms that are even remotely relevant have both these days, and forming a poll list is so much cleaner than forming a select set. What makes forming a select set even remotely bearable is the invalid assumption that the number of file descriptors is bounded at compile time and therefore that fdset_t can be statically allocated. We've had problems in the past with that assumption on Linux, and I've tried to avoid select since then.

Quoted 4 lines

        +	*socklist_p = malloc(sizeof(int));
        +	pfd = calloc(socknum, sizeof(struct pollfd));
Please use xmalloc and xcalloc just for consistency.

Check.

Quoted 7 lines

                test -x $path/git-$cmd && exec $path/git-$cmd "$@" ;;
        +
        +	# In case we're running on Cygwin...
        +	test -x $path/git-$cmd.exe && exec $path/git-$cmd.exe "$@" ;;
         esac

Hmph, I think you forgot to drop double semicolon there.

D'oh!

Quoted 24 lines

The git.sh script is munged by Makefile so presumably we could fix this part up there, like:

        git: git.sh Makefile
                rm -f $@+ $@
                sed -e '1s|#!.*/sh|#!$(SHELL_PATH)|' \
                    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
                    -e 's/@@X@@/$X/g' <[email protected] >$@+
                chmod +x $@+
                mv $@+ $@

And then (a patch on top of your "master"):

git.sh +1 -2

@@ -12,10 +12,14 @@ case "$#" in
 		exit 0 ;;
 	esac
 
-	test -x $path/git-$cmd && exec $path/git-$cmd "$@" ;;
+	test -x $path/git-$cmd && exec $path/git-$cmd "$@"
 
-	# In case we're running on Cygwin...

git.sh case "$#" in · line 25

Quoted 10 lines

git.sh +6 -1

-	test -x $path/git-$cmd.exe && exec $path/git-$cmd.exe "$@" ;;
+	case '@@X@@' in
+	'')
+		;;
+	*)
+		test -x $path/git-$cmd@@X@@ && exec $path/git-$cmd@@X@@ "$@" ;;
+	esac		
 esac
 
 echo "Usage: git COMMAND [OPTIONS] [TARGET]"

That wouldn't work, because the shell scripts don't get the .exe extension. However, I can figure out something equivalent.

-hpa

H. Peter Anvin

author

2016-06-15 22:42 UTC #6 lore raw

Okay, I have updated the git-cygwin repository with the changes proposed, and they seem to work.

-hpa

Junio C Hamano

2016-06-15 22:42 UTC #7 lore raw

In reply to H. Peter Anvin

Quoted 3 lines · H. Peter Anvin

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.

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.

H. Peter Anvin

author

2016-06-15 22:42 UTC #8 lore raw

Quoted 11 lines · Junio C Hamano

Quoted 1 line · H. Peter Anvin
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.
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.

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

Johannes Schindelin

2016-06-15 22:42 UTC #9 lore raw

In reply to H. Peter Anvin

Hi,

Quoted 2 lines · H. Peter Anvin

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.

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

H. Peter Anvin

author

2016-06-15 22:42 UTC #10 lore raw

Quoted 9 lines · Johannes Schindelin

Hi,
Quoted 2 lines · H. Peter Anvin
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.
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.

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

-hpa

H. Peter Anvin

author

2016-06-15 22:42 UTC #11 lore raw

In reply to Johannes Schindelin

Quoted 9 lines · Johannes Schindelin

Hi,
Quoted 2 lines · H. Peter Anvin
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.
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.

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

Jonas Fonseca

2016-06-15 22:42 UTC #12 lore raw

In reply to H. Peter Anvin

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 ...