build system: support automatic reconfiguration for autotools user

PATCH In review Stefano Lattarini 3 participants 31 messages Jun 15, 2016 Kbuild

Share & tools

Stefano Lattarini

author

2016-06-15 22:54 UTC #0 lore raw

cover build system: support automatic reconfiguration for autotools user

This series aims at improving the user experience for those people who (like me) use that Autotools-based interface to the build system of Git.

The two actual improvements (equipped with proper explanations and rationales) are implemented in the last two patches. The other five patches are just preparatory changes.

The series as general or as clean as it could actually be, but it's enough to scratch the itch that motivated me to write it.

Thanks,
Stefano

Stefano Lattarini (7):
autoconf: GIT_CONF_APPEND_LINE: change signature
autoconf: GIT_CONF_APPEND_LINE -> GIT_CONF_SUBST
autoconf: remove some redundant shell indirections
autoconf: remove few redundant semicolons
autoconf: use AC_CONFIG_COMMANDS instead of ad-hoc 'config.mak.append'
build: "make clean" should not remove configure-generated files
build: reconfigure automatically if configure.ac changes

 Makefile     | 17 +++++++++++++++--
 configure.ac | 53 ++++++++++++++++++++++++++++++-----------------------
 2 files changed, 45 insertions(+), 25 deletions(-)

Signature · 1 line

1.7.10.2.1067.g553d16e

Stefano Lattarini

author

2016-06-15 22:54 UTC #1 lore raw

PATCH 1/7 autoconf: GIT_CONF_APPEND_LINE: change signature

From:

GIT_CONF_APPEND_LINE([VAR=VAL])

to:

GIT_CONF_APPEND_LINE([VAR], [VAL])

This is only a preparatory change in view of future refactorings. No semantic change is intended. In fact, the generated configure file doesn't change at all.

Signed-off-by Stefano Lattarini <[email protected]>

---
 configure.ac | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

configure.ac +11 ~~-11~~

index 4e9012f..14c7960 100644
@@ -7,7 +7,7 @@
 # --------------------------
 # Append LINE to file ${config_append}
 AC_DEFUN([GIT_CONF_APPEND_LINE],
-         [echo "$1" >> "${config_append}"])
+         [echo "$1=$2" >> "${config_append}"])
 
 # GIT_ARG_SET_PATH(PROGRAM)
 # -------------------------

@@ -34,8 +34,8 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
 	if test -n "$2"; then
 		GIT_UC_PROGRAM[]_PATH=$withval
 		AC_MSG_NOTICE([Disabling use of ${PROGRAM}])
-		GIT_CONF_APPEND_LINE(NO_${PROGRAM}=YesPlease)
-		GIT_CONF_APPEND_LINE(${PROGRAM}_PATH=)
+		GIT_CONF_APPEND_LINE([NO_${PROGRAM}], [YesPlease])
+		GIT_CONF_APPEND_LINE([${PROGRAM}_PATH], [])
 	else
 		AC_MSG_ERROR([You cannot use git without $1])
 	fi

@@ -45,7 +45,7 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
 	else
 		GIT_UC_PROGRAM[]_PATH=$withval
 		AC_MSG_NOTICE([Setting GIT_UC_PROGRAM[]_PATH to $withval])
-		GIT_CONF_APPEND_LINE(${PROGRAM}_PATH=$withval)
+		GIT_CONF_APPEND_LINE([${PROGRAM}_PATH], [$withval])
 	fi
     fi
     m4_popdef([GIT_UC_PROGRAM])])

@@ -67,7 +67,7 @@ AC_DEFUN([GIT_PARSE_WITH],
 	NO_[]GIT_UC_PACKAGE=
 	GIT_UC_PACKAGE[]DIR=$withval
 	AC_MSG_NOTICE([Setting GIT_UC_PACKAGE[]DIR to $withval])
-	GIT_CONF_APPEND_LINE(${PACKAGE}DIR=$withval)
+	GIT_CONF_APPEND_LINE([${PACKAGE}DIR], [$withval])
     fi
     m4_popdef([GIT_UC_PACKAGE])])

@@ -87,7 +87,7 @@ AC_DEFUN([GIT_PARSE_WITH_SET_MAKE_VAR],
 		     [a value for $1 ($2).  Maybe you do...?])
   fi
   AC_MSG_NOTICE([Setting $2 to $withval])
-  GIT_CONF_APPEND_LINE($2=$withval)
+  GIT_CONF_APPEND_LINE([$2], [$withval])
  fi)])# GIT_PARSE_WITH_SET_MAKE_VAR
 
 #

@@ -150,7 +150,7 @@ AC_ARG_WITH([sane-tool-path],
   else
     AC_MSG_NOTICE([Setting SANE_TOOL_PATH to '$withval'])
   fi
-  GIT_CONF_APPEND_LINE([SANE_TOOL_PATH=$withval])],
+  GIT_CONF_APPEND_LINE([SANE_TOOL_PATH], [$withval])],
   [# If the "--with-sane-tool-path" option was not given, don't touch
    # SANE_TOOL_PATH here, but let defaults in Makefile take care of it.
    # This should minimize spurious differences in the behaviour of the

@@ -169,7 +169,7 @@ AC_ARG_WITH([lib],
   else
 	lib=$withval
 	AC_MSG_NOTICE([Setting lib to '$lib'])
-	GIT_CONF_APPEND_LINE(lib=$withval)
+	GIT_CONF_APPEND_LINE([lib], [$withval])
   fi])
 
 if test -z "$lib"; then

@@ -205,7 +205,7 @@ AC_ARG_ENABLE([jsmin],
 [
   JSMIN=$enableval;
   AC_MSG_NOTICE([Setting JSMIN to '$JSMIN' to enable JavaScript minifying])
-  GIT_CONF_APPEND_LINE(JSMIN=$enableval);
+  GIT_CONF_APPEND_LINE([JSMIN], [$enableval]);
 ])
 
 # Define option to enable CSS minification

@@ -215,7 +215,7 @@ AC_ARG_ENABLE([cssmin],
 [
   CSSMIN=$enableval;
   AC_MSG_NOTICE([Setting CSSMIN to '$CSSMIN' to enable CSS minifying])
-  GIT_CONF_APPEND_LINE(CSSMIN=$enableval);
+  GIT_CONF_APPEND_LINE([CSSMIN], [$enableval]);
 ])
 
 ## Site configuration (override autodetection)

