Re: [PATCH v2] build: do not automatically reconfigure unless configure.ac changed

[Stefano Lattarini <stefano.lattarini@gmail.com>]

On 01/02/2013 05:50 PM, Junio C Hamano wrote:
> Stefano Lattarini <stefano.lattarini@gmail.com> writes:
> 
>> On 01/02/2013 09:48 AM, Jonathan Nieder wrote:
>>> Jeff King wrote:
>>>
>>>> It seems I am late to the party. But FWIW, this looks the most sane to
>>>> me of the patches posted in this thread.
>>> ...
>> FYI, this seems a sane approach to me....
>> The only nit I have to offer is that I'd like to see more comments in
>> the git Makefile about why this "semi-hack" is needed.
> 
> Thanks, everybody.
> 
> Please eyeball the below for (hopefully) the last time, to be
> eventually merged to maint-1.7.12, maint-1.8.0 and maint (aka
> maint-1.8.1) branches.
> 
> -- >8 --
> From: Jonathan Nieder <jrnieder@gmail.com>
> Date: Wed, 2 Jan 2013 00:25:44 -0800
> Subject: [PATCH] build: do not automatically reconfigure unless configure.ac changed
> 
> Starting with v1.7.12-rc0~4^2 (build: reconfigure automatically if
> configure.ac changes, 2012-07-19), "config.status --recheck" is
> automatically run every time the "configure" script changes.  In
> particular, that means the configuration procedure repeats whenever
> the version number changes (since the configure script changes to
> support "./configure --version" and "./configure --help"), making
> bisecting painfully slow.
> 
> The intent was to make the reconfiguration process only trigger for
> changes to configure.ac's logic.  Tweak the Makefile rule to match
> that intent by depending on configure.ac instead of configure.
> 
> Reported-by: Martin von Zweigbergk <martinvonz@gmail.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> Reviewed-by: Jeff King <peff@peff.net>
> Reviewed-by: Stefano Lattarini <stefano.lattarini@gmail.com>
> ---
>  Makefile | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 26b697d..2f5e2ab 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2167,8 +2167,14 @@ configure: configure.ac GIT-VERSION-FILE
>  	$(RM) $<+
>  
>  ifdef AUTOCONFIGURED
> -config.status: configure
> -	$(QUIET_GEN)if test -f config.status; then \
> +# We avoid depending on 'configure' here, because it gets rebuilt
> +# every time GIT-VERSION-FILE is modified, only to update the embedded
> +# version number string, which config.status does not care about.
>
Alas, config.status *do* care about it, in that the '@PACKAGE_VERSION@',
'@PACKAGE_STRING@' and '@DEFS@' substitutions are affected by what is
hard-coded in configure as the version number [1].  But if we do not
use those substitutions in any of our files (and I believe we don't),
then *we* can happily not care about the configure embedded version
number string, and thus avoid the extra configure runs.  Phew.

 [1] Yes, this is a mess.  We know.  Sorry!

> +# We
> +# do want to recheck when the platform/environment detection logic
> +# changes, hence this depends on configure.ac.
> +config.status: configure.ac
> +	$(QUIET_GEN)$(MAKE) configure && \
> +	if test -f config.status; then \
>  	  ./config.status --recheck; \
>  	else \
>  	  ./configure; \

HTH,
  Stefano