Re: [PATCH 2/2] build: don't duplicate substitution of make variables

Stefano Lattarini [off-list ref] writes:

Thanks to our 'GIT_CONF_SUBST' layer in configure.ac, a make variable 'VAR'
can be defined to a value 'VAL' at ./configure runtime in our build system
simply by using "GIT_CONF_SUBST([VAR], [VAL])" in configure.ac, rather than
having both to call "AC_SUBST([VAR], [VAL])" in configure.ac and adding the
'VAR = @VAR@' definition in config.mak.in.  Less duplication, less margin
for error, less possibility of confusion.

While at it, fix some formatting issues in configure.ac that unnecessarily
obscured the code flow.

Signed-off-by: Stefano Lattarini <redacted>
---
 config.mak.in |  49 --------------------
 configure.ac  | 144 +++++++++++++++++++++++++++++++---------------------------
 2 files changed, 76 insertions(+), 117 deletions(-)

Whoa ;-).

Is there some consistent policy regarding SP vs HT in the
indentation you are using in this patch?  These two hunks suggest
that you may be favoring spaces, but other places you seem to use
tabs, so...

On 09/11/2012 07:27 PM, Junio C Hamano wrote:

Stefano Lattarini [off-list ref] writes:

quoted

Thanks to our 'GIT_CONF_SUBST' layer in configure.ac, a make variable 'VAR'
can be defined to a value 'VAL' at ./configure runtime in our build system
simply by using "GIT_CONF_SUBST([VAR], [VAL])" in configure.ac, rather than
having both to call "AC_SUBST([VAR], [VAL])" in configure.ac and adding the
'VAR = @VAR@' definition in config.mak.in.  Less duplication, less margin
for error, less possibility of confusion.

While at it, fix some formatting issues in configure.ac that unnecessarily
obscured the code flow.

Signed-off-by: Stefano Lattarini <redacted>
---
 config.mak.in |  49 --------------------
 configure.ac  | 144 +++++++++++++++++++++++++++++++---------------------------
 2 files changed, 76 insertions(+), 117 deletions(-)

Whoa ;-).

Well, I could have converted one variable at the time, but that seemed
an overkill :-)

quoted

diff --git a/configure.ac b/configure.ac
index 450bbe7..da1f41f 100644
--- a/configure.ac
+++ b/configure.ac

@@ -267,6 +267,8 @@ AS_HELP_STRING([],           [ARG can be also prefix for libpcre library and hea
 	USE_LIBPCRE=YesPlease
 	LIBPCREDIR=$withval
 	AC_MSG_NOTICE([Setting LIBPCREDIR to $LIBPCREDIR])
+        dnl USE_LIBPCRE can still be modified below, so don't substitute
+        dnl it yet.
 	GIT_CONF_SUBST([LIBPCREDIR])
     fi)
 #

...
 AC_CHECK_FUNC([hstrerror],
-	[],
+    [],

Is there some consistent policy regarding SP vs HT in the
indentation you are using in this patch?

Basically I'm trying to follow the style of the surrounding code, while
keeping in mind that in the Git codebase tabs seem to be preferred to spaces.
In this case, the indentation of the following text (that was the "meat" of
the expression) seemed to favour 4 spaces for indentation, so I followed
suit.

These two hunks suggest
that you may be favoring spaces, but other places you seem to use
tabs, so...

I can convert the new tabs to spaces if you prefer (that would have been
my preference too, but thought trying to follow the "Git preferences"
was more important).  No big deal either way for me.

Thanks,
  Stefano