@@ -256,7 +256,7 @@ AS_HELP_STRING([],           [ARG can be also prefix for libpcre library and hea
 	USE_LIBPCRE=YesPlease
 	LIBPCREDIR=$withval
 	AC_MSG_NOTICE([Setting LIBPCREDIR to $withval])
-	GIT_CONF_APPEND_LINE(LIBPCREDIR=$withval)
+	GIT_CONF_APPEND_LINE([LIBPCREDIR], [$withval])
     fi)
 #
 # Define NO_CURL if you do not have curl installed.  git-http-pull and

Signature · 1 line

1.7.10.2.1067.g553d16e

Stefano Lattarini

author

2016-06-15 22:54 UTC #2 lore raw

PATCH 2/7 autoconf: GIT_CONF_APPEND_LINE -> GIT_CONF_SUBST

In reply to Stefano Lattarini

The new name fits better with the macro signature, and underlines the similarities with the autoconf-provided macro AC_SUBST (which will be made even more pronounced in planned future commits).

Once again, no semantic change is intended, and indeed no change to the generated configure script is expected.

Signed-off-by Stefano Lattarini <[email protected]>

---
 configure.ac | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

configure.ac +12 ~~-12~~

index 14c7960..789926f 100644
@@ -3,10 +3,10 @@
 
 ## Definitions of private macros.
 
-# GIT_CONF_APPEND_LINE(LINE)
+# GIT_CONF_SUBST(LINE)
 # --------------------------
 # Append LINE to file ${config_append}
-AC_DEFUN([GIT_CONF_APPEND_LINE],
+AC_DEFUN([GIT_CONF_SUBST],
          [echo "$1=$2" >> "${config_append}"])
 
 # GIT_ARG_SET_PATH(PROGRAM)

@@ -34,8 +34,8 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
 	if test -n "$2"; then
 		GIT_UC_PROGRAM[]_PATH=$withval
 		AC_MSG_NOTICE([Disabling use of ${PROGRAM}])
-		GIT_CONF_APPEND_LINE([NO_${PROGRAM}], [YesPlease])
-		GIT_CONF_APPEND_LINE([${PROGRAM}_PATH], [])
+		GIT_CONF_SUBST([NO_${PROGRAM}], [YesPlease])
+		GIT_CONF_SUBST([${PROGRAM}_PATH], [])
 	else
 		AC_MSG_ERROR([You cannot use git without $1])
 	fi

@@ -45,7 +45,7 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
 	else
 		GIT_UC_PROGRAM[]_PATH=$withval
 		AC_MSG_NOTICE([Setting GIT_UC_PROGRAM[]_PATH to $withval])
-		GIT_CONF_APPEND_LINE([${PROGRAM}_PATH], [$withval])
+		GIT_CONF_SUBST([${PROGRAM}_PATH], [$withval])
 	fi
     fi
     m4_popdef([GIT_UC_PROGRAM])])

@@ -67,7 +67,7 @@ AC_DEFUN([GIT_PARSE_WITH],
 	NO_[]GIT_UC_PACKAGE=
 	GIT_UC_PACKAGE[]DIR=$withval
 	AC_MSG_NOTICE([Setting GIT_UC_PACKAGE[]DIR to $withval])
-	GIT_CONF_APPEND_LINE([${PACKAGE}DIR], [$withval])
+	GIT_CONF_SUBST([${PACKAGE}DIR], [$withval])
     fi
     m4_popdef([GIT_UC_PACKAGE])])

@@ -87,7 +87,7 @@ AC_DEFUN([GIT_PARSE_WITH_SET_MAKE_VAR],
 		     [a value for $1 ($2).  Maybe you do...?])
   fi
   AC_MSG_NOTICE([Setting $2 to $withval])
-  GIT_CONF_APPEND_LINE([$2], [$withval])
+  GIT_CONF_SUBST([$2], [$withval])
  fi)])# GIT_PARSE_WITH_SET_MAKE_VAR
 
 #

@@ -150,7 +150,7 @@ AC_ARG_WITH([sane-tool-path],
   else
     AC_MSG_NOTICE([Setting SANE_TOOL_PATH to '$withval'])
   fi
-  GIT_CONF_APPEND_LINE([SANE_TOOL_PATH], [$withval])],
+  GIT_CONF_SUBST([SANE_TOOL_PATH], [$withval])],
   [# If the "--with-sane-tool-path" option was not given, don't touch
    # SANE_TOOL_PATH here, but let defaults in Makefile take care of it.
    # This should minimize spurious differences in the behaviour of the

@@ -169,7 +169,7 @@ AC_ARG_WITH([lib],
   else
 	lib=$withval
 	AC_MSG_NOTICE([Setting lib to '$lib'])
-	GIT_CONF_APPEND_LINE([lib], [$withval])
+	GIT_CONF_SUBST([lib], [$withval])
   fi])
 
 if test -z "$lib"; then

@@ -205,7 +205,7 @@ AC_ARG_ENABLE([jsmin],
 [
   JSMIN=$enableval;
   AC_MSG_NOTICE([Setting JSMIN to '$JSMIN' to enable JavaScript minifying])
-  GIT_CONF_APPEND_LINE([JSMIN], [$enableval]);
+  GIT_CONF_SUBST([JSMIN], [$enableval]);
 ])
 
 # Define option to enable CSS minification

@@ -215,7 +215,7 @@ AC_ARG_ENABLE([cssmin],
 [
   CSSMIN=$enableval;
   AC_MSG_NOTICE([Setting CSSMIN to '$CSSMIN' to enable CSS minifying])
-  GIT_CONF_APPEND_LINE([CSSMIN], [$enableval]);
+  GIT_CONF_SUBST([CSSMIN], [$enableval]);
 ])
 
 ## Site configuration (override autodetection)

@@ -256,7 +256,7 @@ AS_HELP_STRING([],           [ARG can be also prefix for libpcre library and hea
 	USE_LIBPCRE=YesPlease
 	LIBPCREDIR=$withval
 	AC_MSG_NOTICE([Setting LIBPCREDIR to $withval])
-	GIT_CONF_APPEND_LINE([LIBPCREDIR], [$withval])
+	GIT_CONF_SUBST([LIBPCREDIR], [$withval])
     fi)
 #
 # Define NO_CURL if you do not have curl installed.  git-http-pull and

Signature · 1 line

1.7.10.2.1067.g553d16e

Junio C Hamano

2016-06-15 22:54 UTC #3 lore raw

Re: [PATCH 2/7] autoconf: GIT_CONF_APPEND_LINE -> GIT_CONF_SUBST

Quoted 17 lines · Stefano Lattarini

The new name fits better with the macro signature, and underlines the similarities with the autoconf-provided macro AC_SUBST (which will be made even more pronounced in planned future commits).
Once again, no semantic change is intended, and indeed no change to the generated configure script is expected.
Signed-off-by Stefano Lattarini <[email protected]>
---
 configure.ac | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)
configure.ac +0 -0
index 14c7960..789926f 100644

configure.ac line 6

Quoted 5 lines

configure.ac +1 -1

@@ -3,10 +3,10 @@
 
 ## Definitions of private macros.
 
-# GIT_CONF_APPEND_LINE(LINE)
+# GIT_CONF_SUBST(LINE)

I see that [PATCH 1/7] needs to be updated so that it describes the new two-argument form of GIT_CONF_APPEND_LINE(VAR, VAL), and this patch needs to be updated for GIT_CONF_SUBST() with the same.

configure.ac line 8

Quoted 2 lines

configure.ac +0 -0

 # --------------------------
 # Append LINE to file ${config_append}

Also the description definitely wants to be updated; it is no longer LINEness that matters.

Other than that, 1 & 2 looked very nice and sensible.

Stefano Lattarini

author

2016-06-15 22:54 UTC #4 lore raw

Re: [PATCH 2/7] autoconf: GIT_CONF_APPEND_LINE -> GIT_CONF_SUBST

Quoted 20 lines · Junio C Hamano

Quoted 18 lines · Stefano Lattarini
The new name fits better with the macro signature, and underlines the similarities with the autoconf-provided macro AC_SUBST (which will be made even more pronounced in planned future commits).
Once again, no semantic change is intended, and indeed no change to the generated configure script is expected.
Signed-off-by Stefano Lattarini <[email protected]>
---
 configure.ac | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)
