[PATCH 1/2] build: improve GIT_CONF_SUBST signature

[PATCH 1/2] build: improve GIT_CONF_SUBST signature

[Stefano Lattarini]

Now, in configure.ac, a call like:

    GIT_CONF_SUBST([FOO])

will be considered equivalent to:

    GIT_CONF_SUBST([FOO], [$FOO])

This is mostly a preparatory refactoring in view of future changes.
No semantic change to the generated configure or config.mak.auto is
intended.

Signed-off-by: Stefano Lattarini <stefano.lattarini@gmail.com>
---
 configure.ac | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index df7e376..450bbe7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -7,8 +7,9 @@
 # ------------------------
 # Cause the line "VAR=VAL" to be eventually appended to ${config_file}.
 AC_DEFUN([GIT_CONF_SUBST],
-   [AC_REQUIRE([GIT_CONF_SUBST_INIT])
-   config_appended_defs="$config_appended_defs${newline}$1=$2"])
+[AC_REQUIRE([GIT_CONF_SUBST_INIT])
+config_appended_defs="$config_appended_defs${newline}dnl
+$1=m4_if([$#],[1],[${$1}],[$2])"])
 
 # GIT_CONF_SUBST_INIT
 # -------------------
@@ -179,7 +180,7 @@ AC_ARG_WITH([lib],
   else
 	lib=$withval
 	AC_MSG_NOTICE([Setting lib to '$lib'])
-	GIT_CONF_SUBST([lib], [$withval])
+	GIT_CONF_SUBST([lib])
   fi])
 
 if test -z "$lib"; then
@@ -215,7 +216,7 @@ AC_ARG_ENABLE([jsmin],
 [
   JSMIN=$enableval;
   AC_MSG_NOTICE([Setting JSMIN to '$JSMIN' to enable JavaScript minifying])
-  GIT_CONF_SUBST([JSMIN], [$enableval])
+  GIT_CONF_SUBST([JSMIN])
 ])
 
 # Define option to enable CSS minification
@@ -225,7 +226,7 @@ AC_ARG_ENABLE([cssmin],
 [
   CSSMIN=$enableval;
   AC_MSG_NOTICE([Setting CSSMIN to '$CSSMIN' to enable CSS minifying])
-  GIT_CONF_SUBST([CSSMIN], [$enableval])
+  GIT_CONF_SUBST([CSSMIN])
 ])
 
 ## Site configuration (override autodetection)
@@ -265,8 +266,8 @@ AS_HELP_STRING([],           [ARG can be also prefix for libpcre library and hea
     else
 	USE_LIBPCRE=YesPlease
 	LIBPCREDIR=$withval
-	AC_MSG_NOTICE([Setting LIBPCREDIR to $withval])
-	GIT_CONF_SUBST([LIBPCREDIR], [$withval])
+	AC_MSG_NOTICE([Setting LIBPCREDIR to $LIBPCREDIR])
+	GIT_CONF_SUBST([LIBPCREDIR])
     fi)
 #
 # Define NO_CURL if you do not have curl installed.  git-http-pull and
-- 
1.7.12.317.g1c54b74

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

[Stefano Lattarini]

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 <stefano.lattarini@gmail.com>
---
 config.mak.in |  49 --------------------
 configure.ac  | 144 +++++++++++++++++++++++++++++++---------------------------
 2 files changed, 76 insertions(+), 117 deletions(-)

diff --git a/config.mak.in b/config.mak.in
index 802d342..69d4838 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -5,12 +5,10 @@ CC = @CC@
 CFLAGS = @CFLAGS@
 CPPFLAGS = @CPPFLAGS@
 LDFLAGS = @LDFLAGS@
-CC_LD_DYNPATH = @CC_LD_DYNPATH@
 AR = @AR@
 TAR = @TAR@
 DIFF = @DIFF@
 #INSTALL = @INSTALL@		# needs install-sh or install.sh in sources
-TCLTK_PATH = @TCLTK_PATH@
 
 prefix = @prefix@
 exec_prefix = @exec_prefix@
@@ -27,50 +25,3 @@ VPATH = @srcdir@
 
 export exec_prefix mandir
 export srcdir VPATH
-
-NEEDS_SSL_WITH_CRYPTO=@NEEDS_SSL_WITH_CRYPTO@
-NO_OPENSSL=@NO_OPENSSL@
-NO_CURL=@NO_CURL@
-NO_EXPAT=@NO_EXPAT@
-NO_LIBGEN_H=@NO_LIBGEN_H@
-HAVE_PATHS_H=@HAVE_PATHS_H@
-HAVE_LIBCHARSET_H=@HAVE_LIBCHARSET_H@
-NO_GETTEXT=@NO_GETTEXT@
-LIBC_CONTAINS_LIBINTL=@LIBC_CONTAINS_LIBINTL@
-NEEDS_LIBICONV=@NEEDS_LIBICONV@
-NEEDS_SOCKET=@NEEDS_SOCKET@
-NEEDS_RESOLV=@NEEDS_RESOLV@
-NEEDS_LIBGEN=@NEEDS_LIBGEN@
-NO_SYS_SELECT_H=@NO_SYS_SELECT_H@
-NO_D_INO_IN_DIRENT=@NO_D_INO_IN_DIRENT@
-NO_D_TYPE_IN_DIRENT=@NO_D_TYPE_IN_DIRENT@
-NO_SOCKADDR_STORAGE=@NO_SOCKADDR_STORAGE@
-NO_IPV6=@NO_IPV6@
-NO_HSTRERROR=@NO_HSTRERROR@
-NO_STRCASESTR=@NO_STRCASESTR@
-NO_STRTOK_R=@NO_STRTOK_R@
-NO_FNMATCH=@NO_FNMATCH@
-NO_FNMATCH_CASEFOLD=@NO_FNMATCH_CASEFOLD@
-NO_MEMMEM=@NO_MEMMEM@
-NO_STRLCPY=@NO_STRLCPY@
-NO_UINTMAX_T=@NO_UINTMAX_T@
-NO_STRTOUMAX=@NO_STRTOUMAX@
-NO_SETENV=@NO_SETENV@
-NO_UNSETENV=@NO_UNSETENV@
-NO_MKDTEMP=@NO_MKDTEMP@
-NO_MKSTEMPS=@NO_MKSTEMPS@
-NO_INET_NTOP=@NO_INET_NTOP@
-NO_INET_PTON=@NO_INET_PTON@
-NO_ICONV=@NO_ICONV@
-OLD_ICONV=@OLD_ICONV@
-NO_REGEX=@NO_REGEX@
-USE_LIBPCRE=@USE_LIBPCRE@
-NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@
-INLINE=@INLINE@
-SOCKLEN_T=@SOCKLEN_T@
-FREAD_READS_DIRECTORIES=@FREAD_READS_DIRECTORIES@
-SNPRINTF_RETURNS_BOGUS=@SNPRINTF_RETURNS_BOGUS@
-NO_PTHREADS=@NO_PTHREADS@
-PTHREAD_CFLAGS=@PTHREAD_CFLAGS@
-PTHREAD_LIBS=@PTHREAD_LIBS@
-CHARSET_LIB=@CHARSET_LIB@
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)
 #
@@ -387,9 +389,10 @@ AC_MSG_NOTICE([CHECKS for programs])
 AC_PROG_CC([cc gcc])
 AC_C_INLINE
 case $ac_cv_c_inline in
-  inline | yes | no)	;;
-  *)			AC_SUBST([INLINE], [$ac_cv_c_inline]) ;;
+  inline | yes | no) INLINE='';;
+  *)                 INLINE=$ac_cv_c_inline ;;
 esac
