{"thread":{"id":"31505","subject":"[PATCH 1/2] build: improve GIT_CONF_SUBST signature","startedAt":"2012-09-11T15:45:29Z","lastAt":"2012-09-11T20:17:55Z","messageCount":6,"participants":["Stefano Lattarini","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"198787","messageId":"1be62f9a7bbe728e6422e668d982ddf313d016eb.1347378209.git.stefano.lattarini@gmail.com","threadId":"31505","inReplyTo":null,"subject":"[PATCH 1/2] build: improve GIT_CONF_SUBST signature","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-09-11T15:45:29Z","receivedAt":"2012-09-11T15:45:29Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"Now, in configure.ac, a call like:\n\n    GIT_CONF_SUBST([FOO])\n\nwill be considered equivalent to:\n\n    GIT_CONF_SUBST([FOO], [$FOO])\n\nThis is mostly a preparatory refactoring in view of future changes.\nNo semantic change to the generated configure or config.mak.auto is\nintended.\n\nSigned-off-by: Stefano Lattarini <stefano.lattarini@gmail.com>\n---\n configure.ac | 15 ++++++++-------\n 1 file changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/configure.ac b/configure.ac\nindex df7e376..450bbe7 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -7,8 +7,9 @@\n # ------------------------\n # Cause the line \"VAR=VAL\" to be eventually appended to ${config_file}.\n AC_DEFUN([GIT_CONF_SUBST],\n-   [AC_REQUIRE([GIT_CONF_SUBST_INIT])\n-   config_appended_defs=\"$config_appended_defs${newline}$1=$2\"])\n+[AC_REQUIRE([GIT_CONF_SUBST_INIT])\n+config_appended_defs=\"$config_appended_defs${newline}dnl\n+$1=m4_if([$#],[1],[${$1}],[$2])\"])\n \n # GIT_CONF_SUBST_INIT\n # -------------------\n@@ -179,7 +180,7 @@ AC_ARG_WITH([lib],\n   else\n \tlib=$withval\n \tAC_MSG_NOTICE([Setting lib to '$lib'])\n-\tGIT_CONF_SUBST([lib], [$withval])\n+\tGIT_CONF_SUBST([lib])\n   fi])\n \n if test -z \"$lib\"; then\n@@ -215,7 +216,7 @@ AC_ARG_ENABLE([jsmin],\n [\n   JSMIN=$enableval;\n   AC_MSG_NOTICE([Setting JSMIN to '$JSMIN' to enable JavaScript minifying])\n-  GIT_CONF_SUBST([JSMIN], [$enableval])\n+  GIT_CONF_SUBST([JSMIN])\n ])\n \n # Define option to enable CSS minification\n@@ -225,7 +226,7 @@ AC_ARG_ENABLE([cssmin],\n [\n   CSSMIN=$enableval;\n   AC_MSG_NOTICE([Setting CSSMIN to '$CSSMIN' to enable CSS minifying])\n-  GIT_CONF_SUBST([CSSMIN], [$enableval])\n+  GIT_CONF_SUBST([CSSMIN])\n ])\n \n ## Site configuration (override autodetection)\n@@ -265,8 +266,8 @@ AS_HELP_STRING([],           [ARG can be also prefix for libpcre library and hea\n     else\n \tUSE_LIBPCRE=YesPlease\n \tLIBPCREDIR=$withval\n-\tAC_MSG_NOTICE([Setting LIBPCREDIR to $withval])\n-\tGIT_CONF_SUBST([LIBPCREDIR], [$withval])\n+\tAC_MSG_NOTICE([Setting LIBPCREDIR to $LIBPCREDIR])\n+\tGIT_CONF_SUBST([LIBPCREDIR])\n     fi)\n #\n # Define NO_CURL if you do not have curl installed.  git-http-pull and\n-- \n1.7.12.317.g1c54b74\n"},{"id":"198788","messageId":"1c54b744c0ec6987f7987a41853ab0ae00513d03.1347378210.git.stefano.lattarini@gmail.com","threadId":"31505","inReplyTo":"1be62f9a7bbe728e6422e668d982ddf313d016eb.1347378209.git.stefano.lattarini@gmail.com","subject":"[PATCH 2/2] build: don't duplicate substitution of make variables","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-09-11T15:45:30Z","receivedAt":"2012-09-11T15:45:30Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"Thanks to our 'GIT_CONF_SUBST' layer in configure.ac, a make variable 'VAR'\ncan be defined to a value 'VAL' at ./configure runtime in our build system\nsimply by using \"GIT_CONF_SUBST([VAR], [VAL])\" in configure.ac, rather than\nhaving both to call \"AC_SUBST([VAR], [VAL])\" in configure.ac and adding the\n'VAR = @VAR@' definition in config.mak.in.  Less duplication, less margin\nfor error, less possibility of confusion.\n\nWhile at it, fix some formatting issues in configure.ac that unnecessarily\nobscured the code flow.\n\nSigned-off-by: Stefano Lattarini <stefano.lattarini@gmail.com>\n---\n config.mak.in |  49 --------------------\n configure.ac  | 144 +++++++++++++++++++++++++++++++---------------------------\n 2 files changed, 76 insertions(+), 117 deletions(-)\n\ndiff --git a/config.mak.in b/config.mak.in\nindex 802d342..69d4838 100644\n--- a/config.mak.in\n+++ b/config.mak.in\n@@ -5,12 +5,10 @@ CC = @CC@\n CFLAGS = @CFLAGS@\n CPPFLAGS = @CPPFLAGS@\n LDFLAGS = @LDFLAGS@\n-CC_LD_DYNPATH = @CC_LD_DYNPATH@\n AR = @AR@\n TAR = @TAR@\n DIFF = @DIFF@\n #INSTALL = @INSTALL@\t\t# needs install-sh or install.sh in sources\n-TCLTK_PATH = @TCLTK_PATH@\n \n prefix = @prefix@\n exec_prefix = @exec_prefix@\n@@ -27,50 +25,3 @@ VPATH = @srcdir@\n \n export exec_prefix mandir\n export srcdir VPATH\n-\n-NEEDS_SSL_WITH_CRYPTO=@NEEDS_SSL_WITH_CRYPTO@\n-NO_OPENSSL=@NO_OPENSSL@\n-NO_CURL=@NO_CURL@\n-NO_EXPAT=@NO_EXPAT@\n-NO_LIBGEN_H=@NO_LIBGEN_H@\n-HAVE_PATHS_H=@HAVE_PATHS_H@\n-HAVE_LIBCHARSET_H=@HAVE_LIBCHARSET_H@\n-NO_GETTEXT=@NO_GETTEXT@\n-LIBC_CONTAINS_LIBINTL=@LIBC_CONTAINS_LIBINTL@\n-NEEDS_LIBICONV=@NEEDS_LIBICONV@\n-NEEDS_SOCKET=@NEEDS_SOCKET@\n-NEEDS_RESOLV=@NEEDS_RESOLV@\n-NEEDS_LIBGEN=@NEEDS_LIBGEN@\n-NO_SYS_SELECT_H=@NO_SYS_SELECT_H@\n-NO_D_INO_IN_DIRENT=@NO_D_INO_IN_DIRENT@\n-NO_D_TYPE_IN_DIRENT=@NO_D_TYPE_IN_DIRENT@\n-NO_SOCKADDR_STORAGE=@NO_SOCKADDR_STORAGE@\n-NO_IPV6=@NO_IPV6@\n-NO_HSTRERROR=@NO_HSTRERROR@\n-NO_STRCASESTR=@NO_STRCASESTR@\n-NO_STRTOK_R=@NO_STRTOK_R@\n-NO_FNMATCH=@NO_FNMATCH@\n-NO_FNMATCH_CASEFOLD=@NO_FNMATCH_CASEFOLD@\n-NO_MEMMEM=@NO_MEMMEM@\n-NO_STRLCPY=@NO_STRLCPY@\n-NO_UINTMAX_T=@NO_UINTMAX_T@\n-NO_STRTOUMAX=@NO_STRTOUMAX@\n-NO_SETENV=@NO_SETENV@\n-NO_UNSETENV=@NO_UNSETENV@\n-NO_MKDTEMP=@NO_MKDTEMP@\n-NO_MKSTEMPS=@NO_MKSTEMPS@\n-NO_INET_NTOP=@NO_INET_NTOP@\n-NO_INET_PTON=@NO_INET_PTON@\n-NO_ICONV=@NO_ICONV@\n-OLD_ICONV=@OLD_ICONV@\n-NO_REGEX=@NO_REGEX@\n-USE_LIBPCRE=@USE_LIBPCRE@\n-NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@\n-INLINE=@INLINE@\n-SOCKLEN_T=@SOCKLEN_T@\n-FREAD_READS_DIRECTORIES=@FREAD_READS_DIRECTORIES@\n-SNPRINTF_RETURNS_BOGUS=@SNPRINTF_RETURNS_BOGUS@\n-NO_PTHREADS=@NO_PTHREADS@\n-PTHREAD_CFLAGS=@PTHREAD_CFLAGS@\n-PTHREAD_LIBS=@PTHREAD_LIBS@\n-CHARSET_LIB=@CHARSET_LIB@\ndiff --git a/configure.ac b/configure.ac\nindex 450bbe7..da1f41f 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -267,6 +267,8 @@ AS_HELP_STRING([],           [ARG can be also prefix for libpcre library and hea\n \tUSE_LIBPCRE=YesPlease\n \tLIBPCREDIR=$withval\n \tAC_MSG_NOTICE([Setting LIBPCREDIR to $LIBPCREDIR])\n+        dnl USE_LIBPCRE can still be modified below, so don't substitute\n+        dnl it yet.\n \tGIT_CONF_SUBST([LIBPCREDIR])\n     fi)\n #\n@@ -387,9 +389,10 @@ AC_MSG_NOTICE([CHECKS for programs])\n AC_PROG_CC([cc gcc])\n AC_C_INLINE\n case $ac_cv_c_inline in\n-  inline | yes | no)\t;;\n-  *)\t\t\tAC_SUBST([INLINE], [$ac_cv_c_inline]) ;;\n+  inline | yes | no) INLINE='';;\n+  *)                 INLINE=$ac_cv_c_inline ;;\n esac\n+GIT_CONF_SUBST([INLINE])\n \n # which switch to pass runtime path to dynamic libraries to the linker\n AC_CACHE_CHECK([if linker supports -R], git_cv_ld_dashr, [\n@@ -399,7 +402,7 @@ AC_CACHE_CHECK([if linker supports -R], git_cv_ld_dashr, [\n    LDFLAGS=\"${SAVE_LDFLAGS}\"\n ])\n if test \"$git_cv_ld_dashr\" = \"yes\"; then\n-   AC_SUBST(CC_LD_DYNPATH, [-R])\n+   CC_LD_DYNPATH=-R\n else\n    AC_CACHE_CHECK([if linker supports -Wl,-rpath,], git_cv_ld_wl_rpath, [\n       SAVE_LDFLAGS=\"${LDFLAGS}\"\n@@ -408,7 +411,7 @@ else\n       LDFLAGS=\"${SAVE_LDFLAGS}\"\n    ])\n    if test \"$git_cv_ld_wl_rpath\" = \"yes\"; then\n-      AC_SUBST(CC_LD_DYNPATH, [-Wl,-rpath,])\n+      CC_LD_DYNPATH=-Wl,-rpath\n    else\n       AC_CACHE_CHECK([if linker supports -rpath], git_cv_ld_rpath, [\n          SAVE_LDFLAGS=\"${LDFLAGS}\"\n@@ -417,32 +420,35 @@ else\n          LDFLAGS=\"${SAVE_LDFLAGS}\"\n       ])\n       if test \"$git_cv_ld_rpath\" = \"yes\"; then\n-         AC_SUBST(CC_LD_DYNPATH, [-rpath])\n+         CC_LD_DYNPATH=-rpath\n       else\n+         CC_LD_DYNPATH=\n          AC_MSG_WARN([linker does not support runtime path to dynamic libraries])\n       fi\n    fi\n fi\n+GIT_CONF_SUBST([CC_LD_DYNPATH])\n #AC_PROG_INSTALL\t\t# needs install-sh or install.sh in sources\n AC_CHECK_TOOLS(AR, [gar ar], :)\n AC_CHECK_PROGS(TAR, [gtar tar])\n AC_CHECK_PROGS(DIFF, [gnudiff gdiff diff])\n # TCLTK_PATH will be set to some value if we want Tcl/Tk\n # or will be empty otherwise.\n-if test -z \"$NO_TCLTK\"; then\n+if test -n \"$NO_TCLTK\"; then\n+  TCLTK_PATH=\n+else\n   if test \"$with_tcltk\" = \"\"; then\n   # No Tcl/Tk switches given. Do not check for Tcl/Tk, use bare 'wish'.\n     TCLTK_PATH=wish\n-    AC_SUBST(TCLTK_PATH)\n   elif test \"$with_tcltk\" = \"yes\"; then\n   # Tcl/Tk check requested.\n     AC_CHECK_PROGS(TCLTK_PATH, [wish], )\n   else\n     AC_MSG_RESULT([Using Tcl/Tk interpreter $with_tcltk])\n     TCLTK_PATH=\"$with_tcltk\"\n-    AC_SUBST(TCLTK_PATH)\n   fi\n fi\n+GIT_CONF_SUBST([TCLTK_PATH])\n AC_CHECK_PROGS(ASCIIDOC, [asciidoc])\n if test -n \"$ASCIIDOC\"; then\n \tAC_MSG_CHECKING([for asciidoc version])\n@@ -469,13 +475,13 @@ GIT_STASH_FLAGS($OPENSSLDIR)\n AC_CHECK_LIB([crypto], [SHA1_Init],\n [NEEDS_SSL_WITH_CRYPTO=],\n [AC_CHECK_LIB([ssl], [SHA1_Init],\n- [NEEDS_SSL_WITH_CRYPTO=YesPlease],\n- [NEEDS_SSL_WITH_CRYPTO= NO_OPENSSL=YesPlease])])\n+ [NEEDS_SSL_WITH_CRYPTO=YesPlease NO_OPENSSL=],\n+ [NEEDS_SSL_WITH_CRYPTO=          NO_OPENSSL=YesPlease])])\n \n GIT_UNSTASH_FLAGS($OPENSSLDIR)\n \n-AC_SUBST(NEEDS_SSL_WITH_CRYPTO)\n-AC_SUBST(NO_OPENSSL)\n+GIT_CONF_SUBST([NEEDS_SSL_WITH_CRYPTO])\n+GIT_CONF_SUBST([NO_OPENSSL])\n \n #\n # Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be\n@@ -492,7 +498,7 @@ AC_CHECK_LIB([pcre], [pcre_version],\n \n GIT_UNSTASH_FLAGS($LIBPCREDIR)\n \n-AC_SUBST(USE_LIBPCRE)\n+GIT_CONF_SUBST([USE_LIBPCRE])\n \n fi\n \n@@ -509,7 +515,7 @@ AC_CHECK_LIB([curl], [curl_global_init],\n \n GIT_UNSTASH_FLAGS($CURLDIR)\n \n-AC_SUBST(NO_CURL)\n+GIT_CONF_SUBST([NO_CURL])\n \n #\n # Define NO_EXPAT if you do not have expat installed.  git-http-push is\n@@ -523,7 +529,7 @@ AC_CHECK_LIB([expat], [XML_ParserCreate],\n \n GIT_UNSTASH_FLAGS($EXPATDIR)\n \n-AC_SUBST(NO_EXPAT)\n+GIT_CONF_SUBST([NO_EXPAT])\n \n #\n # Define NEEDS_LIBICONV if linking with libc is not enough (Darwin and\n@@ -569,8 +575,8 @@ LIBS=\"$old_LIBS\"\n \n GIT_UNSTASH_FLAGS($ICONVDIR)\n \n-AC_SUBST(NEEDS_LIBICONV)\n-AC_SUBST(NO_ICONV)\n+GIT_CONF_SUBST([NEEDS_LIBICONV])\n+GIT_CONF_SUBST([NO_ICONV])\n \n if test -n \"$NO_ICONV\"; then\n     NEEDS_LIBICONV=\n@@ -597,7 +603,7 @@ LIBS=\"$old_LIBS\"\n \n GIT_UNSTASH_FLAGS($ZLIB_PATH)\n \n-AC_SUBST(NO_DEFLATE_BOUND)\n+GIT_CONF_SUBST([NO_DEFLATE_BOUND])\n \n #\n # Define NEEDS_SOCKET if linking with libc is not enough (SunOS,\n@@ -605,7 +611,7 @@ AC_SUBST(NO_DEFLATE_BOUND)\n AC_CHECK_LIB([c], [socket],\n [NEEDS_SOCKET=],\n [NEEDS_SOCKET=YesPlease])\n-AC_SUBST(NEEDS_SOCKET)\n+GIT_CONF_SUBST([NEEDS_SOCKET])\n test -n \"$NEEDS_SOCKET\" && LIBS=\"$LIBS -lsocket\"\n \n #\n@@ -613,40 +619,43 @@ test -n \"$NEEDS_SOCKET\" && LIBS=\"$LIBS -lsocket\"\n # libresolv provides some of the functions we would normally get\n # from libc.\n NEEDS_RESOLV=\n-AC_SUBST(NEEDS_RESOLV)\n #\n # Define NO_INET_NTOP if linking with -lresolv is not enough.\n # Solaris 2.7 in particular hos inet_ntop in -lresolv.\n NO_INET_NTOP=\n-AC_SUBST(NO_INET_NTOP)\n AC_CHECK_FUNC([inet_ntop],\n-\t[],\n+    [],\n     [AC_CHECK_LIB([resolv], [inet_ntop],\n-\t    [NEEDS_RESOLV=YesPlease],\n+\t[NEEDS_RESOLV=YesPlease],\n \t[NO_INET_NTOP=YesPlease])\n ])\n+GIT_CONF_SUBST([NO_INET_NTOP])\n #\n # Define NO_INET_PTON if linking with -lresolv is not enough.\n # Solaris 2.7 in particular hos inet_pton in -lresolv.\n NO_INET_PTON=\n-AC_SUBST(NO_INET_PTON)\n AC_CHECK_FUNC([inet_pton],\n-\t[],\n+    [],\n     [AC_CHECK_LIB([resolv], [inet_pton],\n-\t    [NEEDS_RESOLV=YesPlease],\n+\t[NEEDS_RESOLV=YesPlease],\n \t[NO_INET_PTON=YesPlease])\n ])\n+GIT_CONF_SUBST([NO_INET_PTON])\n #\n # Define NO_HSTRERROR if linking with -lresolv is not enough.\n # Solaris 2.6 in particular has no hstrerror, even in -lresolv.\n NO_HSTRERROR=\n AC_CHECK_FUNC([hstrerror],\n-\t[],\n+    [],\n     [AC_CHECK_LIB([resolv], [hstrerror],\n-\t    [NEEDS_RESOLV=YesPlease],\n+\t[NEEDS_RESOLV=YesPlease],\n \t[NO_HSTRERROR=YesPlease])\n ])\n-AC_SUBST(NO_HSTRERROR)\n+GIT_CONF_SUBST([NO_HSTRERROR])\n+\n+dnl This must go after all the possible places for its initialization,\n+dnl in the AC_CHECK_FUNC invocations above.\n+GIT_CONF_SUBST([NEEDS_RESOLV])\n #\n # If any of the above tests determined that -lresolv is needed at\n # build-time, also set it here for remaining configure-time checks.\n@@ -655,13 +664,13 @@ test -n \"$NEEDS_RESOLV\" && LIBS=\"$LIBS -lresolv\"\n AC_CHECK_LIB([c], [basename],\n [NEEDS_LIBGEN=],\n [NEEDS_LIBGEN=YesPlease])\n-AC_SUBST(NEEDS_LIBGEN)\n+GIT_CONF_SUBST([NEEDS_LIBGEN])\n test -n \"$NEEDS_LIBGEN\" && LIBS=\"$LIBS -lgen\"\n \n AC_CHECK_LIB([c], [gettext],\n [LIBC_CONTAINS_LIBINTL=YesPlease],\n [LIBC_CONTAINS_LIBINTL=])\n-AC_SUBST(LIBC_CONTAINS_LIBINTL)\n+GIT_CONF_SUBST([LIBC_CONTAINS_LIBINTL])\n \n #\n # Define NO_GETTEXT if you don't want Git output to be translated.\n@@ -669,7 +678,7 @@ AC_SUBST(LIBC_CONTAINS_LIBINTL)\n AC_CHECK_HEADER([libintl.h],\n [NO_GETTEXT=],\n [NO_GETTEXT=YesPlease])\n-AC_SUBST(NO_GETTEXT)\n+GIT_CONF_SUBST([NO_GETTEXT])\n \n if test -z \"$NO_GETTEXT\"; then\n     test -n \"$LIBC_CONTAINS_LIBINTL\" || LIBS=\"$LIBS -lintl\"\n@@ -682,19 +691,19 @@ AC_MSG_NOTICE([CHECKS for header files])\n AC_CHECK_HEADER([sys/select.h],\n [NO_SYS_SELECT_H=],\n [NO_SYS_SELECT_H=UnfortunatelyYes])\n-AC_SUBST(NO_SYS_SELECT_H)\n+GIT_CONF_SUBST([NO_SYS_SELECT_H])\n #\n # Define NO_SYS_POLL_H if you don't have sys/poll.h\n AC_CHECK_HEADER([sys/poll.h],\n [NO_SYS_POLL_H=],\n [NO_SYS_POLL_H=UnfortunatelyYes])\n-AC_SUBST(NO_SYS_POLL_H)\n+GIT_CONF_SUBST([NO_SYS_POLL_H])\n #\n # Define NO_INTTYPES_H if you don't have inttypes.h\n AC_CHECK_HEADER([inttypes.h],\n [NO_INTTYPES_H=],\n [NO_INTTYPES_H=UnfortunatelyYes])\n-AC_SUBST(NO_INTTYPES_H)\n+GIT_CONF_SUBST([NO_INTTYPES_H])\n #\n # Define OLD_ICONV if your library has an old iconv(), where the second\n # (input buffer pointer) parameter is declared with type (const char **).\n@@ -717,23 +726,24 @@ AC_COMPILE_IFELSE([OLDICONVTEST_SRC],\n \n GIT_UNSTASH_FLAGS($ICONVDIR)\n \n-AC_SUBST(OLD_ICONV)\n+GIT_CONF_SUBST([OLD_ICONV])\n \n ## Checks for typedefs, structures, and compiler characteristics.\n AC_MSG_NOTICE([CHECKS for typedefs, structures, and compiler characteristics])\n #\n TYPE_SOCKLEN_T\n case $ac_cv_type_socklen_t in\n-  yes)\t;;\n-  *)  \tAC_SUBST([SOCKLEN_T], [$git_cv_socklen_t_equiv]) ;;\n+  yes)\tSOCKLEN_T='';;\n+  *)  \tSOCKLEN_T=$git_cv_socklen_t_equiv;;\n esac\n+GIT_CONF_SUBST([SOCKLEN_T])\n \n # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.\n AC_CHECK_MEMBER(struct dirent.d_ino,\n [NO_D_INO_IN_DIRENT=],\n [NO_D_INO_IN_DIRENT=YesPlease],\n [#include <dirent.h>])\n-AC_SUBST(NO_D_INO_IN_DIRENT)\n+GIT_CONF_SUBST([NO_D_INO_IN_DIRENT])\n #\n # Define NO_D_TYPE_IN_DIRENT if your platform defines DT_UNKNOWN but lacks\n # d_type in struct dirent (latest Cygwin -- will be fixed soonish).\n@@ -741,7 +751,7 @@ AC_CHECK_MEMBER(struct dirent.d_type,\n [NO_D_TYPE_IN_DIRENT=],\n [NO_D_TYPE_IN_DIRENT=YesPlease],\n [#include <dirent.h>])\n-AC_SUBST(NO_D_TYPE_IN_DIRENT)\n+GIT_CONF_SUBST([NO_D_TYPE_IN_DIRENT])\n #\n # Define NO_SOCKADDR_STORAGE if your platform does not have struct\n # sockaddr_storage.\n@@ -751,7 +761,7 @@ AC_CHECK_TYPE(struct sockaddr_storage,\n #include <sys/types.h>\n #include <sys/socket.h>\n ])\n-AC_SUBST(NO_SOCKADDR_STORAGE)\n+GIT_CONF_SUBST([NO_SOCKADDR_STORAGE])\n #\n # Define NO_IPV6 if you lack IPv6 support and getaddrinfo().\n AC_CHECK_TYPE([struct addrinfo],[\n@@ -763,7 +773,7 @@ AC_CHECK_TYPE([struct addrinfo],[\n #include <sys/socket.h>\n #include <netdb.h>\n ])\n-AC_SUBST(NO_IPV6)\n+GIT_CONF_SUBST([NO_IPV6])\n #\n # Define NO_REGEX if you have no or inferior regex support in your C library.\n AC_CACHE_CHECK([whether the platform regex can handle null bytes],\n@@ -784,7 +794,7 @@ if test $ac_cv_c_excellent_regex = yes; then\n else\n \tNO_REGEX=YesPlease\n fi\n-AC_SUBST(NO_REGEX)\n+GIT_CONF_SUBST([NO_REGEX])\n #\n # Define FREAD_READS_DIRECTORIES if your are on a system which succeeds\n # when attempting to read from an fopen'ed directory.\n@@ -804,7 +814,7 @@ if test $ac_cv_fread_reads_directories = yes; then\n else\n \tFREAD_READS_DIRECTORIES=\n fi\n-AC_SUBST(FREAD_READS_DIRECTORIES)\n+GIT_CONF_SUBST([FREAD_READS_DIRECTORIES])\n #\n # Define SNPRINTF_RETURNS_BOGUS if your are on a system which snprintf()\n # or vsnprintf() return -1 instead of number of characters which would\n@@ -838,7 +848,7 @@ if test $ac_cv_snprintf_returns_bogus = yes; then\n else\n \tSNPRINTF_RETURNS_BOGUS=\n fi\n-AC_SUBST(SNPRINTF_RETURNS_BOGUS)\n+GIT_CONF_SUBST([SNPRINTF_RETURNS_BOGUS])\n \n \n ## Checks for library functions.\n@@ -849,47 +859,45 @@ AC_MSG_NOTICE([CHECKS for library functions])\n AC_CHECK_HEADER([libgen.h],\n [NO_LIBGEN_H=],\n [NO_LIBGEN_H=YesPlease])\n-AC_SUBST(NO_LIBGEN_H)\n+GIT_CONF_SUBST([NO_LIBGEN_H])\n #\n # Define HAVE_PATHS_H if you have paths.h.\n AC_CHECK_HEADER([paths.h],\n [HAVE_PATHS_H=YesPlease],\n [HAVE_PATHS_H=])\n-AC_SUBST(HAVE_PATHS_H)\n+GIT_CONF_SUBST([HAVE_PATHS_H])\n #\n # Define HAVE_LIBCHARSET_H if have libcharset.h\n AC_CHECK_HEADER([libcharset.h],\n [HAVE_LIBCHARSET_H=YesPlease],\n [HAVE_LIBCHARSET_H=])\n-AC_SUBST(HAVE_LIBCHARSET_H)\n+GIT_CONF_SUBST([HAVE_LIBCHARSET_H])\n # Define CHARSET_LIB if libiconv does not export the locale_charset symbol\n # and libcharset does\n CHARSET_LIB=\n AC_CHECK_LIB([iconv], [locale_charset],\n        [],\n        [AC_CHECK_LIB([charset], [locale_charset],\n-                     [CHARSET_LIB=-lcharset])\n-       ]\n-)\n-AC_SUBST(CHARSET_LIB)\n+                     [CHARSET_LIB=-lcharset])])\n+GIT_CONF_SUBST([CHARSET_LIB])\n #\n # Define NO_STRCASESTR if you don't have strcasestr.\n GIT_CHECK_FUNC(strcasestr,\n [NO_STRCASESTR=],\n [NO_STRCASESTR=YesPlease])\n-AC_SUBST(NO_STRCASESTR)\n+GIT_CONF_SUBST([NO_STRCASESTR])\n #\n # Define NO_STRTOK_R if you don't have strtok_r\n GIT_CHECK_FUNC(strtok_r,\n [NO_STRTOK_R=],\n [NO_STRTOK_R=YesPlease])\n-AC_SUBST(NO_STRTOK_R)\n+GIT_CONF_SUBST([NO_STRTOK_R])\n #\n # Define NO_FNMATCH if you don't have fnmatch\n GIT_CHECK_FUNC(fnmatch,\n [NO_FNMATCH=],\n [NO_FNMATCH=YesPlease])\n-AC_SUBST(NO_FNMATCH)\n+GIT_CONF_SUBST([NO_FNMATCH])\n #\n # Define NO_FNMATCH_CASEFOLD if your fnmatch function doesn't have the\n # FNM_CASEFOLD GNU extension.\n@@ -911,19 +919,19 @@ if test $ac_cv_c_excellent_fnmatch = yes; then\n else\n \tNO_FNMATCH_CASEFOLD=YesPlease\n fi\n-AC_SUBST(NO_FNMATCH_CASEFOLD)\n+GIT_CONF_SUBST([NO_FNMATCH_CASEFOLD])\n #\n # Define NO_MEMMEM if you don't have memmem.\n GIT_CHECK_FUNC(memmem,\n [NO_MEMMEM=],\n [NO_MEMMEM=YesPlease])\n-AC_SUBST(NO_MEMMEM)\n+GIT_CONF_SUBST([NO_MEMMEM])\n #\n # Define NO_STRLCPY if you don't have strlcpy.\n GIT_CHECK_FUNC(strlcpy,\n [NO_STRLCPY=],\n [NO_STRLCPY=YesPlease])\n-AC_SUBST(NO_STRLCPY)\n+GIT_CONF_SUBST([NO_STRLCPY])\n #\n # Define NO_UINTMAX_T if your platform does not have uintmax_t\n AC_CHECK_TYPE(uintmax_t,\n@@ -931,43 +939,43 @@ AC_CHECK_TYPE(uintmax_t,\n [NO_UINTMAX_T=YesPlease],[\n #include <inttypes.h>\n ])\n-AC_SUBST(NO_UINTMAX_T)\n+GIT_CONF_SUBST([NO_UINTMAX_T])\n #\n # Define NO_STRTOUMAX if you don't have strtoumax in the C library.\n GIT_CHECK_FUNC(strtoumax,\n [NO_STRTOUMAX=],\n [NO_STRTOUMAX=YesPlease])\n-AC_SUBST(NO_STRTOUMAX)\n+GIT_CONF_SUBST([NO_STRTOUMAX])\n #\n # Define NO_SETENV if you don't have setenv in the C library.\n GIT_CHECK_FUNC(setenv,\n [NO_SETENV=],\n [NO_SETENV=YesPlease])\n-AC_SUBST(NO_SETENV)\n+GIT_CONF_SUBST([NO_SETENV])\n #\n # Define NO_UNSETENV if you don't have unsetenv in the C library.\n GIT_CHECK_FUNC(unsetenv,\n [NO_UNSETENV=],\n [NO_UNSETENV=YesPlease])\n-AC_SUBST(NO_UNSETENV)\n+GIT_CONF_SUBST([NO_UNSETENV])\n #\n # Define NO_MKDTEMP if you don't have mkdtemp in the C library.\n GIT_CHECK_FUNC(mkdtemp,\n [NO_MKDTEMP=],\n [NO_MKDTEMP=YesPlease])\n-AC_SUBST(NO_MKDTEMP)\n+GIT_CONF_SUBST([NO_MKDTEMP])\n #\n # Define NO_MKSTEMPS if you don't have mkstemps in the C library.\n GIT_CHECK_FUNC(mkstemps,\n [NO_MKSTEMPS=],\n [NO_MKSTEMPS=YesPlease])\n-AC_SUBST(NO_MKSTEMPS)\n+GIT_CONF_SUBST([NO_MKSTEMPS])\n #\n # Define NO_INITGROUPS if you don't have initgroups in the C library.\n GIT_CHECK_FUNC(initgroups,\n [NO_INITGROUPS=],\n [NO_INITGROUPS=YesPlease])\n-AC_SUBST(NO_INITGROUPS)\n+GIT_CONF_SUBST([NO_INITGROUPS])\n #\n #\n # Define NO_MMAP if you want to avoid mmap.\n@@ -1049,9 +1057,9 @@ fi\n \n CFLAGS=\"$old_CFLAGS\"\n \n-AC_SUBST(PTHREAD_CFLAGS)\n-AC_SUBST(PTHREAD_LIBS)\n-AC_SUBST(NO_PTHREADS)\n+GIT_CONF_SUBST([PTHREAD_CFLAGS])\n+GIT_CONF_SUBST([PTHREAD_LIBS])\n+GIT_CONF_SUBST([NO_PTHREADS])\n \n ## Output files\n AC_CONFIG_FILES([\"${config_file}\":\"${config_in}\"])\n-- \n1.7.12.317.g1c54b74\n"},{"id":"198801","messageId":"7vtxv4h3lh.fsf@alter.siamese.dyndns.org","threadId":"31505","inReplyTo":"1c54b744c0ec6987f7987a41853ab0ae00513d03.1347378210.git.stefano.lattarini@gmail.com","subject":"Re: [PATCH 2/2] build: don't duplicate substitution of make variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-11T17:27:38Z","receivedAt":"2012-09-11T17:27:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefano Lattarini <stefano.lattarini@gmail.com> writes:\n\n> Thanks to our 'GIT_CONF_SUBST' layer in configure.ac, a make variable 'VAR'\n> can be defined to a value 'VAL' at ./configure runtime in our build system\n> simply by using \"GIT_CONF_SUBST([VAR], [VAL])\" in configure.ac, rather than\n> having both to call \"AC_SUBST([VAR], [VAL])\" in configure.ac and adding the\n> 'VAR = @VAR@' definition in config.mak.in.  Less duplication, less margin\n> for error, less possibility of confusion.\n>\n> While at it, fix some formatting issues in configure.ac that unnecessarily\n> obscured the code flow.\n>\n> Signed-off-by: Stefano Lattarini <stefano.lattarini@gmail.com>\n> ---\n>  config.mak.in |  49 --------------------\n>  configure.ac  | 144 +++++++++++++++++++++++++++++++---------------------------\n>  2 files changed, 76 insertions(+), 117 deletions(-)\n\nWhoa ;-).\n\n> diff --git a/configure.ac b/configure.ac\n> index 450bbe7..da1f41f 100644\n> --- a/configure.ac\n> +++ b/configure.ac\n> @@ -267,6 +267,8 @@ AS_HELP_STRING([],           [ARG can be also prefix for libpcre library and hea\n>  \tUSE_LIBPCRE=YesPlease\n>  \tLIBPCREDIR=$withval\n>  \tAC_MSG_NOTICE([Setting LIBPCREDIR to $LIBPCREDIR])\n> +        dnl USE_LIBPCRE can still be modified below, so don't substitute\n> +        dnl it yet.\n>  \tGIT_CONF_SUBST([LIBPCREDIR])\n>      fi)\n>  #\n> ...\n>  AC_CHECK_FUNC([hstrerror],\n> -\t[],\n> +    [],\n\nIs there some consistent policy regarding SP vs HT in the\nindentation you are using in this patch?  These two hunks suggest\nthat you may be favoring spaces, but other places you seem to use\ntabs, so...\n"},{"id":"198804","messageId":"504F824F.3050903@gmail.com","threadId":"31505","inReplyTo":"7vtxv4h3lh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] build: don't duplicate substitution of make variables","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-09-11T18:26:23Z","receivedAt":"2012-09-11T18:26:23Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"On 09/11/2012 07:27 PM, Junio C Hamano wrote:\n> Stefano Lattarini <stefano.lattarini@gmail.com> writes:\n> \n>> Thanks to our 'GIT_CONF_SUBST' layer in configure.ac, a make variable 'VAR'\n>> can be defined to a value 'VAL' at ./configure runtime in our build system\n>> simply by using \"GIT_CONF_SUBST([VAR], [VAL])\" in configure.ac, rather than\n>> having both to call \"AC_SUBST([VAR], [VAL])\" in configure.ac and adding the\n>> 'VAR = @VAR@' definition in config.mak.in.  Less duplication, less margin\n>> for error, less possibility of confusion.\n>>\n>> While at it, fix some formatting issues in configure.ac that unnecessarily\n>> obscured the code flow.\n>>\n>> Signed-off-by: Stefano Lattarini <stefano.lattarini@gmail.com>\n>> ---\n>>  config.mak.in |  49 --------------------\n>>  configure.ac  | 144 +++++++++++++++++++++++++++++++---------------------------\n>>  2 files changed, 76 insertions(+), 117 deletions(-)\n> \n> Whoa ;-).\n>\nWell, I could have converted one variable at the time, but that seemed\nan overkill :-)\n\n>> diff --git a/configure.ac b/configure.ac\n>> index 450bbe7..da1f41f 100644\n>> --- a/configure.ac\n>> +++ b/configure.ac\n>> @@ -267,6 +267,8 @@ AS_HELP_STRING([],           [ARG can be also prefix for libpcre library and hea\n>>  \tUSE_LIBPCRE=YesPlease\n>>  \tLIBPCREDIR=$withval\n>>  \tAC_MSG_NOTICE([Setting LIBPCREDIR to $LIBPCREDIR])\n>> +        dnl USE_LIBPCRE can still be modified below, so don't substitute\n>> +        dnl it yet.\n>>  \tGIT_CONF_SUBST([LIBPCREDIR])\n>>      fi)\n>>  #\n>> ...\n>>  AC_CHECK_FUNC([hstrerror],\n>> -\t[],\n>> +    [],\n> \n> Is there some consistent policy regarding SP vs HT in the\n> indentation you are using in this patch?\n>\nBasically I'm trying to follow the style of the surrounding code, while\nkeeping in mind that in the Git codebase tabs seem to be preferred to spaces.\nIn this case, the indentation of the following text (that was the \"meat\" of\nthe expression) seemed to favour 4 spaces for indentation, so I followed\nsuit.\n\n> These two hunks suggest\n> that you may be favoring spaces, but other places you seem to use\n> tabs, so...\n>\nI can convert the new tabs to spaces if you prefer (that would have been\nmy preference too, but thought trying to follow the \"Git preferences\"\nwas more important).  No big deal either way for me.\n\nThanks,\n  Stefano\n"},{"id":"198808","messageId":"7vd31sgww4.fsf@alter.siamese.dyndns.org","threadId":"31505","inReplyTo":"504F824F.3050903@gmail.com","subject":"Re: [PATCH 2/2] build: don't duplicate substitution of make variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-11T19:52:27Z","receivedAt":"2012-09-11T19:52:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefano Lattarini <stefano.lattarini@gmail.com> writes:\n\n> On 09/11/2012 07:27 PM, Junio C Hamano wrote:\n>> Stefano Lattarini <stefano.lattarini@gmail.com> writes:\n>> \n>>> Thanks to our 'GIT_CONF_SUBST' layer in configure.ac, a make variable 'VAR'\n>>> can be defined to a value 'VAL' at ./configure runtime in our build system\n>>> simply by using \"GIT_CONF_SUBST([VAR], [VAL])\" in configure.ac, rather than\n>>> having both to call \"AC_SUBST([VAR], [VAL])\" in configure.ac and adding the\n>>> 'VAR = @VAR@' definition in config.mak.in.  Less duplication, less margin\n>>> for error, less possibility of confusion.\n>>>\n>>> While at it, fix some formatting issues in configure.ac that unnecessarily\n>>> obscured the code flow.\n>>>\n>>> Signed-off-by: Stefano Lattarini <stefano.lattarini@gmail.com>\n>>> ---\n>>>  config.mak.in |  49 --------------------\n>>>  configure.ac  | 144 +++++++++++++++++++++++++++++++---------------------------\n>>>  2 files changed, 76 insertions(+), 117 deletions(-)\n>> \n>> Whoa ;-).\n>>\n> Well, I could have converted one variable at the time, but that seemed\n> an overkill :-)\n\nNo, I was happy to see many lines go ;-)\n>> These two hunks suggest\n>> that you may be favoring spaces, but other places you seem to use\n>> tabs, so...\n>>\n> I can convert the new tabs to spaces if you prefer (that would have been\n> my preference too, but thought trying to follow the \"Git preferences\"\n> was more important).  No big deal either way for me.\n\nIf this were other parts of the system, my preference would be to\nuse tabs, but because I do not help very much in the autoconf part\nmyself, I do not have a particular preference.  If it is more common\nto indent the configure.ac script with spaces, that would be more\nfamiliar to the folks who work on it, and I do not have much against\nchoosing and sticking to space indented configure.ac file if that is\nthe policy.\n\nBut if this patch is not about cleaning up the style to make it\nconform to a policy (whichever it is), I would have preferred to see\na clean-up patch as a separate step, not mixed together with this.\n\nThat's all; either way, no big deal.\n"},{"id":"198810","messageId":"504F9C73.1040409@gmail.com","threadId":"31505","inReplyTo":"7vd31sgww4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] build: don't duplicate substitution of make variables","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-09-11T20:17:55Z","receivedAt":"2012-09-11T20:17:55Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"On 09/11/2012 09:52 PM, Junio C Hamano wrote:\n> Stefano Lattarini <stefano.lattarini@gmail.com> writes:\n> \n>> On 09/11/2012 07:27 PM, Junio C Hamano wrote:\n>>\n>>> These two hunks suggest that you may be favoring spaces, but other\n>>> places you seem to use tabs, so...\n>>>\n>> I can convert the new tabs to spaces if you prefer (that would have been\n>> my preference too, but thought trying to follow the \"Git preferences\"\n>> was more important).  No big deal either way for me.\n> \n> If this were other parts of the system, my preference would be to\n> use tabs, but because I do not help very much in the autoconf part\n> myself, I do not have a particular preference.  If it is more common\n> to indent the configure.ac script with spaces,\n>\nThere is no general standard about it that I know of; it's just that\nGNU projects tend to prefer space-based indentation over tab-based one,\nand since I mostly touch configure.ac files from GNU projects, I've\npicked up the habit.\n\n> that would be more\n> familiar to the folks who work on it, and I do not have much against\n> choosing and sticking to space indented configure.ac file if that is\n> the policy.\n> \nThen I might send a patch that normalize indentation in configure.ac\nto \"spaces only\", if that's OK with you.  But that's obviously for a\nseparated thread.\n\n> But if this patch is not about cleaning up the style to make it\n> conform to a policy (whichever it is), I would have preferred to see\n> a clean-up patch as a separate step, not mixed together with this.\n>\nThe reason those few clean-ups are mixed into this patch is that the\npre-existing strange indentation style was actually making it more\ndifficult for me to grasp the code flow; that is, I didn't see them\nas a cosmetic change, but as a way to make it easier for me and the\nreader to see that my changes were correct and sensible.\n\n> That's all; either way, no big deal.\n> \nOK.  Just let me know if you'd still prefer to have the indentation\ncleanups done by a preparatory patch, and I'll send a re-roll.\n\nThanks,\n  Stefano\n"}]}