Re: [PATCH 1/5] General configuration portability (Resend)

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 11, 2010, 16:51 UTC
Message-ID: <alpine.DEB.1.00.1003111750130.3748@intel-tinevez-2-302>
In-Reply-To: <20100311162750.GA7877@thor.il.thewrittenword.com>

Hi,

On Thu, 11 Mar 2010, Gary V. Vaughan wrote:

Show 24 quoted lines

> Resending some patches that didn't make it, in the correct format
> and taking into account the feedback I already had on one of the
> later patches...
> 
> This patch is in part to deal with peculiarities of our build
> environment (software packages are installed in their own directory
> tree, for example, and oftentimes we need to set additional -I options
> in CPPFLAGS, or -L options in LDFLAGS, among others).
> 
> However this is also where we put configuration changes to add
> additional tests, or fix Makefile problems so that everything builds
> on all of our supported architectures.  Probably, much of this patch
> is useful upstream.
> ---
>  Makefile             |   50 +++++++++++++++++++++++++++++++++++++++---------
>  aclocal.m4           |   41 ++++++++++++++++++++++++++++++++++++++++
>  config.mak.in        |   10 ++++++++-
>  configure.ac         |   51 ++++++++++++++++++++++++++++++++++++++++++-------
>  git-compat-util.h    |    8 +++++-
>  perl/Makefile        |    3 +-
>  perl/Makefile.PL     |    2 +-
>  t/t7610-mergetool.sh |   25 +++++++++++++----------
>  8 files changed, 156 insertions(+), 34 deletions(-)
>  create mode 100644 aclocal.m4

Whoa.

Pretty large patch, no? And looking at things like this:

Show 10 quoted lines

> diff --git a/Makefile b/Makefile
> index 8fdc421..6058805 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -243,7 +243,7 @@ endif
>  
>  CFLAGS = -g -O2 -Wall
>  LDFLAGS =
> -ALL_CFLAGS = $(CFLAGS)
> +ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)

It seems to me that this wants to be a relatively large patch series of well-contained, easy-to-review, tiny patches.

Am I correct? Dscho