+GIT_CONF_SUBST([INLINE])
 
 # which switch to pass runtime path to dynamic libraries to the linker
 AC_CACHE_CHECK([if linker supports -R], git_cv_ld_dashr, [
@@ -399,7 +402,7 @@ AC_CACHE_CHECK([if linker supports -R], git_cv_ld_dashr, [
    LDFLAGS="${SAVE_LDFLAGS}"
 ])
 if test "$git_cv_ld_dashr" = "yes"; then
-   AC_SUBST(CC_LD_DYNPATH, [-R])
+   CC_LD_DYNPATH=-R
 else
    AC_CACHE_CHECK([if linker supports -Wl,-rpath,], git_cv_ld_wl_rpath, [
       SAVE_LDFLAGS="${LDFLAGS}"
@@ -408,7 +411,7 @@ else
       LDFLAGS="${SAVE_LDFLAGS}"
    ])
    if test "$git_cv_ld_wl_rpath" = "yes"; then
-      AC_SUBST(CC_LD_DYNPATH, [-Wl,-rpath,])
+      CC_LD_DYNPATH=-Wl,-rpath
    else
       AC_CACHE_CHECK([if linker supports -rpath], git_cv_ld_rpath, [
          SAVE_LDFLAGS="${LDFLAGS}"
@@ -417,32 +420,35 @@ else
          LDFLAGS="${SAVE_LDFLAGS}"
       ])
       if test "$git_cv_ld_rpath" = "yes"; then
-         AC_SUBST(CC_LD_DYNPATH, [-rpath])
+         CC_LD_DYNPATH=-rpath
       else
+         CC_LD_DYNPATH=
          AC_MSG_WARN([linker does not support runtime path to dynamic libraries])
       fi
    fi
 fi
+GIT_CONF_SUBST([CC_LD_DYNPATH])
 #AC_PROG_INSTALL		# needs install-sh or install.sh in sources
 AC_CHECK_TOOLS(AR, [gar ar], :)
 AC_CHECK_PROGS(TAR, [gtar tar])
 AC_CHECK_PROGS(DIFF, [gnudiff gdiff diff])
 # TCLTK_PATH will be set to some value if we want Tcl/Tk
 # or will be empty otherwise.
-if test -z "$NO_TCLTK"; then
+if test -n "$NO_TCLTK"; then
+  TCLTK_PATH=
+else
   if test "$with_tcltk" = ""; then
   # No Tcl/Tk switches given. Do not check for Tcl/Tk, use bare 'wish'.
     TCLTK_PATH=wish
-    AC_SUBST(TCLTK_PATH)
   elif test "$with_tcltk" = "yes"; then
   # Tcl/Tk check requested.
     AC_CHECK_PROGS(TCLTK_PATH, [wish], )
   else
     AC_MSG_RESULT([Using Tcl/Tk interpreter $with_tcltk])
     TCLTK_PATH="$with_tcltk"
-    AC_SUBST(TCLTK_PATH)
   fi
 fi
+GIT_CONF_SUBST([TCLTK_PATH])
 AC_CHECK_PROGS(ASCIIDOC, [asciidoc])
 if test -n "$ASCIIDOC"; then
 	AC_MSG_CHECKING([for asciidoc version])
@@ -469,13 +475,13 @@ GIT_STASH_FLAGS($OPENSSLDIR)
 AC_CHECK_LIB([crypto], [SHA1_Init],
 [NEEDS_SSL_WITH_CRYPTO=],
 [AC_CHECK_LIB([ssl], [SHA1_Init],
- [NEEDS_SSL_WITH_CRYPTO=YesPlease],
- [NEEDS_SSL_WITH_CRYPTO= NO_OPENSSL=YesPlease])])
+ [NEEDS_SSL_WITH_CRYPTO=YesPlease NO_OPENSSL=],
+ [NEEDS_SSL_WITH_CRYPTO=          NO_OPENSSL=YesPlease])])
 
 GIT_UNSTASH_FLAGS($OPENSSLDIR)
 