configure.ac +0 -0
index 14c7960..789926f 100644
@@ -3,10 +3,10 @@
 

Quoted 9 lines

configure.ac line 6
Quoted 4 lines
configure.ac +1 -1
 ## Definitions of private macros.
 
-# GIT_CONF_APPEND_LINE(LINE)
+# GIT_CONF_SUBST(LINE)
I see that [PATCH 1/7] needs to be updated so that it describes the new two-argument form of GIT_CONF_APPEND_LINE(VAR, VAL), and this patch needs to be updated for GIT_CONF_SUBST() with the same.

Oops, you're right. I will fix that in the re-roll.

Quoted 8 lines

configure.ac line 8
Quoted 2 lines
configure.ac +0 -0
 # --------------------------
 # Append LINE to file ${config_append}
Also the description definitely wants to be updated; it is no longer LINEness that matters.
Other than that, 1 & 2 looked very nice and sensible.

Thanks!

Regards,
Stefano

Stefano Lattarini

author

2016-06-15 22:54 UTC #5 lore raw

PATCH 3/7 autoconf: remove some redundant shell indirections

In reply to Stefano Lattarini

They are merely useless now, but would get in the way of future changes.

No semantic change is intended.

Signed-off-by Stefano Lattarini <[email protected]>

---
 configure.ac | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

configure.ac +5 -7

index 789926f..9472f6b 100644
@@ -29,13 +29,12 @@ AC_DEFUN([GIT_ARG_SET_PATH],
 # --without-PROGRAM is used.
 AC_DEFUN([GIT_CONF_APPEND_PATH],
     [m4_pushdef([GIT_UC_PROGRAM], m4_toupper([$1]))dnl
-    PROGRAM=GIT_UC_PROGRAM
     if test "$withval" = "no"; then
 	if test -n "$2"; then
 		GIT_UC_PROGRAM[]_PATH=$withval
-		AC_MSG_NOTICE([Disabling use of ${PROGRAM}])
-		GIT_CONF_SUBST([NO_${PROGRAM}], [YesPlease])
-		GIT_CONF_SUBST([${PROGRAM}_PATH], [])
+		AC_MSG_NOTICE([Disabling use of GIT_UC_PROGRAM])
+		GIT_CONF_SUBST([NO_]GIT_UC_PROGRAM, [YesPlease])
+		GIT_CONF_SUBST(GIT_UC_PROGRAM[_PATH], [])
 	else
 		AC_MSG_ERROR([You cannot use git without $1])
 	fi

@@ -45,7 +44,7 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
 	else
 		GIT_UC_PROGRAM[]_PATH=$withval
 		AC_MSG_NOTICE([Setting GIT_UC_PROGRAM[]_PATH to $withval])
-		GIT_CONF_SUBST([${PROGRAM}_PATH], [$withval])
+		GIT_CONF_SUBST(GIT_UC_PROGRAM[_PATH], [$withval])
 	fi
     fi
     m4_popdef([GIT_UC_PROGRAM])])

@@ -58,7 +57,6 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
 # * Unset NO_PACKAGE for --with-PACKAGE without ARG
 AC_DEFUN([GIT_PARSE_WITH],
     [m4_pushdef([GIT_UC_PACKAGE], m4_toupper([$1]))dnl
-    PACKAGE=GIT_UC_PACKAGE
     if test "$withval" = "no"; then
 	NO_[]GIT_UC_PACKAGE=YesPlease
     elif test "$withval" = "yes"; then

@@ -67,7 +65,7 @@ AC_DEFUN([GIT_PARSE_WITH],
 	NO_[]GIT_UC_PACKAGE=
 	GIT_UC_PACKAGE[]DIR=$withval
 	AC_MSG_NOTICE([Setting GIT_UC_PACKAGE[]DIR to $withval])
-	GIT_CONF_SUBST([${PACKAGE}DIR], [$withval])
+	GIT_CONF_SUBST(GIT_UC_PACKAGE[DIR], [$withval])
     fi
     m4_popdef([GIT_UC_PACKAGE])])

Signature · 1 line

1.7.10.2.1067.g553d16e

Junio C Hamano

2016-06-15 22:54 UTC #6 lore raw

Re: [PATCH 3/7] autoconf: remove some redundant shell indirections

Quoted 5 lines · Stefano Lattarini

configure.ac +0 -0
index 789926f..9472f6b 100644

configure.ac AC_DEFUN([GIT_ARG_SET_PATH], · line 37

Quoted 9 lines

configure.ac +3 -0

@@ -29,13 +29,12 @@ AC_DEFUN([GIT_ARG_SET_PATH],
 # --without-PROGRAM is used.
 AC_DEFUN([GIT_CONF_APPEND_PATH],
     [m4_pushdef([GIT_UC_PROGRAM], m4_toupper([$1]))dnl
     if test "$withval" = "no"; then
 	if test -n "$2"; then
 		GIT_UC_PROGRAM[]_PATH=$withval
+		AC_MSG_NOTICE([Disabling use of GIT_UC_PROGRAM])
+		GIT_CONF_SUBST([NO_]GIT_UC_PROGRAM, [YesPlease])
+		GIT_CONF_SUBST(GIT_UC_PROGRAM[_PATH], [])

Nice.

The inconsistency between the existing one that does not quote the string that is not substituted, i.e.

	GIT_UC_PROGRAM[]_PATH=$withval

and the new one that quotes _PATH, i.e.

	GIT_CONF_SUBST(GIT_UC_PROGRAM[_PATH], [])

looks somewhat strange, though.

Stefano Lattarini

author

2016-06-15 22:54 UTC #7 lore raw

Re: [PATCH 3/7] autoconf: remove some redundant shell indirections

Quoted 4 lines · Junio C Hamano

The inconsistency between the existing one that does not quote the string that is not substituted, i.e.

Quoted 8 lines

	GIT_UC_PROGRAM[]_PATH=$withval
and the new one that quotes _PATH, i.e.
	GIT_CONF_SUBST(GIT_UC_PROGRAM[_PATH], [])
looks somewhat strange, though.

Will fix that in the re-roll.

Thanks,
Stefano

Junio C Hamano

2016-06-15 22:54 UTC #8 lore raw

Re: [PATCH 3/7] autoconf: remove some redundant shell indirections

Quoted 7 lines · Stefano Lattarini

Quoted 6 lines · Junio C Hamano
The inconsistency between the existing one that does not quote the string that is not substituted, i.e.
	GIT_UC_PROGRAM[]_PATH=$withval

Quoted 7 lines

Quoted 6 lines
and the new one that quotes _PATH, i.e.
	GIT_CONF_SUBST(GIT_UC_PROGRAM[_PATH], [])
looks somewhat strange, though.
Will fix that in the re-roll.

I see you already used the []_PATH in your reroll, and I do not think it matters either way in pracice, but I suspect that it is technically more correct to have _PATH part inside the bra-ket quotes (of course, changing the style to maximally quote like that is a totally different topic, and should be done as a separate patch, so I think your reroll is the right thing to do within the scope of this series).

Thanks.

Stefano Lattarini

author

2016-06-15 22:54 UTC #9 lore raw

Re: [PATCH 3/7] autoconf: remove some redundant shell indirections

Quoted 15 lines · Junio C Hamano

Quoted 13 lines · Stefano Lattarini
Quoted 12 lines · Junio C Hamano
The inconsistency between the existing one that does not quote the string that is not substituted, i.e.
	GIT_UC_PROGRAM[]_PATH=$withval
and the new one that quotes _PATH, i.e.
	GIT_CONF_SUBST(GIT_UC_PROGRAM[_PATH], [])
looks somewhat strange, though.

Quoted 9 lines

Quoted 1 line
Will fix that in the re-roll.
I see you already used the []_PATH in your reroll, and I do not think it matters either way in pracice, but I suspect that it is technically more correct to have _PATH part inside the bra-ket quotes (of course, changing the style to maximally quote like that is a totally different topic, and should be done as a separate patch,

Yep, that's what I thought. Glad I got it right, saved another re-roll :-)

Quoted 3 lines

so I think your reroll is the right thing to do within the scope of this series).

And as an aside, if we'd somehow got '_PATH' to be a defined macro, I say that something would be very, very wrong with the current setup (Autoconf strives to be very namespace-clean these days, apart from some old m4 builtins, which must remain available out of the 'm4_' and 'AC_' namespaces for sake of backward-compatibility).

Today, excluding few exceptions, I'd say that the main reason to properly quote macro arguments is to avoid commas or quoting characters in macro invocations or expansion, or even quoting characters themselves, to be spuriously considered as metacharacters and thus processed.

Quoted 2 lines

Thanks.

Thanks to you for the quick feedback.

Regards,
Stefano

Stefano Lattarini

author

2016-06-15 22:54 UTC #10 lore raw

PATCH 4/7 autoconf: remove few redundant semicolons

In reply to Stefano Lattarini

They are merely useless now, but would get in the way of future changes.

No semantic change is intended.

Signed-off-by Stefano Lattarini <[email protected]>

---
 configure.ac | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

configure.ac +2 -2

index 9472f6b..5fb9734 100644
@@ -203,7 +203,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], [$enableval])
 ])
 
 # Define option to enable CSS minification

@@ -213,7 +213,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], [$enableval])
 ])
 
 ## Site configuration (override autodetection)

