Re: [PATCH] 0003 This patch is to allow 12 different OS's to compile and run git.

From: Boyd Lynn Gerber <gerberb@zenez.com>
Date: Jun 7, 2008, 02:40 UTC
Message-ID: <Pine.LNX.4.64.0806062015440.18454@xenau.zenez.com>
In-Reply-To: <7vzlpyouwf.fsf@gitster.siamese.dyndns.org>

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.

Show 28 quoted lines

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

Show 11 quoted lines

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

Show 15 quoted lines

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

> > +#if !defined(__APPLE__) && !defined(__FreeBSD__)  && !defined(__USLC__) && !de
> > +fined(_M_UNIX)
> 
> Linewrapped patch?  It is very curious that you have + in front of "fined(_M_UNIX)".

It is not line wrapped in my file. I do the following after reverting every and strarting with a clean slate.

 cp -p .new/* . 
 git add Makefile git-compat-util.h progress.c
 git diff --cached
 git commit -s
 git format-patch -p -M -B origin

The last command above is creating the line wrapped because the line is so long. I do not know enough to force the tools to not do that line wrap.

-- Boyd Gerber <gerberb@zenez.com> ZENEZ 1042 East Fort Union #135, Midvale Utah 84047