-AC_SUBST(NEEDS_SSL_WITH_CRYPTO)
-AC_SUBST(NO_OPENSSL)
+GIT_CONF_SUBST([NEEDS_SSL_WITH_CRYPTO])
+GIT_CONF_SUBST([NO_OPENSSL])
 
 #
 # Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be
@@ -492,7 +498,7 @@ AC_CHECK_LIB([pcre], [pcre_version],
 
 GIT_UNSTASH_FLAGS($LIBPCREDIR)
 
-AC_SUBST(USE_LIBPCRE)
+GIT_CONF_SUBST([USE_LIBPCRE])
 
 fi
 
@@ -509,7 +515,7 @@ AC_CHECK_LIB([curl], [curl_global_init],
 
 GIT_UNSTASH_FLAGS($CURLDIR)
 
-AC_SUBST(NO_CURL)
+GIT_CONF_SUBST([NO_CURL])
 
 #
 # Define NO_EXPAT if you do not have expat installed.  git-http-push is
@@ -523,7 +529,7 @@ AC_CHECK_LIB([expat], [XML_ParserCreate],
 
 GIT_UNSTASH_FLAGS($EXPATDIR)
 
-AC_SUBST(NO_EXPAT)
+GIT_CONF_SUBST([NO_EXPAT])
 
 #
 # Define NEEDS_LIBICONV if linking with libc is not enough (Darwin and
@@ -569,8 +575,8 @@ LIBS="$old_LIBS"
 
 GIT_UNSTASH_FLAGS($ICONVDIR)
 
-AC_SUBST(NEEDS_LIBICONV)
-AC_SUBST(NO_ICONV)
+GIT_CONF_SUBST([NEEDS_LIBICONV])
+GIT_CONF_SUBST([NO_ICONV])
 
 if test -n "$NO_ICONV"; then
     NEEDS_LIBICONV=
@@ -597,7 +603,7 @@ LIBS="$old_LIBS"
 
 GIT_UNSTASH_FLAGS($ZLIB_PATH)
 
-AC_SUBST(NO_DEFLATE_BOUND)
+GIT_CONF_SUBST([NO_DEFLATE_BOUND])
 
 #
 # Define NEEDS_SOCKET if linking with libc is not enough (SunOS,
@@ -605,7 +611,7 @@ AC_SUBST(NO_DEFLATE_BOUND)
 AC_CHECK_LIB([c], [socket],
 [NEEDS_SOCKET=],
 [NEEDS_SOCKET=YesPlease])
-AC_SUBST(NEEDS_SOCKET)
+GIT_CONF_SUBST([NEEDS_SOCKET])
 test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket"
 
 #
@@ -613,40 +619,43 @@ test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket"
 # libresolv provides some of the functions we would normally get
 # from libc.
 NEEDS_RESOLV=
-AC_SUBST(NEEDS_RESOLV)
 #
 # Define NO_INET_NTOP if linking with -lresolv is not enough.
 # Solaris 2.7 in particular hos inet_ntop in -lresolv.
 NO_INET_NTOP=
-AC_SUBST(NO_INET_NTOP)
 AC_CHECK_FUNC([inet_ntop],
-	[],
+    [],
     [AC_CHECK_LIB([resolv], [inet_ntop],
-	    [NEEDS_RESOLV=YesPlease],
+	[NEEDS_RESOLV=YesPlease],
 	[NO_INET_NTOP=YesPlease])
 ])
+GIT_CONF_SUBST([NO_INET_NTOP])
 #
 # Define NO_INET_PTON if linking with -lresolv is not enough.
 # Solaris 2.7 in particular hos inet_pton in -lresolv.
 NO_INET_PTON=
-AC_SUBST(NO_INET_PTON)
 AC_CHECK_FUNC([inet_pton],
-	[],
+    [],
     [AC_CHECK_LIB([resolv], [inet_pton],
-	    [NEEDS_RESOLV=YesPlease],
+	[NEEDS_RESOLV=YesPlease],
 	[NO_INET_PTON=YesPlease])
 ])
+GIT_CONF_SUBST([NO_INET_PTON])
 #
 # Define NO_HSTRERROR if linking with -lresolv is not enough.
 # Solaris 2.6 in particular has no hstrerror, even in -lresolv.
 NO_HSTRERROR=
 AC_CHECK_FUNC([hstrerror],
-	[],
+    [],
     [AC_CHECK_LIB([resolv], [hstrerror],
-	    [NEEDS_RESOLV=YesPlease],
+	[NEEDS_RESOLV=YesPlease],
 	[NO_HSTRERROR=YesPlease])
 ])
-AC_SUBST(NO_HSTRERROR)
+GIT_CONF_SUBST([NO_HSTRERROR])
+
+dnl This must go after all the possible places for its initialization,
+dnl in the AC_CHECK_FUNC invocations above.
+GIT_CONF_SUBST([NEEDS_RESOLV])
 #
 # If any of the above tests determined that -lresolv is needed at
 # build-time, also set it here for remaining configure-time checks.
@@ -655,13 +664,13 @@ test -n "$NEEDS_RESOLV" && LIBS="$LIBS -lresolv"
 AC_CHECK_LIB([c], [basename],
 [NEEDS_LIBGEN=],
 [NEEDS_LIBGEN=YesPlease])
-AC_SUBST(NEEDS_LIBGEN)
+GIT_CONF_SUBST([NEEDS_LIBGEN])
 test -n "$NEEDS_LIBGEN" && LIBS="$LIBS -lgen"
 
 AC_CHECK_LIB([c], [gettext],
 [LIBC_CONTAINS_LIBINTL=YesPlease],
 [LIBC_CONTAINS_LIBINTL=])
-AC_SUBST(LIBC_CONTAINS_LIBINTL)
+GIT_CONF_SUBST([LIBC_CONTAINS_LIBINTL])
 
 #
 # Define NO_GETTEXT if you don't want Git output to be translated.
@@ -669,7 +678,7 @@ AC_SUBST(LIBC_CONTAINS_LIBINTL)
 AC_CHECK_HEADER([libintl.h],
 [NO_GETTEXT=],
 [NO_GETTEXT=YesPlease])
-AC_SUBST(NO_GETTEXT)
+GIT_CONF_SUBST([NO_GETTEXT])
 
 if test -z "$NO_GETTEXT"; then
     test -n "$LIBC_CONTAINS_LIBINTL" || LIBS="$LIBS -lintl"
@@ -682,19 +691,19 @@ AC_MSG_NOTICE([CHECKS for header files])
 AC_CHECK_HEADER([sys/select.h],
 [NO_SYS_SELECT_H=],
 [NO_SYS_SELECT_H=UnfortunatelyYes])
-AC_SUBST(NO_SYS_SELECT_H)
+GIT_CONF_SUBST([NO_SYS_SELECT_H])
 #
 # Define NO_SYS_POLL_H if you don't have sys/poll.h
 AC_CHECK_HEADER([sys/poll.h],
 [NO_SYS_POLL_H=],
 [NO_SYS_POLL_H=UnfortunatelyYes])
-AC_SUBST(NO_SYS_POLL_H)
+GIT_CONF_SUBST([NO_SYS_POLL_H])
 #
 # Define NO_INTTYPES_H if you don't have inttypes.h
 AC_CHECK_HEADER([inttypes.h],
 [NO_INTTYPES_H=],
 [NO_INTTYPES_H=UnfortunatelyYes])
-AC_SUBST(NO_INTTYPES_H)
+GIT_CONF_SUBST([NO_INTTYPES_H])
 #
 # Define OLD_ICONV if your library has an old iconv(), where the second
 # (input buffer pointer) parameter is declared with type (const char **).
@@ -717,23 +726,24 @@ AC_COMPILE_IFELSE([OLDICONVTEST_SRC],
 
 GIT_UNSTASH_FLAGS($ICONVDIR)
 
-AC_SUBST(OLD_ICONV)
+GIT_CONF_SUBST([OLD_ICONV])
 
 ## Checks for typedefs, structures, and compiler characteristics.
 AC_MSG_NOTICE([CHECKS for typedefs, structures, and compiler characteristics])
 #
 TYPE_SOCKLEN_T
 case $ac_cv_type_socklen_t in
-  yes)	;;
-  *)  	AC_SUBST([SOCKLEN_T], [$git_cv_socklen_t_equiv]) ;;
+  yes)	SOCKLEN_T='';;
+  *)  	SOCKLEN_T=$git_cv_socklen_t_equiv;;
 esac