Signature · 1 line

1.7.10.2.1067.g553d16e

Junio C Hamano

2016-06-15 22:54 UTC #11 lore raw

In reply to Stefano Lattarini

Except for miniscule nits in the the bottom two, I didn't find anything objectionable---nicely done.

Stefano Lattarini

author

2016-06-15 22:54 UTC #12 lore raw

v2 cover build system: support automatic reconfiguration for autotools user

Quoted 3 lines

Except for miniscule nits in the the bottom two, I didn't find anything objectionable---nicely done.

Thanks.

Here is the re-roll, which should address all the reported nits.

 Makefile     | 17 +++++++++++++++--
 configure.ac | 56 +++++++++++++++++++++++++++++++-------------------------
 2 files changed, 46 insertions(+), 27 deletions(-)

Signature · 1 line

1.7.10.2.1067.g553d16e

Stefano Lattarini

author

2016-06-15 22:54 UTC #13 lore raw

PATCH v2 1/7 autoconf: GIT_CONF_APPEND_LINE: change signature

From:

GIT_CONF_APPEND_LINE([VAR=VAL])

to:

GIT_CONF_APPEND_LINE([VAR], [VAL])

This is only a preparatory change in view of future refactorings. No semantic change is intended. In fact, the generated configure file doesn't change at all.

Signed-off-by Stefano Lattarini <[email protected]>

---
 configure.ac | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

configure.ac +14 ~~-14~~

index 4e9012f..5f63269 100644
@@ -3,11 +3,11 @@
 
 ## Definitions of private macros.
 
-# GIT_CONF_APPEND_LINE(LINE)
-# --------------------------
-# Append LINE to file ${config_append}
+# GIT_CONF_APPEND_LINE(VAL, VAR)
+# ------------------------------
+# Append the line "VAR=VAL" to file ${config_append}
 AC_DEFUN([GIT_CONF_APPEND_LINE],
-         [echo "$1" >> "${config_append}"])
+         [echo "$1=$2" >> "${config_append}"])
 
 # GIT_ARG_SET_PATH(PROGRAM)
 # -------------------------

@@ -34,8 +34,8 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
 	if test -n "$2"; then
 		GIT_UC_PROGRAM[]_PATH=$withval
 		AC_MSG_NOTICE([Disabling use of ${PROGRAM}])
-		GIT_CONF_APPEND_LINE(NO_${PROGRAM}=YesPlease)
-		GIT_CONF_APPEND_LINE(${PROGRAM}_PATH=)
+		GIT_CONF_APPEND_LINE([NO_${PROGRAM}], [YesPlease])
+		GIT_CONF_APPEND_LINE([${PROGRAM}_PATH], [])
 	else
 		AC_MSG_ERROR([You cannot use git without $1])
 	fi

@@ -45,7 +45,7 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
 	else
 		GIT_UC_PROGRAM[]_PATH=$withval
 		AC_MSG_NOTICE([Setting GIT_UC_PROGRAM[]_PATH to $withval])
-		GIT_CONF_APPEND_LINE(${PROGRAM}_PATH=$withval)
+		GIT_CONF_APPEND_LINE([${PROGRAM}_PATH], [$withval])
 	fi
     fi
     m4_popdef([GIT_UC_PROGRAM])])

@@ -67,7 +67,7 @@ AC_DEFUN([GIT_PARSE_WITH],
 	NO_[]GIT_UC_PACKAGE=
 	GIT_UC_PACKAGE[]DIR=$withval
 	AC_MSG_NOTICE([Setting GIT_UC_PACKAGE[]DIR to $withval])
-	GIT_CONF_APPEND_LINE(${PACKAGE}DIR=$withval)
+	GIT_CONF_APPEND_LINE([${PACKAGE}DIR], [$withval])
     fi
     m4_popdef([GIT_UC_PACKAGE])])

@@ -87,7 +87,7 @@ AC_DEFUN([GIT_PARSE_WITH_SET_MAKE_VAR],
 		     [a value for $1 ($2).  Maybe you do...?])
   fi
   AC_MSG_NOTICE([Setting $2 to $withval])
-  GIT_CONF_APPEND_LINE($2=$withval)
+  GIT_CONF_APPEND_LINE([$2], [$withval])
  fi)])# GIT_PARSE_WITH_SET_MAKE_VAR
 
 #

@@ -150,7 +150,7 @@ AC_ARG_WITH([sane-tool-path],
   else
     AC_MSG_NOTICE([Setting SANE_TOOL_PATH to '$withval'])
   fi
-  GIT_CONF_APPEND_LINE([SANE_TOOL_PATH=$withval])],
+  GIT_CONF_APPEND_LINE([SANE_TOOL_PATH], [$withval])],
   [# If the "--with-sane-tool-path" option was not given, don't touch
    # SANE_TOOL_PATH here, but let defaults in Makefile take care of it.
    # This should minimize spurious differences in the behaviour of the

@@ -169,7 +169,7 @@ AC_ARG_WITH([lib],
   else
 	lib=$withval
 	AC_MSG_NOTICE([Setting lib to '$lib'])
-	GIT_CONF_APPEND_LINE(lib=$withval)
+	GIT_CONF_APPEND_LINE([lib], [$withval])
   fi])
 
 if test -z "$lib"; then

