From: "H. Peter Anvin" <hpa@zytor.com>
To: Junio C Hamano <junkio@cox.net>
Cc: git@vger.kernel.org
Subject: Re: First cut at git port to Cygwin
Date: Fri, 30 Sep 2005 10:01:22 -0700 [thread overview]
Message-ID: <433D6F62.3030906@zytor.com> (raw)
In-Reply-To: <7v4q826ffy.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
>
> Could you do update-server-info there, please?
>
Done...
>
> 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.
> # 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)
>
> 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.
> 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.
> +#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.
> + *socklist_p = malloc(sizeof(int));
> + pfd = calloc(socknum, sizeof(struct pollfd));
>
> Please use xmalloc and xcalloc just for consistency.
Check.
> 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!
> 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"):
>
> diff --git a/git.sh b/git.sh
> --- a/git.sh
> +++ b/git.sh
> @@ -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]"
That wouldn't work, because the shell scripts don't get the .exe
extension. However, I can figure out something equivalent.
-hpa
next prev parent reply other threads:[~2005-09-30 17:01 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-09-29 0:53 First cut at git port to Cygwin H. Peter Anvin
2005-09-29 4:30 ` Junio C Hamano
2005-09-29 5:07 ` H. Peter Anvin
2005-09-29 4:46 ` Martin Langhoff
2005-09-29 5:13 ` Junio C Hamano
2005-09-29 6:19 ` H. Peter Anvin
2005-09-29 8:46 ` Johannes Schindelin
2005-09-29 16:11 ` H. Peter Anvin
2005-09-29 17:25 ` H. Peter Anvin
2005-09-30 10:02 ` Junio C Hamano
2005-09-30 17:01 ` H. Peter Anvin [this message]
2005-09-30 19:08 ` H. Peter Anvin
2005-10-04 12:31 ` Alex Riesen
2005-10-04 13:06 ` Alex Riesen
2005-10-04 14:06 ` H. Peter Anvin
2005-10-05 3:15 ` Christopher Faylor
2005-10-04 15:03 ` H. Peter Anvin
2005-10-05 3:16 ` Christopher Faylor
2005-10-05 5:25 ` H. Peter Anvin
2005-10-05 11:24 ` Alex Riesen
2005-10-05 15:46 ` Alex Riesen
2005-10-05 15:54 ` Christopher Faylor
2005-10-05 16:09 ` Davide Libenzi
2005-10-05 16:15 ` Christopher Faylor
2005-10-05 16:23 ` H. Peter Anvin
2005-10-05 16:28 ` Christopher Faylor
2005-10-05 17:29 ` Davide Libenzi
2005-10-05 19:17 ` Alex Riesen
2005-10-05 20:29 ` Christopher Faylor
2005-10-06 9:05 ` Alex Riesen
2005-10-06 10:07 ` Alex Riesen
2005-10-07 12:44 ` Alex Riesen
2005-10-07 15:34 ` Linus Torvalds
2005-10-07 20:54 ` Alex Riesen
2005-10-07 21:22 ` Alex Riesen
2005-10-07 21:29 ` Chuck Lever
2005-10-07 21:39 ` Alex Riesen
2005-10-08 16:11 ` Linus Torvalds
2005-10-08 17:38 ` Elfyn McBratney
2005-10-08 17:43 ` Elfyn McBratney
2005-10-08 18:27 ` Johannes Schindelin
2005-10-08 18:44 ` Junio C Hamano
2005-10-08 19:04 ` Johannes Schindelin
2005-10-08 21:10 ` Junio C Hamano
2005-10-08 22:06 ` Johannes Schindelin
2005-10-10 18:43 ` H. Peter Anvin
2005-10-10 19:01 ` Johannes Schindelin
2005-10-10 19:26 ` H. Peter Anvin
2005-10-10 19:42 ` Johannes Schindelin
2005-10-10 20:21 ` Junio C Hamano
2005-10-10 20:34 ` Junio C Hamano
2005-10-10 20:52 ` H. Peter Anvin
2005-10-10 20:27 ` Daniel Barkalow
2005-10-08 18:49 ` Alex Riesen
2005-10-09 20:40 ` Commit text BEFORE the dashes (Re: First cut at git port to Cygwin) Matthias Urlichs
[not found] ` <7vfyrdyre5.fsf@assigned-by-dhcp.cox.net>
2005-10-07 23:45 ` First cut at git port to Cygwin Alex Riesen
2005-10-08 1:00 ` Elfyn McBratney
2005-10-10 18:45 ` H. Peter Anvin
2005-10-05 13:16 ` Jonas Fonseca
2005-10-05 13:58 ` Johannes Schindelin
2005-10-05 15:52 ` [PATCH] Fix symbolic ref validation Jonas Fonseca
2005-10-05 16:54 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=433D6F62.3030906@zytor.com \
--to=hpa@zytor.com \
--cc=git@vger.kernel.org \
--cc=junkio@cox.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 NNTP newsgroup(s).