+GIT_CONF_SUBST([SOCKLEN_T])
 
 # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
 AC_CHECK_MEMBER(struct dirent.d_ino,
 [NO_D_INO_IN_DIRENT=],
 [NO_D_INO_IN_DIRENT=YesPlease],
 [#include <dirent.h>])
-AC_SUBST(NO_D_INO_IN_DIRENT)
+GIT_CONF_SUBST([NO_D_INO_IN_DIRENT])
 #
 # Define NO_D_TYPE_IN_DIRENT if your platform defines DT_UNKNOWN but lacks
 # d_type in struct dirent (latest Cygwin -- will be fixed soonish).
@@ -741,7 +751,7 @@ AC_CHECK_MEMBER(struct dirent.d_type,
 [NO_D_TYPE_IN_DIRENT=],
 [NO_D_TYPE_IN_DIRENT=YesPlease],
 [#include <dirent.h>])
-AC_SUBST(NO_D_TYPE_IN_DIRENT)
+GIT_CONF_SUBST([NO_D_TYPE_IN_DIRENT])
 #
 # Define NO_SOCKADDR_STORAGE if your platform does not have struct
 # sockaddr_storage.
@@ -751,7 +761,7 @@ AC_CHECK_TYPE(struct sockaddr_storage,
 #include <sys/types.h>
 #include <sys/socket.h>
 ])
-AC_SUBST(NO_SOCKADDR_STORAGE)
+GIT_CONF_SUBST([NO_SOCKADDR_STORAGE])
 #
 # Define NO_IPV6 if you lack IPv6 support and getaddrinfo().
 AC_CHECK_TYPE([struct addrinfo],[
@@ -763,7 +773,7 @@ AC_CHECK_TYPE([struct addrinfo],[
 #include <sys/socket.h>
 #include <netdb.h>
 ])
-AC_SUBST(NO_IPV6)
+GIT_CONF_SUBST([NO_IPV6])
 #
 # Define NO_REGEX if you have no or inferior regex support in your C library.
 AC_CACHE_CHECK([whether the platform regex can handle null bytes],
@@ -784,7 +794,7 @@ if test $ac_cv_c_excellent_regex = yes; then
 else
 	NO_REGEX=YesPlease
 fi
-AC_SUBST(NO_REGEX)
+GIT_CONF_SUBST([NO_REGEX])
 #
 # Define FREAD_READS_DIRECTORIES if your are on a system which succeeds
 # when attempting to read from an fopen'ed directory.
@@ -804,7 +814,7 @@ if test $ac_cv_fread_reads_directories = yes; then
 else
 	FREAD_READS_DIRECTORIES=
 fi
-AC_SUBST(FREAD_READS_DIRECTORIES)
+GIT_CONF_SUBST([FREAD_READS_DIRECTORIES])
 #
 # Define SNPRINTF_RETURNS_BOGUS if your are on a system which snprintf()
 # or vsnprintf() return -1 instead of number of characters which would
@@ -838,7 +848,7 @@ if test $ac_cv_snprintf_returns_bogus = yes; then
 else
 	SNPRINTF_RETURNS_BOGUS=
 fi
-AC_SUBST(SNPRINTF_RETURNS_BOGUS)
+GIT_CONF_SUBST([SNPRINTF_RETURNS_BOGUS])
 
 
 ## Checks for library functions.
@@ -849,47 +859,45 @@ AC_MSG_NOTICE([CHECKS for library functions])
 AC_CHECK_HEADER([libgen.h],
 [NO_LIBGEN_H=],
 [NO_LIBGEN_H=YesPlease])
-AC_SUBST(NO_LIBGEN_H)
+GIT_CONF_SUBST([NO_LIBGEN_H])
 #
 # Define HAVE_PATHS_H if you have paths.h.
 AC_CHECK_HEADER([paths.h],
 [HAVE_PATHS_H=YesPlease],
 [HAVE_PATHS_H=])
-AC_SUBST(HAVE_PATHS_H)
+GIT_CONF_SUBST([HAVE_PATHS_H])
 #
 # Define HAVE_LIBCHARSET_H if have libcharset.h
 AC_CHECK_HEADER([libcharset.h],
 [HAVE_LIBCHARSET_H=YesPlease],
 [HAVE_LIBCHARSET_H=])
-AC_SUBST(HAVE_LIBCHARSET_H)
+GIT_CONF_SUBST([HAVE_LIBCHARSET_H])
 # Define CHARSET_LIB if libiconv does not export the locale_charset symbol
 # and libcharset does
 CHARSET_LIB=
 AC_CHECK_LIB([iconv], [locale_charset],
        [],
        [AC_CHECK_LIB([charset], [locale_charset],
-                     [CHARSET_LIB=-lcharset])
-       ]
-)
-AC_SUBST(CHARSET_LIB)
+                     [CHARSET_LIB=-lcharset])])
+GIT_CONF_SUBST([CHARSET_LIB])
 #
 # Define NO_STRCASESTR if you don't have strcasestr.
 GIT_CHECK_FUNC(strcasestr,
 [NO_STRCASESTR=],
 [NO_STRCASESTR=YesPlease])
-AC_SUBST(NO_STRCASESTR)
+GIT_CONF_SUBST([NO_STRCASESTR])
 #
 # Define NO_STRTOK_R if you don't have strtok_r
 GIT_CHECK_FUNC(strtok_r,
 [NO_STRTOK_R=],
 [NO_STRTOK_R=YesPlease])
-AC_SUBST(NO_STRTOK_R)
+GIT_CONF_SUBST([NO_STRTOK_R])
 #
 # Define NO_FNMATCH if you don't have fnmatch
 GIT_CHECK_FUNC(fnmatch,
 [NO_FNMATCH=],
 [NO_FNMATCH=YesPlease])
-AC_SUBST(NO_FNMATCH)
+GIT_CONF_SUBST([NO_FNMATCH])
 #
 # Define NO_FNMATCH_CASEFOLD if your fnmatch function doesn't have the
 # FNM_CASEFOLD GNU extension.
@@ -911,19 +919,19 @@ if test $ac_cv_c_excellent_fnmatch = yes; then
 else
 	NO_FNMATCH_CASEFOLD=YesPlease
 fi
-AC_SUBST(NO_FNMATCH_CASEFOLD)
+GIT_CONF_SUBST([NO_FNMATCH_CASEFOLD])
 #
 # Define NO_MEMMEM if you don't have memmem.
 GIT_CHECK_FUNC(memmem,
 [NO_MEMMEM=],
 [NO_MEMMEM=YesPlease])
-AC_SUBST(NO_MEMMEM)
+GIT_CONF_SUBST([NO_MEMMEM])
 #
 # Define NO_STRLCPY if you don't have strlcpy.
 GIT_CHECK_FUNC(strlcpy,
 [NO_STRLCPY=],
 [NO_STRLCPY=YesPlease])
-AC_SUBST(NO_STRLCPY)
+GIT_CONF_SUBST([NO_STRLCPY])
 #
 # Define NO_UINTMAX_T if your platform does not have uintmax_t
 AC_CHECK_TYPE(uintmax_t,
@@ -931,43 +939,43 @@ AC_CHECK_TYPE(uintmax_t,
 [NO_UINTMAX_T=YesPlease],[
 #include <inttypes.h>
 ])
-AC_SUBST(NO_UINTMAX_T)
+GIT_CONF_SUBST([NO_UINTMAX_T])
 #
 # Define NO_STRTOUMAX if you don't have strtoumax in the C library.
 GIT_CHECK_FUNC(strtoumax,
 [NO_STRTOUMAX=],
 [NO_STRTOUMAX=YesPlease])
-AC_SUBST(NO_STRTOUMAX)
+GIT_CONF_SUBST([NO_STRTOUMAX])
 #
 # Define NO_SETENV if you don't have setenv in the C library.
 GIT_CHECK_FUNC(setenv,
 [NO_SETENV=],
 [NO_SETENV=YesPlease])
-AC_SUBST(NO_SETENV)
+GIT_CONF_SUBST([NO_SETENV])
 #
 # Define NO_UNSETENV if you don't have unsetenv in the C library.
 GIT_CHECK_FUNC(unsetenv,
 [NO_UNSETENV=],
 [NO_UNSETENV=YesPlease])
-AC_SUBST(NO_UNSETENV)
+GIT_CONF_SUBST([NO_UNSETENV])
 #
 # Define NO_MKDTEMP if you don't have mkdtemp in the C library.
 GIT_CHECK_FUNC(mkdtemp,
 [NO_MKDTEMP=],
 [NO_MKDTEMP=YesPlease])
-AC_SUBST(NO_MKDTEMP)
+GIT_CONF_SUBST([NO_MKDTEMP])
 #
 # Define NO_MKSTEMPS if you don't have mkstemps in the C library.
 GIT_CHECK_FUNC(mkstemps,
 [NO_MKSTEMPS=],
 [NO_MKSTEMPS=YesPlease])
-AC_SUBST(NO_MKSTEMPS)
+GIT_CONF_SUBST([NO_MKSTEMPS])
 #
 # Define NO_INITGROUPS if you don't have initgroups in the C library.
 GIT_CHECK_FUNC(initgroups,
 [NO_INITGROUPS=],
 [NO_INITGROUPS=YesPlease])
-AC_SUBST(NO_INITGROUPS)
+GIT_CONF_SUBST([NO_INITGROUPS])
 #
 #
 # Define NO_MMAP if you want to avoid mmap.
@@ -1049,9 +1057,9 @@ fi
 
 CFLAGS="$old_CFLAGS"
 
-AC_SUBST(PTHREAD_CFLAGS)
-AC_SUBST(PTHREAD_LIBS)
-AC_SUBST(NO_PTHREADS)
+GIT_CONF_SUBST([PTHREAD_CFLAGS])
+GIT_CONF_SUBST([PTHREAD_LIBS])
+GIT_CONF_SUBST([NO_PTHREADS])
 
 ## Output files
 AC_CONFIG_FILES(["${config_file}":"${config_in}"])
-- 
1.7.12.317.g1c54b74

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

[Junio C Hamano]

Stefano Lattarini <stefano.lattarini@gmail.com> 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 <stefano.lattarini@gmail.com>
> ---
>  config.mak.in |  49 --------------------
>  configure.ac  | 144 +++++++++++++++++++++++++++++++---------------------------
>  2 files changed, 76 insertions(+), 117 deletions(-)

Whoa ;-).

> 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?  These two hunks suggest
that you may be favoring spaces, but other places you seem to use
tabs, so...

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

[Stefano Lattarini]

On 09/11/2012 07:27 PM, Junio C Hamano wrote:
> Stefano Lattarini <stefano.lattarini@gmail.com> 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 <stefano.lattarini@gmail.com>
>> ---
>>  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 :-)

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

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

[Junio C Hamano]

Stefano Lattarini <stefano.lattarini@gmail.com> writes:

> On 09/11/2012 07:27 PM, Junio C Hamano wrote:
>> Stefano Lattarini <stefano.lattarini@gmail.com> 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 <stefano.lattarini@gmail.com>
>>> ---
>>>  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 :-)

No, I was happy to see many lines go ;-)
>> 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.