@@ -205,7 +205,7 @@ AC_ARG_ENABLE([jsmin],
 [
   JSMIN=$enableval;
   AC_MSG_NOTICE([Setting JSMIN to '$JSMIN' to enable JavaScript minifying])
-  GIT_CONF_APPEND_LINE(JSMIN=$enableval);
+  GIT_CONF_APPEND_LINE([JSMIN], [$enableval]);
 ])
 
 # Define option to enable CSS minification

@@ -215,7 +215,7 @@ AC_ARG_ENABLE([cssmin],
 [
   CSSMIN=$enableval;
   AC_MSG_NOTICE([Setting CSSMIN to '$CSSMIN' to enable CSS minifying])
-  GIT_CONF_APPEND_LINE(CSSMIN=$enableval);
+  GIT_CONF_APPEND_LINE([CSSMIN], [$enableval]);
 ])
 
 ## Site configuration (override autodetection)

@@ -256,7 +256,7 @@ AS_HELP_STRING([],           [ARG can be also prefix for libpcre library and hea
 	USE_LIBPCRE=YesPlease
 	LIBPCREDIR=$withval
 	AC_MSG_NOTICE([Setting LIBPCREDIR to $withval])
-	GIT_CONF_APPEND_LINE(LIBPCREDIR=$withval)
+	GIT_CONF_APPEND_LINE([LIBPCREDIR], [$withval])
     fi)
 #
 # Define NO_CURL if you do not have curl installed.  git-http-pull and

Signature · 1 line

1.7.10.2.1067.g553d16e

Stefano Lattarini

author

2016-06-15 22:54 UTC #14 lore raw

PATCH v2 2/7 autoconf: GIT_CONF_APPEND_LINE -> GIT_CONF_SUBST

In reply to Stefano Lattarini

The new name fits better with the macro signature, and underlines the similarities with the autoconf-provided macro AC_SUBST (which will be made even more pronounced in planned future commits).

Once again, no semantic change is intended, and indeed no change to the generated configure script is expected.

Signed-off-by Stefano Lattarini <[email protected]>

---
 configure.ac | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

configure.ac +12 ~~-12~~

index 5f63269..02b9a49 100644
@@ -3,8 +3,8 @@
 
 ## Definitions of private macros.
 
-# GIT_CONF_APPEND_LINE(VAL, VAR)
-# ------------------------------
+# GIT_CONF_SUBST(VAL, VAR)
+# ------------------------
 # Append the line "VAR=VAL" to file ${config_append}
 AC_DEFUN([GIT_CONF_APPEND_LINE],
          [echo "$1=$2" >> "${config_append}"])

@@ -34,8 +34,8 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
 	if test -n "$2"; then
 		GIT_UC_PROGRAM[]_PATH=$withval
 		AC_MSG_NOTICE([Disabling use of ${PROGRAM}])
-		GIT_CONF_APPEND_LINE([NO_${PROGRAM}], [YesPlease])
-		GIT_CONF_APPEND_LINE([${PROGRAM}_PATH], [])
+		GIT_CONF_SUBST([NO_${PROGRAM}], [YesPlease])
+		GIT_CONF_SUBST([${PROGRAM}_PATH], [])
 	else
 		AC_MSG_ERROR([You cannot use git without $1])
 	fi

@@ -45,7 +45,7 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
 	else
 		GIT_UC_PROGRAM[]_PATH=$withval
 		AC_MSG_NOTICE([Setting GIT_UC_PROGRAM[]_PATH to $withval])
-		GIT_CONF_APPEND_LINE([${PROGRAM}_PATH], [$withval])
+		GIT_CONF_SUBST([${PROGRAM}_PATH], [$withval])
 	fi
     fi
     m4_popdef([GIT_UC_PROGRAM])])

@@ -67,7 +67,7 @@ AC_DEFUN([GIT_PARSE_WITH],
 	NO_[]GIT_UC_PACKAGE=
 	GIT_UC_PACKAGE[]DIR=$withval
 	AC_MSG_NOTICE([Setting GIT_UC_PACKAGE[]DIR to $withval])
-	GIT_CONF_APPEND_LINE([${PACKAGE}DIR], [$withval])
+	GIT_CONF_SUBST([${PACKAGE}DIR], [$withval])
     fi
     m4_popdef([GIT_UC_PACKAGE])])

@@ -87,7 +87,7 @@ AC_DEFUN([GIT_PARSE_WITH_SET_MAKE_VAR],
 		     [a value for $1 ($2).  Maybe you do...?])
   fi
   AC_MSG_NOTICE([Setting $2 to $withval])
-  GIT_CONF_APPEND_LINE([$2], [$withval])
+  GIT_CONF_SUBST([$2], [$withval])
  fi)])# GIT_PARSE_WITH_SET_MAKE_VAR
 
 #

@@ -150,7 +150,7 @@ AC_ARG_WITH([sane-tool-path],
   else
     AC_MSG_NOTICE([Setting SANE_TOOL_PATH to '$withval'])
   fi
-  GIT_CONF_APPEND_LINE([SANE_TOOL_PATH], [$withval])],
+  GIT_CONF_SUBST([SANE_TOOL_PATH], [$withval])],
   [# If the "--with-sane-tool-path" option was not given, don't touch
    # SANE_TOOL_PATH here, but let defaults in Makefile take care of it.
    # This should minimize spurious differences in the behaviour of the

@@ -169,7 +169,7 @@ AC_ARG_WITH([lib],
   else
 	lib=$withval
 	AC_MSG_NOTICE([Setting lib to '$lib'])
-	GIT_CONF_APPEND_LINE([lib], [$withval])
+	GIT_CONF_SUBST([lib], [$withval])
   fi])
 
 if test -z "$lib"; then

@@ -205,7 +205,7 @@ AC_ARG_ENABLE([jsmin],
 [
   JSMIN=$enableval;
   AC_MSG_NOTICE([Setting JSMIN to '$JSMIN' to enable JavaScript minifying])
-  GIT_CONF_APPEND_LINE([JSMIN], [$enableval]);
+  GIT_CONF_SUBST([JSMIN], [$enableval]);
 ])
 
 # Define option to enable CSS minification

@@ -215,7 +215,7 @@ AC_ARG_ENABLE([cssmin],
 [
   CSSMIN=$enableval;
   AC_MSG_NOTICE([Setting CSSMIN to '$CSSMIN' to enable CSS minifying])
-  GIT_CONF_APPEND_LINE([CSSMIN], [$enableval]);
+  GIT_CONF_SUBST([CSSMIN], [$enableval]);
 ])
 
 ## Site configuration (override autodetection)

