{"thread":{"id":"34907","subject":"[PATCH v2] configure.ac: move the private git m4 macros to a dedicated directory","startedAt":"2013-09-11T15:46:57Z","lastAt":"2013-09-11T20:05:18Z","messageCount":3,"participants":["Elia Pinto","Stefano Lattarini"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"227452","messageId":"1378914417-32605-1-git-send-email-gitter.spiros@gmail.com","threadId":"34907","inReplyTo":null,"subject":"[PATCH v2] configure.ac: move the private git m4 macros to a dedicated directory","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2013-09-11T15:46:57Z","receivedAt":"2013-09-11T15:46:57Z","isPatch":true,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"Git use, as many project that use autoconf, private m4 macros.\n\nWhen not using automake, and just relying on autoconf, the macro\nfiles are not picked up by default.\n\nA possibility, as git do today, is to put the private m4 macro\nin the configure.ac file, so they will copied over the final configure\nwhen calling autoreconf(that call also autoconf).\nBut this makes configure.ac difficult to read and maintain,\nespecially if you want to introduce new macros later. By separating\nthe definitions of the macros from configure.ac file the build system\nwould be more modular.\n\nStarting from version 2.58, autoconf provide the macro AC_CONFIG_MACRO_DIR\nto declare where additional macro files are to be put and found by aclocal.\nThe argument passed to this macro is commonly m4. Despite the documentation,\nautoconf do nothing with it, only aclocal can use directly if invoked by\n-I m4 or indirectly using automake. But autoreconf don't invoke aclocal\nin this way. So in summary you can not use this macro in a useful\nway if you only use autoconf, as git does.\n\nAnother historical possibility is to list all your macros in acinclude.m4.\nThis file will be included in aclocal.m4 when you run aclocal, and its macro(s)\nwill henceforth be visible to autoconf. However if it contains numerous macros,\nit will rapidly become difficult to maintain, and for git this don't provide\nany benefits or very little.\n\nThe actual autotool documentation recommend to write each\nmacro in its own file and gather all these files in a separate directory.\n\nGiven the limitations i mentioned earlier, the only possibility is to use the m4_include\nfor including every macro file. The m4_include directive works quite like the\n#include directive of the C programming language, and simply copies over the content\nof the file(s).\n\nSigned-off-by: Elia Pinto <gitter.spiros@gmail.com>\n---\nThis is a second version of this patch http://article.gmane.org/gmane.comp.version-control.git/231984.\nThe first was plain wrong, my bad. I am sorry for the long delay. \nSure it is something low-hanging fruit\n\n\n configure.ac                      |  148 +++----------------------------------\n m4/git_arg_set_path.m4            |   14 ++++\n m4/git_check_func.m4              |   13 ++++\n m4/git_conf_append_path.m4        |   30 ++++++++\n m4/git_conf_subst.m4              |   10 +++\n m4/git_conf_subst_init.m4         |   15 ++++\n m4/git_parse_with.m4              |   22 ++++++\n m4/git_parse_with_set_make_var.m4 |   20 +++++\n m4/git_stash_flags.m4             |   15 ++++\n m4/git_unstash_flags.m4           |   13 ++++\n 10 files changed, 162 insertions(+), 138 deletions(-)\n create mode 100644 m4/git_arg_set_path.m4\n create mode 100644 m4/git_check_func.m4\n create mode 100644 m4/git_conf_append_path.m4\n create mode 100644 m4/git_conf_subst.m4\n create mode 100644 m4/git_conf_subst_init.m4\n create mode 100644 m4/git_parse_with.m4\n create mode 100644 m4/git_parse_with_set_make_var.m4\n create mode 100644 m4/git_stash_flags.m4\n create mode 100644 m4/git_unstash_flags.m4\n\ndiff --git a/configure.ac b/configure.ac\nindex 2f43393..81a876f 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -1,144 +1,6 @@\n #                                               -*- Autoconf -*-\n # Process this file with autoconf to produce a configure script.\n \n-## Definitions of private macros.\n-\n-# GIT_CONF_SUBST(VAL, VAR)\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}dnl\n-$1=m4_if([$#],[1],[${$1}],[$2])\"])\n-\n-# GIT_CONF_SUBST_INIT\n-# -------------------\n-# Prepare shell variables and autoconf machine required by later calls\n-# to GIT_CONF_SUBST.\n-AC_DEFUN([GIT_CONF_SUBST_INIT],\n-    [config_appended_defs=; newline='\n-'\n-    AC_CONFIG_COMMANDS([$config_file],\n-                       [echo \"$config_appended_defs\" >> \"$config_file\"],\n-                       [config_file=$config_file\n-                        config_appended_defs=\"$config_appended_defs\"])])\n-\n-# GIT_ARG_SET_PATH(PROGRAM)\n-# -------------------------\n-# Provide --with-PROGRAM=PATH option to set PATH to PROGRAM\n-# Optional second argument allows setting NO_PROGRAM=YesPlease if\n-# --without-PROGRAM version used.\n-AC_DEFUN([GIT_ARG_SET_PATH],\n-    [AC_ARG_WITH([$1],\n-        [AS_HELP_STRING([--with-$1=PATH],\n-                        [provide PATH to $1])],\n-        [GIT_CONF_APPEND_PATH([$1], [$2])],\n-        [])])\n-\n-# GIT_CONF_APPEND_PATH(PROGRAM)\n-# -----------------------------\n-# Parse --with-PROGRAM=PATH option to set PROGRAM_PATH=PATH\n-# Used by GIT_ARG_SET_PATH(PROGRAM)\n-# Optional second argument allows setting NO_PROGRAM=YesPlease if\n-# --without-PROGRAM is used.\n-AC_DEFUN([GIT_CONF_APPEND_PATH],\n-    [m4_pushdef([GIT_UC_PROGRAM], m4_toupper([$1]))dnl\n-    if test \"$withval\" = \"no\"; then\n-\tif test -n \"$2\"; then\n-\t\tGIT_UC_PROGRAM[]_PATH=$withval\n-\t\tAC_MSG_NOTICE([Disabling use of GIT_UC_PROGRAM])\n-\t\tGIT_CONF_SUBST([NO_]GIT_UC_PROGRAM, [YesPlease])\n-\t\tGIT_CONF_SUBST(GIT_UC_PROGRAM[]_PATH, [])\n-\telse\n-\t\tAC_MSG_ERROR([You cannot use git without $1])\n-\tfi\n-    else\n-\tif test \"$withval\" = \"yes\"; then\n-\t\tAC_MSG_WARN([You should provide path for --with-$1=PATH])\n-\telse\n-\t\tGIT_UC_PROGRAM[]_PATH=$withval\n-\t\tAC_MSG_NOTICE([Setting GIT_UC_PROGRAM[]_PATH to $withval])\n-\t\tGIT_CONF_SUBST(GIT_UC_PROGRAM[]_PATH, [$withval])\n-\tfi\n-    fi\n-    m4_popdef([GIT_UC_PROGRAM])])\n-\n-# GIT_PARSE_WITH(PACKAGE)\n-# -----------------------\n-# For use in AC_ARG_WITH action-if-found, for packages default ON.\n-# * Set NO_PACKAGE=YesPlease for --without-PACKAGE\n-# * Set PACKAGEDIR=PATH for --with-PACKAGE=PATH\n-# * Unset NO_PACKAGE for --with-PACKAGE without ARG\n-AC_DEFUN([GIT_PARSE_WITH],\n-    [m4_pushdef([GIT_UC_PACKAGE], m4_toupper([$1]))dnl\n-    if test \"$withval\" = \"no\"; then\n-\tNO_[]GIT_UC_PACKAGE=YesPlease\n-    elif test \"$withval\" = \"yes\"; then\n-\tNO_[]GIT_UC_PACKAGE=\n-    else\n-\tNO_[]GIT_UC_PACKAGE=\n-\tGIT_UC_PACKAGE[]DIR=$withval\n-\tAC_MSG_NOTICE([Setting GIT_UC_PACKAGE[]DIR to $withval])\n-\tGIT_CONF_SUBST(GIT_UC_PACKAGE[DIR], [$withval])\n-    fi\n-    m4_popdef([GIT_UC_PACKAGE])])\n-\n-# GIT_PARSE_WITH_SET_MAKE_VAR(WITHNAME, VAR, HELP_TEXT)\n-# -----------------------------------------------------\n-# Set VAR to the value specied by --with-WITHNAME.\n-# No verification of arguments is performed, but warnings are issued\n-# if either 'yes' or 'no' is specified.\n-# HELP_TEXT is presented when --help is called.\n-# This is a direct way to allow setting variables in the Makefile.\n-AC_DEFUN([GIT_PARSE_WITH_SET_MAKE_VAR],\n-[AC_ARG_WITH([$1],\n- [AS_HELP_STRING([--with-$1=VALUE], $3)],\n- if test -n \"$withval\"; then\n-  if test \"$withval\" = \"yes\" -o \"$withval\" = \"no\"; then\n-    AC_MSG_WARN([You likely do not want either 'yes' or 'no' as]\n-\t\t     [a value for $1 ($2).  Maybe you do...?])\n-  fi\n-  AC_MSG_NOTICE([Setting $2 to $withval])\n-  GIT_CONF_SUBST([$2], [$withval])\n- fi)])# GIT_PARSE_WITH_SET_MAKE_VAR\n-\n-#\n-# GIT_CHECK_FUNC(FUNCTION, IFTRUE, IFFALSE)\n-# -----------------------------------------\n-# Similar to AC_CHECK_FUNC, but on systems that do not generate\n-# warnings for missing prototypes (e.g. FreeBSD when compiling without\n-# -Wall), it does not work.  By looking for function definition in\n-# libraries, this problem can be worked around.\n-AC_DEFUN([GIT_CHECK_FUNC],[AC_CHECK_FUNC([$1],[\n-  AC_SEARCH_LIBS([$1],,\n-  [$2],[$3])\n-],[$3])])\n-\n-#\n-# GIT_STASH_FLAGS(BASEPATH_VAR)\n-# -----------------------------\n-# Allow for easy stashing of LDFLAGS and CPPFLAGS before running\n-# tests that may want to take user settings into account.\n-AC_DEFUN([GIT_STASH_FLAGS],[\n-if test -n \"$1\"; then\n-   old_CPPFLAGS=\"$CPPFLAGS\"\n-   old_LDFLAGS=\"$LDFLAGS\"\n-   CPPFLAGS=\"-I$1/include $CPPFLAGS\"\n-   LDFLAGS=\"-L$1/$lib $LDFLAGS\"\n-fi\n-])\n-\n-dnl\n-dnl GIT_UNSTASH_FLAGS(BASEPATH_VAR)\n-dnl -----------------------------\n-dnl Restore the stashed *FLAGS values.\n-AC_DEFUN([GIT_UNSTASH_FLAGS],[\n-if test -n \"$1\"; then\n-   CPPFLAGS=\"$old_CPPFLAGS\"\n-   LDFLAGS=\"$old_LDFLAGS\"\n-fi\n-])\n-\n ## Configure body starts here.\n \n AC_PREREQ(2.59)\n@@ -146,6 +8,16 @@ AC_INIT([git], [@@GIT_VERSION@@], [git@vger.kernel.org])\n \n AC_CONFIG_SRCDIR([git.c])\n \n+m4_include([m4/git_arg_set_path.m4])\n+m4_include([m4/git_check_func.m4])\n+m4_include([m4/git_conf_append_path.m4])\n+m4_include([m4/git_conf_subst_init.m4])\n+m4_include([m4/git_conf_subst.m4])\n+m4_include([m4/git_parse_with.m4])\n+m4_include([m4/git_parse_with_set_make_var.m4])\n+m4_include([m4/git_stash_flags.m4])\n+m4_include([m4/git_unstash_flags.m4])\n+\n config_file=config.mak.autogen\n config_in=config.mak.in\n \ndiff --git a/m4/git_arg_set_path.m4 b/m4/git_arg_set_path.m4\nnew file mode 100644\nindex 0000000..112a52f\n--- /dev/null\n+++ b/m4/git_arg_set_path.m4\n@@ -0,0 +1,14 @@\n+\n+## Definitions of git private macro.\n+\n+# GIT_ARG_SET_PATH(PROGRAM)\n+# -------------------------\n+# Provide --with-PROGRAM=PATH option to set PATH to PROGRAM\n+# Optional second argument allows setting NO_PROGRAM=YesPlease if\n+# --without-PROGRAM version used.\n+AC_DEFUN([GIT_ARG_SET_PATH],\n+    [AC_ARG_WITH([$1],\n+        [AS_HELP_STRING([--with-$1=PATH],\n+                        [provide PATH to $1])],\n+        [GIT_CONF_APPEND_PATH([$1], [$2])],\n+        [])])\ndiff --git a/m4/git_check_func.m4 b/m4/git_check_func.m4\nnew file mode 100644\nindex 0000000..7ea71d8\n--- /dev/null\n+++ b/m4/git_check_func.m4\n@@ -0,0 +1,13 @@\n+## Definitions of git private macro.\n+\n+#\n+# GIT_CHECK_FUNC(FUNCTION, IFTRUE, IFFALSE)\n+# -----------------------------------------\n+# Similar to AC_CHECK_FUNC, but on systems that do not generate\n+# warnings for missing prototypes (e.g. FreeBSD when compiling without\n+# -Wall), it does not work.  By looking for function definition in\n+# libraries, this problem can be worked around.\n+AC_DEFUN([GIT_CHECK_FUNC],[AC_CHECK_FUNC([$1],[\n+  AC_SEARCH_LIBS([$1],,\n+  [$2],[$3])\n+],[$3])])\ndiff --git a/m4/git_conf_append_path.m4 b/m4/git_conf_append_path.m4\nnew file mode 100644\nindex 0000000..e9e3cc8\n--- /dev/null\n+++ b/m4/git_conf_append_path.m4\n@@ -0,0 +1,30 @@\n+\n+## Definitions of git private macro.\n+\n+# GIT_CONF_APPEND_PATH(PROGRAM)\n+# -----------------------------\n+# Parse --with-PROGRAM=PATH option to set PROGRAM_PATH=PATH\n+# Used by GIT_ARG_SET_PATH(PROGRAM)\n+# Optional second argument allows setting NO_PROGRAM=YesPlease if\n+# --without-PROGRAM is used.\n+AC_DEFUN([GIT_CONF_APPEND_PATH],\n+    [m4_pushdef([GIT_UC_PROGRAM], m4_toupper([$1]))dnl\n+    if test \"$withval\" = \"no\"; then\n+\tif test -n \"$2\"; then\n+\t\tGIT_UC_PROGRAM[]_PATH=$withval\n+\t\tAC_MSG_NOTICE([Disabling use of GIT_UC_PROGRAM])\n+\t\tGIT_CONF_SUBST([NO_]GIT_UC_PROGRAM, [YesPlease])\n+\t\tGIT_CONF_SUBST(GIT_UC_PROGRAM[]_PATH, [])\n+\telse\n+\t\tAC_MSG_ERROR([You cannot use git without $1])\n+\tfi\n+    else\n+\tif test \"$withval\" = \"yes\"; then\n+\t\tAC_MSG_WARN([You should provide path for --with-$1=PATH])\n+\telse\n+\t\tGIT_UC_PROGRAM[]_PATH=$withval\n+\t\tAC_MSG_NOTICE([Setting GIT_UC_PROGRAM[]_PATH to $withval])\n+\t\tGIT_CONF_SUBST(GIT_UC_PROGRAM[]_PATH, [$withval])\n+\tfi\n+    fi\n+    m4_popdef([GIT_UC_PROGRAM])])\ndiff --git a/m4/git_conf_subst.m4 b/m4/git_conf_subst.m4\nnew file mode 100644\nindex 0000000..c625c81\n--- /dev/null\n+++ b/m4/git_conf_subst.m4\n@@ -0,0 +1,10 @@\n+\n+## Definitions of git private macro.\n+\n+# GIT_CONF_SUBST(VAL, VAR)\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}dnl\n+$1=m4_if([$#],[1],[${$1}],[$2])\"])\ndiff --git a/m4/git_conf_subst_init.m4 b/m4/git_conf_subst_init.m4\nnew file mode 100644\nindex 0000000..febf496\n--- /dev/null\n+++ b/m4/git_conf_subst_init.m4\n@@ -0,0 +1,15 @@\n+\n+## Definitions of git private macro.\n+\n+# GIT_CONF_SUBST_INIT\n+# -------------------\n+# Prepare shell variables and autoconf machine required by later calls\n+# to GIT_CONF_SUBST.\n+AC_DEFUN([GIT_CONF_SUBST_INIT],\n+    [config_appended_defs=; newline='\n+'\n+    AC_CONFIG_COMMANDS([$config_file],\n+                       [echo \"$config_appended_defs\" >> \"$config_file\"],\n+                       [config_file=$config_file\n+                        config_appended_defs=\"$config_appended_defs\"])])\n+\ndiff --git a/m4/git_parse_with.m4 b/m4/git_parse_with.m4\nnew file mode 100644\nindex 0000000..67866ee\n--- /dev/null\n+++ b/m4/git_parse_with.m4\n@@ -0,0 +1,22 @@\n+\n+## Definitions of git private macro.\n+\n+# GIT_PARSE_WITH(PACKAGE)\n+# -----------------------\n+# For use in AC_ARG_WITH action-if-found, for packages default ON.\n+# * Set NO_PACKAGE=YesPlease for --without-PACKAGE\n+# * Set PACKAGEDIR=PATH for --with-PACKAGE=PATH\n+# * Unset NO_PACKAGE for --with-PACKAGE without ARG\n+AC_DEFUN([GIT_PARSE_WITH],\n+    [m4_pushdef([GIT_UC_PACKAGE], m4_toupper([$1]))dnl\n+    if test \"$withval\" = \"no\"; then\n+\tNO_[]GIT_UC_PACKAGE=YesPlease\n+    elif test \"$withval\" = \"yes\"; then\n+\tNO_[]GIT_UC_PACKAGE=\n+    else\n+\tNO_[]GIT_UC_PACKAGE=\n+\tGIT_UC_PACKAGE[]DIR=$withval\n+\tAC_MSG_NOTICE([Setting GIT_UC_PACKAGE[]DIR to $withval])\n+\tGIT_CONF_SUBST(GIT_UC_PACKAGE[DIR], [$withval])\n+    fi\n+    m4_popdef([GIT_UC_PACKAGE])])\ndiff --git a/m4/git_parse_with_set_make_var.m4 b/m4/git_parse_with_set_make_var.m4\nnew file mode 100644\nindex 0000000..68523a6\n--- /dev/null\n+++ b/m4/git_parse_with_set_make_var.m4\n@@ -0,0 +1,20 @@\n+## Definitions of git private macro.\n+\n+# GIT_PARSE_WITH_SET_MAKE_VAR(WITHNAME, VAR, HELP_TEXT)\n+# -----------------------------------------------------\n+# Set VAR to the value specied by --with-WITHNAME.\n+# No verification of arguments is performed, but warnings are issued\n+# if either 'yes' or 'no' is specified.\n+# HELP_TEXT is presented when --help is called.\n+# This is a direct way to allow setting variables in the Makefile.\n+AC_DEFUN([GIT_PARSE_WITH_SET_MAKE_VAR],\n+[AC_ARG_WITH([$1],\n+ [AS_HELP_STRING([--with-$1=VALUE], $3)],\n+ if test -n \"$withval\"; then\n+  if test \"$withval\" = \"yes\" -o \"$withval\" = \"no\"; then\n+    AC_MSG_WARN([You likely do not want either 'yes' or 'no' as]\n+\t\t     [a value for $1 ($2).  Maybe you do...?])\n+  fi\n+  AC_MSG_NOTICE([Setting $2 to $withval])\n+  GIT_CONF_SUBST([$2], [$withval])\n+ fi)])# GIT_PARSE_WITH_SET_MAKE_VAR\ndiff --git a/m4/git_stash_flags.m4 b/m4/git_stash_flags.m4\nnew file mode 100644\nindex 0000000..7719d3e\n--- /dev/null\n+++ b/m4/git_stash_flags.m4\n@@ -0,0 +1,15 @@\n+## Definitions of git private macro.\n+\n+#\n+# GIT_STASH_FLAGS(BASEPATH_VAR)\n+# -----------------------------\n+# Allow for easy stashing of LDFLAGS and CPPFLAGS before running\n+# tests that may want to take user settings into account.\n+AC_DEFUN([GIT_STASH_FLAGS],[\n+if test -n \"$1\"; then\n+   old_CPPFLAGS=\"$CPPFLAGS\"\n+   old_LDFLAGS=\"$LDFLAGS\"\n+   CPPFLAGS=\"-I$1/include $CPPFLAGS\"\n+   LDFLAGS=\"-L$1/$lib $LDFLAGS\"\n+fi\n+])\ndiff --git a/m4/git_unstash_flags.m4 b/m4/git_unstash_flags.m4\nnew file mode 100644\nindex 0000000..0a2d4b2\n--- /dev/null\n+++ b/m4/git_unstash_flags.m4\n@@ -0,0 +1,13 @@\n+## Definitions of git private macro.\n+\n+dnl\n+dnl GIT_UNSTASH_FLAGS(BASEPATH_VAR)\n+dnl -----------------------------\n+dnl Restore the stashed *FLAGS values.\n+AC_DEFUN([GIT_UNSTASH_FLAGS],[\n+if test -n \"$1\"; then\n+   CPPFLAGS=\"$old_CPPFLAGS\"\n+   LDFLAGS=\"$old_LDFLAGS\"\n+fi\n+])\n+\n-- \n1.7.9.5\n"},{"id":"227472","messageId":"5230B08D.6080109@gmail.com","threadId":"34907","inReplyTo":"1378914417-32605-1-git-send-email-gitter.spiros@gmail.com","subject":"Re: [PATCH v2] configure.ac: move the private git m4 macros to a dedicated directory","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2013-09-11T18:03:57Z","receivedAt":"2013-09-11T18:03:57Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"Hi Elia.  Sorry, but I have to give my NAK to this patch.\n\nOn 09/11/2013 04:46 PM, Elia Pinto wrote:\n> Git use, as many project that use autoconf, private m4 macros.\n>\n> When not using automake, and just relying on autoconf, the macro\n> files are not picked up by default.\n>\n> A possibility, as git do today, is to put the private m4 macro\n> in the configure.ac file, so they will copied over the final configure\n> when calling autoreconf(that call also autoconf).\n> But this makes configure.ac difficult to read and maintain,\n> especially if you want to introduce new macros later. By separating\n> the definitions of the macros from configure.ac file the build system\n> would be more modular.\n>\nIn which sense are we being more modular exactly?  After all:\n\n   - the configure.ac of Git is the only user of these macros,\n\n   - using m4_include doesn't offer any performance improvement, and\n\n   - m4 doesn't offer any namespace granularity anyway.\n\nSo it seems to me that this patch only adds extra indirections without\nadding any real benefit.\n\n> Starting from version 2.58, autoconf provide the macro AC_CONFIG_MACRO_DIR\n> to declare where additional macro files are to be put and found by aclocal.\n> The argument passed to this macro is commonly m4. Despite the documentation,\n> autoconf do nothing with it, only aclocal can use directly if invoked by\n> -I m4 or indirectly using automake. But autoreconf don't invoke aclocal\n> in this way. So in summary you can not use this macro in a useful\n> way if you only use autoconf, as git does.\n>\n> Another historical possibility is to list all your macros in acinclude.m4.\n> This file will be included in aclocal.m4 when you run aclocal, and its macro(s)\n> will henceforth be visible to autoconf. However if it contains numerous macros,\n> it will rapidly become difficult to maintain, and for git this don't provide\n> any benefits or very little.\n>\n> The actual autotool documentation recommend to write each\n> macro in its own file and gather all these files in a separate directory.\n>\nWhere exactly id you find that recommendation?  If the autotools docs tell\nto do so *unconditionally*, they are wrong and should be fixed.  In fact,\neven the configure.ac from Automake itself keeps definition of private\nmacros in configure.ac...\n\n> Given the limitations i mentioned earlier, the only possibility is to use the m4_include\n> for including every macro file. The m4_include directive works quite like the\n> #include directive of the C programming language, and simply copies over the content\n> of the file(s).\n>\n> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>\n> ---\n> This is a second version of this patch http://article.gmane.org/gmane.comp.version-control.git/231984.\n> The first was plain wrong, my bad. I am sorry for the long delay.\n> Sure it is something low-hanging fruit\n>\n>\n>   configure.ac                      |  148 +++----------------------------------\n>   m4/git_arg_set_path.m4            |   14 ++++\n>   m4/git_check_func.m4              |   13 ++++\n>   m4/git_conf_append_path.m4        |   30 ++++++++\n>   m4/git_conf_subst.m4              |   10 +++\n>   m4/git_conf_subst_init.m4         |   15 ++++\n>   m4/git_parse_with.m4              |   22 ++++++\n>   m4/git_parse_with_set_make_var.m4 |   20 +++++\n>   m4/git_stash_flags.m4             |   15 ++++\n>   m4/git_unstash_flags.m4           |   13 ++++\n>   10 files changed, 162 insertions(+), 138 deletions(-)\n>   create mode 100644 m4/git_arg_set_path.m4\n>   create mode 100644 m4/git_check_func.m4\n>   create mode 100644 m4/git_conf_append_path.m4\n>   create mode 100644 m4/git_conf_subst.m4\n>   create mode 100644 m4/git_conf_subst_init.m4\n>   create mode 100644 m4/git_parse_with.m4\n>   create mode 100644 m4/git_parse_with_set_make_var.m4\n>   create mode 100644 m4/git_stash_flags.m4\n>   create mode 100644 m4/git_unstash_flags.m4\n>\n > [SNIP]\n >\n\nRegards,\n   Stefano\n"},{"id":"227491","messageId":"CA+EOSBkwqBc+1hU_7u0KwoP7EVDszng1c2SKf4xh8YVxG0D3wA@mail.gmail.com","threadId":"34907","inReplyTo":"5230B08D.6080109@gmail.com","subject":"Re: [PATCH v2] configure.ac: move the private git m4 macros to a dedicated directory","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2013-09-11T20:05:18Z","receivedAt":"2013-09-11T20:05:18Z","isPatch":true,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"2013/9/11 Stefano Lattarini <stefano.lattarini@gmail.com>:\n> Hi Elia.  Sorry, but I have to give my NAK to this patch.\n>\nI hold in great consideration the comments of Stephen in this area.\n>\n> On 09/11/2013 04:46 PM, Elia Pinto wrote:\n>>\n>> Git use, as many project that use autoconf, private m4 macros.\n>>\n>> When not using automake, and just relying on autoconf, the macro\n>> files are not picked up by default.\n>>\n>> A possibility, as git do today, is to put the private m4 macro\n>> in the configure.ac file, so they will copied over the final configure\n>> when calling autoreconf(that call also autoconf).\n>> But this makes configure.ac difficult to read and maintain,\n>> especially if you want to introduce new macros later. By separating\n>> the definitions of the macros from configure.ac file the build system\n>> would be more modular.\n>>\n> In which sense are we being more modular exactly?  After all:\n>\n>   - the configure.ac of Git is the only user of these macros,\n>\n>   - using m4_include doesn't offer any performance improvement, and\n>\n>   - m4 doesn't offer any namespace granularity anyway.\n>\n> So it seems to me that this patch only adds extra indirections without\n> adding any real benefit.\n\nHaving a big #include have the same effect di reduced modularization,\nlarger coupling.\n\nDon't splitting header in include in general is not a good thing. If\nyou introduce new macros and start using\nit in configure.ac in the same commit it becomes more apparent the\nsignificance of their use. less probability of mistake, ecc. But it is\nmy opinion. Performarce wasn't my goal.\n>\n>\n>> Starting from version 2.58, autoconf provide the macro AC_CONFIG_MACRO_DIR\n>> to declare where additional macro files are to be put and found by\n>> aclocal.\n>> The argument passed to this macro is commonly m4. Despite the\n>> documentation,\n>> autoconf do nothing with it, only aclocal can use directly if invoked by\n>> -I m4 or indirectly using automake. But autoreconf don't invoke aclocal\n>> in this way. So in summary you can not use this macro in a useful\n>> way if you only use autoconf, as git does.\n>>\n>> Another historical possibility is to list all your macros in acinclude.m4.\n>> This file will be included in aclocal.m4 when you run aclocal, and its\n>> macro(s)\n>> will henceforth be visible to autoconf. However if it contains numerous\n>> macros,\n>> it will rapidly become difficult to maintain, and for git this don't\n>> provide\n>> any benefits or very little.\n>>\n>> The actual autotool documentation recommend to write each\n>> macro in its own file and gather all these files in a separate directory.\n>>\n> Where exactly id you find that recommendation?  If the autotools docs tell\n> to do so *unconditionally*, they are wrong and should be fixed.  In fact,\n> even the configure.ac from Automake itself keeps definition of private\n> macros in configure.ac...\n\nMaybe I misinterpreted the ufficial documentation (sure it is for\nautomake ) http://www.gnu.org/software/automake/manual/html_node/Local-Macros.html\n\nAnd this a good reference for me but it is not ufficial\nhttps://www.flameeyes.eu/autotools-mythbuster/autoconf/macros.html\n\nHowever, if an automake maintainer can not find the right the patch\ncertainly has the definitive voice.\n>\n>> Given the limitations i mentioned earlier, the only possibility is to use\n>> the m4_include\n>> for including every macro file. The m4_include directive works quite like\n>> the\n>> #include directive of the C programming language, and simply copies over\n>> the content\n>> of the file(s).\n>>\n>> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>\n>> ---\n>> This is a second version of this patch\n>> http://article.gmane.org/gmane.comp.version-control.git/231984.\n>> The first was plain wrong, my bad. I am sorry for the long delay.\n>> Sure it is something low-hanging fruit\n>>\n>>\n>>   configure.ac                      |  148\n>> +++----------------------------------\n>>   m4/git_arg_set_path.m4            |   14 ++++\n>>   m4/git_check_func.m4              |   13 ++++\n>>   m4/git_conf_append_path.m4        |   30 ++++++++\n>>   m4/git_conf_subst.m4              |   10 +++\n>>   m4/git_conf_subst_init.m4         |   15 ++++\n>>   m4/git_parse_with.m4              |   22 ++++++\n>>   m4/git_parse_with_set_make_var.m4 |   20 +++++\n>>   m4/git_stash_flags.m4             |   15 ++++\n>>   m4/git_unstash_flags.m4           |   13 ++++\n>>   10 files changed, 162 insertions(+), 138 deletions(-)\n>>   create mode 100644 m4/git_arg_set_path.m4\n>>   create mode 100644 m4/git_check_func.m4\n>>   create mode 100644 m4/git_conf_append_path.m4\n>>   create mode 100644 m4/git_conf_subst.m4\n>>   create mode 100644 m4/git_conf_subst_init.m4\n>>   create mode 100644 m4/git_parse_with.m4\n>>   create mode 100644 m4/git_parse_with_set_make_var.m4\n>>   create mode 100644 m4/git_stash_flags.m4\n>>   create mode 100644 m4/git_unstash_flags.m4\n>>\n>> [SNIP]\n>>\n>\n> Regards,\n>   Stefano\n"}]}