Re: [PATCH] 0003 This patch is to allow 12 different OS's to compile and run git.
- From
Daniel Barkalow <barkalow@iabervon.org>
- Date
- Jun 7, 2008, 03:40 UTC
- Message-ID
- <alpine.LNX.1.00.0806062316270.19665@iabervon.org>
- In-Reply-To
- <Pine.LNX.4.64.0806062015440.18454@xenau.zenez.com>
On Fri, 6 Jun 2008, Boyd Lynn Gerber wrote:
Show 73 quoted lines
> On Fri, 6 Jun 2008, Junio C Hamano wrote: > > Boyd Lynn Gerber <gerberb@zenez.com> writes: > > I guess the patch text itself seems to be getting reasonable, and perhaps > > the next few rounds would be to fix the commit log message ;-) > > I have it the same, without all the >> as I first posted, but I agree. > that it needs some tweaking. > > > > diff --git a/Makefile b/Makefile > > > index cce5a6e..000bf1f 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -165,6 +165,11 @@ uname_P := $(shell sh -c 'uname -p 2>/dev/null || echo not') > > > # CFLAGS and LDFLAGS are for the users to override from the command line. > > > > > > CFLAGS = -g -O2 -Wall > > > +ifeq ($(uname_S),SCO_SV) > > > + ifeq ($(uname_R),3.2) > > > + CFLAGS = -O2 > > > + endif > > > +endif > > > > What makes SCO_SV so special that this platform specific tweak does not > > live in "Platform specific tweaks" section like others? > > > > CFLAGS is for the user to oerride from the command line, and I do not very > > much like any tweaks in Makefile. I'd suggest dropping this hunk. > > > > > @@ -564,6 +569,42 @@ endif > > > ifeq ($(uname_S),GNU/kFreeBSD) > > > NO_STRLCPY = YesPlease > > > endif > > > +ifeq ($(uname_S),UnixWare) > > > + CC=cc > > > > s/=/ = /; you have similar one elsewhere. > > I only have the one section now. I will look at it some more. > > > > + NEEDS_SOCKET = YesPlease > > > + NEEDS_NSL = YesPlease > > > + NEEDS_SSL_WITH_CRYPTO = YesPlease > > > + NEEDS_LIBICONV = YesPlease > > > + SHELL_PATH = /usr/local/bin/bash > > > + NO_IPV6 = YesPlease > > > + NO_HSTRERROR = YesPlease > > > + BASIC_CFLAGS += -Kalloca -Kthread > > > > I am only guessing what -Kalloca is, but is it for alloca(3), and if so do > > you still need it? > > I will make tests without it on the next run. It takes about 3 hours for > me to get the patches to all the system review them and then run > everything. Some of the machines are really slow. > > > > diff --git a/git-compat-util.h b/git-compat-util.h > > > index 01c4045..b3cd7b3 100644 > > > --- a/git-compat-util.h > > > +++ b/git-compat-util.h > > > @@ -39,7 +39,12 @@ > > > /* Approximation of the length of the decimal representation of this type. */ > > > #define decimal_length(x) ((int)(sizeof(x) * 2.56 + 0.5) + 1) > > > > > > -#if !defined(__APPLE__) && !defined(__FreeBSD__) > > > +/* Added for __USLC__ for any Novell devrived Compiler and Some Sys V > > > + Added _M_UNIX for any XENIX/SCO UNIX/OpenServer less than or equal > > > + OpenServer 5.0.7 This is do avoided compiler hell like the other > > > + OS's __APPLE__ and __FreeBSD__ */ > > > > We generally do not do changelog inside the code comment. > > Where do you put your change log stuff, to explain why you made the > change. Maybe it is a bit left over from doing things for MySQL AB.
In the commit message. That is, if your commit message goes:
Allow more systems to build git
__USLC__ indicates a Novell-derived compiler or some SysV __M_UNIX indicates XENIX/SCO UNIX/OpenServer before 5.0.7 Like Apple and BSD, both of these shouldn't have _XOPEN_SOURCE defined
then an interested user can use "git blame" on the file, and then read that info in the commit message for the commit that introduced those lines.
Also on the commit message: you should put the list of affected files after a line with just "---", so it appears in the patch but not in the commit; if we want to find out what the commit affects, we can use "git log --stat" and find out for sure. The explanation of the sign-off can probably go after the "---", too, although the "Signed-off-by" line should be above it.
-Daniel *This .sig left intentionally blank*