If this were other parts of the system, my preference would be to
use tabs, but because I do not help very much in the autoconf part
myself, I do not have a particular preference.  If it is more common
to indent the configure.ac script with spaces, that would be more
familiar to the folks who work on it, and I do not have much against
choosing and sticking to space indented configure.ac file if that is
the policy.

But if this patch is not about cleaning up the style to make it
conform to a policy (whichever it is), I would have preferred to see
a clean-up patch as a separate step, not mixed together with this.

That's all; either way, no big deal.

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

[Stefano Lattarini]

On 09/11/2012 09:52 PM, Junio C Hamano wrote:
> Stefano Lattarini <stefano.lattarini@gmail.com> writes:
> 
>> On 09/11/2012 07:27 PM, Junio C Hamano wrote:
>>
>>> 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.
> 
> If this were other parts of the system, my preference would be to
> use tabs, but because I do not help very much in the autoconf part
> myself, I do not have a particular preference.  If it is more common
> to indent the configure.ac script with spaces,
>
There is no general standard about it that I know of; it's just that
GNU projects tend to prefer space-based indentation over tab-based one,
and since I mostly touch configure.ac files from GNU projects, I've
picked up the habit.

> that would be more
> familiar to the folks who work on it, and I do not have much against
> choosing and sticking to space indented configure.ac file if that is
> the policy.
> 
Then I might send a patch that normalize indentation in configure.ac
to "spaces only", if that's OK with you.  But that's obviously for a
separated thread.

> But if this patch is not about cleaning up the style to make it
> conform to a policy (whichever it is), I would have preferred to see
> a clean-up patch as a separate step, not mixed together with this.
>
The reason those few clean-ups are mixed into this patch is that the
pre-existing strange indentation style was actually making it more
difficult for me to grasp the code flow; that is, I didn't see them
as a cosmetic change, but as a way to make it easier for me and the
reader to see that my changes were correct and sensible.

> That's all; either way, no big deal.
> 
OK.  Just let me know if you'd still prefer to have the indentation
cleanups done by a preparatory patch, and I'll send a re-roll.

Thanks,
  Stefano