@@ -256,7 +256,7 @@ AS_HELP_STRING([],           [ARG can be also prefix for libpcre library and hea
 	USE_LIBPCRE=YesPlease
 	LIBPCREDIR=$withval
 	AC_MSG_NOTICE([Setting LIBPCREDIR to $withval])
-	GIT_CONF_APPEND_LINE([LIBPCREDIR], [$withval])
+	GIT_CONF_SUBST([LIBPCREDIR], [$withval])
     fi)
 #
 # Define NO_CURL if you do not have curl installed.  git-http-pull and

Signature · 1 line

1.7.10.2.1067.g553d16e

Stefano Lattarini

author

2016-06-15 22:54 UTC #15 lore raw

PATCH v2 3/7 autoconf: remove some redundant shell indirections

In reply to Stefano Lattarini

They are merely useless now, but would get in the way of future changes.

No semantic change is intended.

Signed-off-by Stefano Lattarini <[email protected]>

---
 configure.ac | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

configure.ac +5 -7

index 02b9a49..200776f 100644
@@ -29,13 +29,12 @@ AC_DEFUN([GIT_ARG_SET_PATH],
 # --without-PROGRAM is used.
 AC_DEFUN([GIT_CONF_APPEND_PATH],
     [m4_pushdef([GIT_UC_PROGRAM], m4_toupper([$1]))dnl
-    PROGRAM=GIT_UC_PROGRAM
     if test "$withval" = "no"; then
 	if test -n "$2"; then
 		GIT_UC_PROGRAM[]_PATH=$withval
-		AC_MSG_NOTICE([Disabling use of ${PROGRAM}])
-		GIT_CONF_SUBST([NO_${PROGRAM}], [YesPlease])
-		GIT_CONF_SUBST([${PROGRAM}_PATH], [])
+		AC_MSG_NOTICE([Disabling use of GIT_UC_PROGRAM])
+		GIT_CONF_SUBST([NO_]GIT_UC_PROGRAM, [YesPlease])
+		GIT_CONF_SUBST(GIT_UC_PROGRAM[]_PATH, [])
 	else
 		AC_MSG_ERROR([You cannot use git without $1])
 	fi

@@ -45,7 +44,7 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
 	else
 		GIT_UC_PROGRAM[]_PATH=$withval
 		AC_MSG_NOTICE([Setting GIT_UC_PROGRAM[]_PATH to $withval])
-		GIT_CONF_SUBST([${PROGRAM}_PATH], [$withval])
+		GIT_CONF_SUBST(GIT_UC_PROGRAM[]_PATH, [$withval])
 	fi
     fi
     m4_popdef([GIT_UC_PROGRAM])])

@@ -58,7 +57,6 @@ AC_DEFUN([GIT_CONF_APPEND_PATH],
 # * Unset NO_PACKAGE for --with-PACKAGE without ARG
 AC_DEFUN([GIT_PARSE_WITH],
     [m4_pushdef([GIT_UC_PACKAGE], m4_toupper([$1]))dnl
-    PACKAGE=GIT_UC_PACKAGE
     if test "$withval" = "no"; then
 	NO_[]GIT_UC_PACKAGE=YesPlease
     elif test "$withval" = "yes"; then

@@ -67,7 +65,7 @@ AC_DEFUN([GIT_PARSE_WITH],
 	NO_[]GIT_UC_PACKAGE=
 	GIT_UC_PACKAGE[]DIR=$withval
 	AC_MSG_NOTICE([Setting GIT_UC_PACKAGE[]DIR to $withval])
-	GIT_CONF_SUBST([${PACKAGE}DIR], [$withval])
+	GIT_CONF_SUBST(GIT_UC_PACKAGE[DIR], [$withval])
     fi
     m4_popdef([GIT_UC_PACKAGE])])

Signature · 1 line

1.7.10.2.1067.g553d16e

Stefano Lattarini

author

2016-06-15 22:54 UTC #16 lore raw

PATCH v2 4/7 autoconf: remove few redundant semicolons

In reply to Stefano Lattarini

They are merely useless now, but would get in the way of future changes.

No semantic change is intended.

Signed-off-by Stefano Lattarini <[email protected]>

---
 configure.ac | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

configure.ac +2 -2

index 200776f..b453ba5 100644
@@ -203,7 +203,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], [$enableval])
 ])
 
 # Define option to enable CSS minification

@@ -213,7 +213,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], [$enableval])
 ])
 
 ## Site configuration (override autodetection)

Signature · 1 line

1.7.10.2.1067.g553d16e

Stefano Lattarini

author

2016-06-15 22:54 UTC #17 lore raw

PATCH v2 5/7 autoconf: use AC_CONFIG_COMMANDS instead of ad-hoc 'config.mak.append'

In reply to Stefano Lattarini

This will allow "./config.status --recheck; ./config.status" to work correctly as a mean of reconfiguring the tree with the same configure argument used in the previous "./configure" invocation.

Signed-off-by Stefano Lattarini <[email protected]>

---
 configure.ac | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

configure.ac +17 ~~-11~~

index b453ba5..a63fe77 100644
@@ -5,9 +5,22 @@
 
 # GIT_CONF_SUBST(VAL, VAR)
 # ------------------------
-# Append the line "VAR=VAL" to file ${config_append}
-AC_DEFUN([GIT_CONF_APPEND_LINE],
-         [echo "$1=$2" >> "${config_append}"])
+# 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"])
+
+# GIT_CONF_SUBST_INIT
+# -------------------
+# Prepare shell variables and autoconf machine required by later calls
+# to GIT_CONF_SUBST.
+AC_DEFUN([GIT_CONF_SUBST_INIT],
+    [config_appended_defs=; newline='
+'
+    AC_CONFIG_COMMANDS([$config_file],
+                       [echo "$config_appended_defs" >> "$config_file"],
+                       [config_file=$config_file
+                        config_appended_defs="$config_appended_defs"])])
 
 # GIT_ARG_SET_PATH(PROGRAM)
 # -------------------------

@@ -133,11 +146,8 @@ AC_INIT([git], [@@GIT_VERSION@@], [[email protected]])
 AC_CONFIG_SRCDIR([git.c])
 
 config_file=config.mak.autogen
-config_append=config.mak.append
 config_in=config.mak.in
 
