Re: First cut at git port to Cygwin

"H. Peter Anvin" <hpa@zytor.com> writes:

: 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' <$@.sh >$@+
                chmod +x $@+
                mv $@+ $@

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