-echo "# ${config_append}.  Generated by configure." > "${config_append}"
-
 # Directories holding "saner" versions of common or POSIX binaries.
 AC_ARG_WITH([sane-tool-path],
   [AS_HELP_STRING(

@@ -1041,9 +1051,5 @@ AC_SUBST(PTHREAD_LIBS)
 AC_SUBST(NO_PTHREADS)
 
 ## Output files
-AC_CONFIG_FILES(["${config_file}":"${config_in}":"${config_append}"])
+AC_CONFIG_FILES(["${config_file}":"${config_in}"])
 AC_OUTPUT
-
-
-## Cleanup
-rm -f "${config_append}"

Signature · 1 line

1.7.10.2.1067.g553d16e

Stefano Lattarini

author

2016-06-15 22:54 UTC #18 lore raw

PATCH v2 6/7 build: "make clean" should not remove configure-generated files

In reply to Stefano Lattarini

Those filed hold variables, settings and information set by the configuration process run by './configure'; in Autotools-based build system that kind of stuff should only be removed by "make distclean". Having it removed by "make clean" is not only inconsistent, but causes real confusion for that part of the Git audience that is used to the Autotools semantics; for example, an autotools old-timer that has run:

    ./configure --prefix /opt/git

in the past, without running "make distclean" afterwards, would expect a "make install" issued after a "make clean" to rebuild and install git in '/opt/git'; but with the current behaviour, the "make clean" invocation removes (among the other things) the file 'config.mak.autogen', so that the "make install" falls back to the default prefix of '$HOME', thus installing git in the user's home directory -- definitely unexpected.

Signed-off-by Stefano Lattarini <[email protected]>

---
 Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Makefile +3 -2

index 285c660..88a76a3 100644
@@ -2742,6 +2742,9 @@ dist-doc:
 
 distclean: clean
 	$(RM) configure
+	$(RM) config.log config.status config.cache
+	$(RM) config.mak.autogen config.mak.append
+	$(RM) -r autom4te.cache
 
 profile-clean:
 	$(RM) $(addsuffix *.gcda,$(addprefix $(PROFILE_DIR)/, $(object_dirs)))

@@ -2756,8 +2759,6 @@ clean: profile-clean
 	$(RM) -r $(dep_dirs)
 	$(RM) -r po/build/
 	$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h $(ETAGS_TARGET) tags cscope*
-	$(RM) -r autom4te.cache
-	$(RM) config.log config.mak.autogen config.mak.append config.status config.cache
 	$(RM) -r $(GIT_TARNAME) .doc-tmp-dir
 	$(RM) $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
 	$(RM) $(htmldocs).tar.gz $(manpages).tar.gz

Signature · 1 line

1.7.10.2.1067.g553d16e

Stefano Lattarini

author

2016-06-15 22:54 UTC #19 lore raw

Re: [PATCH v2 6/7] build: "make clean" should not remove configure-generated files

Hi Junio.

Quoted 2 lines · Stefano Lattarini

Those filed

Oops, this should read "files", not "filed" ...

Quoted 8 lines

hold variables, settings and information set by the configuration process run by './configure'; in Autotools-based build system that kind of stuff should only be removed by "make distclean". Having it removed by "make clean" is not only inconsistent, but causes real confusion for that part of the Git audience that is used to the Autotools semantics; for example, an autotools old-timer that has run:

Quoted 8 lines

    ./configure --prefix /opt/git
in the past, without running "make distclean" afterwards, would expect a "make install" issued after a "make clean" to rebuild and install git in '/opt/git'; but with the current behaviour, the "make clean" invocation removes (among the other things) the file 'config.mak.autogen', so that the "make install"

... and here we should add "invocation":

... the "make install" invocation ...

Quoted 2 lines

falls back to the default prefix of '$HOME', thus installing git in the user's home directory -- definitely unexpected.

Can you fix those nits locally before merging to 'next', or should I send a re-roll?

Thanks, and sorry for the confusion,
Stefano

Junio C Hamano

2016-06-15 22:54 UTC #20 lore raw

Re: [PATCH v2 6/7] build: "make clean" should not remove configure-generated files

Quoted 9 lines · Stefano Lattarini

... and here we should add "invocation":
... the "make install" invocation ...
Quoted 2 lines
falls back to the default prefix of '$HOME', thus installing git in the user's home directory -- definitely unexpected.
Can you fix those nits locally before merging to 'next', or should I send a re-roll?

Too late X-<.

Stefano Lattarini

author

2016-06-15 22:54 UTC #21 lore raw

PATCH v2 7/7 build: reconfigure automatically if configure.ac changes

In reply to Stefano Lattarini

This provides a reduced but still useful sibling of the Automake's "automatic Makefile rebuild" feature. It's important to note that we take care to enable the new rules only if the tree that has already be configured with './configure', so that users relying on manual configuration won't be negatively impacted.

Signed-off-by Stefano Lattarini <[email protected]>

---
 Makefile     | 12 ++++++++++++
 configure.ac |  2 ++
 2 files changed, 14 insertions(+)

Makefile +12 -0

index 88a76a3..f4e8fac 100644
@@ -2158,6 +2158,18 @@ configure: configure.ac GIT-VERSION-FILE
 	autoconf -o $@ $<+ && \
 	$(RM) $<+
 
+ifdef AUTOCONFIGURED
+config.status: configure
+	$(QUIET_GEN)if test -f config.status; then \
+	  ./config.status --recheck; \
+	else \
+	  ./configure; \
+	fi
+reconfigure config.mak.autogen: config.status
+	$(QUIET_GEN)./config.status
+.PHONY: reconfigure # This is a convenience target.
+endif
+
 XDIFF_OBJS += xdiff/xdiffi.o
 XDIFF_OBJS += xdiff/xprepare.o
 XDIFF_OBJS += xdiff/xutils.o

configure.ac +2 -0

index a63fe77..df7e376 100644
@@ -148,6 +148,8 @@ AC_CONFIG_SRCDIR([git.c])
 config_file=config.mak.autogen
 config_in=config.mak.in
 
+GIT_CONF_SUBST([AUTOCONFIGURED], [YesPlease])
+
 # Directories holding "saner" versions of common or POSIX binaries.
 AC_ARG_WITH([sane-tool-path],
   [AS_HELP_STRING(

Signature · 1 line

1.7.10.2.1067.g553d16e

Stefano Lattarini

author

2016-06-15 22:54 UTC #22 lore raw

Re: [PATCH v2 7/7] build: reconfigure automatically if configure.ac changes

Hi Junio. I've just noticed a minor typo in the commit message ...

Quoted 5 lines · Stefano Lattarini

This provides a reduced but still useful sibling of the Automake's "automatic Makefile rebuild" feature. It's important to note that we take care to enable the new rules only if the tree that has already be

... here, it should read "been", not "be". Can you fix that locally before merging to 'next', or should I send a re-roll?

Quoted 2 lines

configured with './configure', so that users relying on manual configuration won't be negatively impacted.

Thanks,
Stefano

Stefano Lattarini

author

2016-06-15 22:54 UTC #23 lore raw

PATCH 5/7 autoconf: use AC_CONFIG_COMMANDS instead of ad-hoc 'config.mak.append'

In reply to Stefano Lattarini

This will allow "./config.status --recheck; ./config.status" to work correctly as a mean of reconfiguring the tree with the same configure argument used in the previous "./configure" invocation.

Signed-off-by Stefano Lattarini <[email protected]>

---
 configure.ac | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

configure.ac +16 -9

index 5fb9734..64eecbc 100644
@@ -7,7 +7,21 @@
 # --------------------------
 # Append LINE to file ${config_append}
 AC_DEFUN([GIT_CONF_SUBST],
-         [echo "$1=$2" >> "${config_append}"])
+   [AC_REQUIRE([GIT_CONF_SUBST_INIT])
+   config_appended_defs="$config_appended_defs${newline}$1=$2"])
+
+
+# GIT_CONF_SUBST_INIT
+# -------------------
+# Prepare shell variables and autoconf machine required by later calls
+# to GIT_CONF_SUBST.
+AC_DEFUN([GIT_CONF_SUBST_INIT], 
+    [config_appended_defs=; newline='
+'
+    AC_CONFIG_COMMANDS([$config_file],
+                       [echo "$config_appended_defs" >> "$config_file"],
+                       [config_file=$config_file
+                        config_appended_defs="$config_appended_defs"])])
 
 # GIT_ARG_SET_PATH(PROGRAM)
 # -------------------------

@@ -133,11 +147,8 @@ AC_INIT([git], [@@GIT_VERSION@@], [[email protected]])
 AC_CONFIG_SRCDIR([git.c])
 
 config_file=config.mak.autogen
-config_append=config.mak.append
 config_in=config.mak.in
 
-echo "# ${config_append}.  Generated by configure." > "${config_append}"
-
 # Directories holding "saner" versions of common or POSIX binaries.
 AC_ARG_WITH([sane-tool-path],
   [AS_HELP_STRING(

@@ -1041,9 +1052,5 @@ AC_SUBST(PTHREAD_LIBS)
 AC_SUBST(NO_PTHREADS)
 
 ## Output files
-AC_CONFIG_FILES(["${config_file}":"${config_in}":"${config_append}"])
+AC_CONFIG_FILES(["${config_file}":"${config_in}"])
 AC_OUTPUT
-
-
-## Cleanup
-rm -f "${config_append}"

Signature · 1 line

1.7.10.2.1067.g553d16e

Stefano Lattarini

author

2016-06-15 22:54 UTC #24 lore raw

PATCH 6/7 build: "make clean" should not remove configure-generated files

In reply to Stefano Lattarini

    ./configure --prefix /opt/git

Signed-off-by Stefano Lattarini <[email protected]>

---
 Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Makefile +3 -2

index 285c660..88a76a3 100644
@@ -2742,6 +2742,9 @@ dist-doc:
 
 distclean: clean
 	$(RM) configure
+	$(RM) config.log config.status config.cache
+	$(RM) config.mak.autogen config.mak.append
+	$(RM) -r autom4te.cache
 
 profile-clean:
 	$(RM) $(addsuffix *.gcda,$(addprefix $(PROFILE_DIR)/, $(object_dirs)))

@@ -2756,8 +2759,6 @@ clean: profile-clean
 	$(RM) -r $(dep_dirs)
 	$(RM) -r po/build/
 	$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h $(ETAGS_TARGET) tags cscope*
-	$(RM) -r autom4te.cache
-	$(RM) config.log config.mak.autogen config.mak.append config.status config.cache
 	$(RM) -r $(GIT_TARNAME) .doc-tmp-dir
 	$(RM) $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
 	$(RM) $(htmldocs).tar.gz $(manpages).tar.gz

Signature · 1 line

1.7.10.2.1067.g553d16e

Matthieu Moy

2016-06-15 22:54 UTC #25 lore raw

Re: [PATCH 6/7] build: "make clean" should not remove configure-generated files

Quoted 7 lines · Stefano Lattarini

for example, an autotools old-timer that has run:
    ./configure --prefix /opt/git
in the past, without running "make distclean" afterwards, would expect a "make install" issued after a "make clean" to rebuild and install git in '/opt/git';

I've been hit by that behavior once. Thanks for fixing it. The patch looks good.

Signature · 2 lines

Matthieu Moy
http://www-verimag.imag.fr/~moy/

Stefano Lattarini

author

2016-06-15 22:54 UTC #26 lore raw

Re: [PATCH 6/7] build: "make clean" should not remove configure-generated files

Quoted 4 lines · Matthieu Moy

Quoted 2 lines · Stefano Lattarini
for example, an autotools old-timer that has run:

Quoted 9 lines

Quoted 5 lines
    ./configure --prefix /opt/git
in the past, without running "make distclean" afterwards, would expect a "make install" issued after a "make clean" to rebuild and install git in '/opt/git';
I've been hit by that behavior once. Thanks for fixing it. The patch looks good.

Should I add "Acked-by: Matthieu Moy" then? (Sorry if it's a dumb question, but I'm not sure which the preferred policy is around here).

Thanks,
Stefano

Junio C Hamano

2016-06-15 22:54 UTC #27 lore raw

Re: [PATCH 6/7] build: "make clean" should not remove configure-generated files

Quoted 7 lines · Stefano Lattarini

Quoted 6 lines · Matthieu Moy
Quoted 4 lines · Stefano Lattarini
for example, an autotools old-timer that has run:
    ./configure --prefix /opt/git

Quoted 8 lines

Quoted 7 lines
Quoted 3 lines
in the past, without running "make distclean" afterwards, would expect a "make install" issued after a "make clean" to rebuild and install git in '/opt/git';
I've been hit by that behavior once. Thanks for fixing it. The patch looks good.
Should I add "Acked-by: Matthieu Moy" then?

Given that we see nothing in the ouptut from

 $ git shortlog --no-merges --author='Matthieu Moy' -- Makefile configure.ac

that remotely relates to the current state of affairs in this area, I do not think we can say Matthieu _owns_ autoconf-related part of the code in any way, so I do not think it is needed.

Thanks.

Matthieu Moy

2016-06-15 22:54 UTC #28 lore raw

Re: [PATCH 6/7] build: "make clean" should not remove configure-generated files

In reply to Stefano Lattarini

Quoted 2 lines · Stefano Lattarini

Should I add "Acked-by: Matthieu Moy" then? (Sorry if it's a dumb question, but I'm not sure which the preferred policy is around here).

Not necessarily needed if the patch is not otherwise controversial. The email discussions are usually sufficient for that. I'd say you may add it if you need to resend.

Signature · 2 lines

Matthieu Moy
http://www-verimag.imag.fr/~moy/

Stefano Lattarini

author

2016-06-15 22:54 UTC #29 lore raw

Re: [PATCH 6/7] build: "make clean" should not remove configure-generated files

Quoted 9 lines · Matthieu Moy

Quoted 2 lines · Stefano Lattarini
Should I add "Acked-by: Matthieu Moy" then? (Sorry if it's a dumb question, but I'm not sure which the preferred policy is around here).
Not necessarily needed if the patch is not otherwise controversial. The email discussions are usually sufficient for that. I'd say you may add it if you need to resend.

OK, thanks for the info. In light of this, since I've already send a re-roll without any "Acked-by:" line added, I won't bother sending a further re-roll just to add such lines.

Thanks,
Stefano

Stefano Lattarini

author

2016-06-15 22:54 UTC #30 lore raw

PATCH 7/7 build: reconfigure automatically if configure.ac changes

In reply to Stefano Lattarini

Signed-off-by Stefano Lattarini <[email protected]>

---
 Makefile     | 12 ++++++++++++
 configure.ac |  2 ++
 2 files changed, 14 insertions(+)

Makefile +12 -0

index 88a76a3..f4e8fac 100644
@@ -2158,6 +2158,18 @@ configure: configure.ac GIT-VERSION-FILE
 	autoconf -o $@ $<+ && \
 	$(RM) $<+
 
+ifdef AUTOCONFIGURED
+config.status: configure
+	$(QUIET_GEN)if test -f config.status; then \
+	  ./config.status --recheck; \
+	else \
+	  ./configure; \
+	fi
+reconfigure config.mak.autogen: config.status
+	$(QUIET_GEN)./config.status
+.PHONY: reconfigure # This is a convenience target.
+endif
+
 XDIFF_OBJS += xdiff/xdiffi.o
 XDIFF_OBJS += xdiff/xprepare.o
 XDIFF_OBJS += xdiff/xutils.o

configure.ac +2 -0

index 64eecbc..8949935 100644
@@ -149,6 +149,8 @@ AC_CONFIG_SRCDIR([git.c])
 config_file=config.mak.autogen
 config_in=config.mak.in
 
+GIT_CONF_SUBST([AUTOCONFIGURED], [YesPlease])
+
 # Directories holding "saner" versions of common or POSIX binaries.
 AC_ARG_WITH([sane-tool-path],
   [AS_HELP_STRING(

Signature · 1 line

1.7.10.2.1067.g553d16e