[BUG] Infinite make recursion when configure.ac changes

git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [BUG] Infinite make recursion when configure.ac changes
@ 2013-02-20  8:44 Stefano Lattarini
  2013-02-21  6:04 ` Jeff King
  0 siblings, 1 reply; 5+ messages in thread
From: Stefano Lattarini @ 2013-02-20  8:44 UTC (permalink / raw)
  To: Git List; +Cc: Martin von Zweigbergk, Jonathan Nieder, Jeff King

>From a pristine master checkout:

  $ make configure && ./configure make
  ... # All successfull
  $ touch configure.ac
  $ make
    GEN config.status
  make[1]: Entering directory `/storage/home/stefano/git/src'
    GEN config.status
  make[2]: Entering directory `/storage/home/stefano/git/src'
    GEN config.status
  make[3]: Entering directory `/storage/home/stefano/git/src'
    GEN config.status
  make[4]: Entering directory `/storage/home/stefano/git/src'
    GEN config.status
  make[5]: Entering directory `/storage/home/stefano/git/src'
    GEN config.status
  ...

and I have to hit ^C to interrupt that recursion.

This seems due to the change in commit v1.7.12.4-1-g1226504: the
issue is still present there, but no longer is in the preceding
commit 7e201053 (a.k.a. v1.7.12.4).

I haven't investigated this any further for the moment.

Regards,
  Stefano

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [BUG] Infinite make recursion when configure.ac changes
  2013-02-20  8:44 [BUG] Infinite make recursion when configure.ac changes Stefano Lattarini
@ 2013-02-21  6:04 ` Jeff King
  2013-02-21  6:10   ` Junio C Hamano
  0 siblings, 1 reply; 5+ messages in thread
From: Jeff King @ 2013-02-21  6:04 UTC (permalink / raw)
  To: Stefano Lattarini; +Cc: Git List, Martin von Zweigbergk, Jonathan Nieder

On Wed, Feb 20, 2013 at 09:44:58AM +0100, Stefano Lattarini wrote:

> From a pristine master checkout:
> 
>   $ make configure && ./configure make
>   ... # All successfull
>   $ touch configure.ac
>   $ make
>     GEN config.status
>   make[1]: Entering directory `/storage/home/stefano/git/src'
>     GEN config.status
>   make[2]: Entering directory `/storage/home/stefano/git/src'
>     GEN config.status
>   make[3]: Entering directory `/storage/home/stefano/git/src'
>     GEN config.status
>   make[4]: Entering directory `/storage/home/stefano/git/src'
>     GEN config.status
>   make[5]: Entering directory `/storage/home/stefano/git/src'
>     GEN config.status
>   ...
> 
> and I have to hit ^C to interrupt that recursion.

I can easily replicate it here.

> This seems due to the change in commit v1.7.12.4-1-g1226504: the
> issue is still present there, but no longer is in the preceding
> commit 7e201053 (a.k.a. v1.7.12.4).
> 
> I haven't investigated this any further for the moment.

Hmm. It looks like config.status depends on configure.ac. So make
rebuilds config.status, which runs "make configure", which includes
"config.mak.autogen", leading "make" to think that it should rebuild the
include file to make sure it is up to date. The include file depends on
"config.status", which needs to be rebuilt due to configure.ac, which
entails running "make configure", and so on.

So the real problem is that make things that "make configure" depends on
"config.mak.autogen" being up to date, which isn't true at all; the
whole point is to make a new configure script so we can write a new
config.mak.autogen. The simplest thing is to just avoid re-running
"make" to get the configure script; the only thing we are gaining is
that we don't have to repeat the build recipe, but we can sneak around
that by sticking it in a variable. Something like this seems to work:

diff --git a/Makefile b/Makefile
index 3b2c92c..ee1c0b0 100644
--- a/Makefile
+++ b/Makefile
@@ -1871,12 +1871,14 @@ configure: configure.ac GIT-VERSION-FILE
 	mv $@+ $@
 endif # NO_PYTHON
 
+CONFIGURE_RECIPE = $(RM) configure configure.ac+ && \
+		   sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
+			configure.ac >configure.ac+ && \
+		   autoconf -o configure configure.ac+ && \
+		   $(RM) configure.ac+
+
 configure: configure.ac GIT-VERSION-FILE
-	$(QUIET_GEN)$(RM) $@ $<+ && \
-	sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
-	    $< > $<+ && \
-	autoconf -o $@ $<+ && \
-	$(RM) $<+
+	$(QUIET_GEN)$(CONFIGURE_RECIPE)
 
 ifdef AUTOCONFIGURED
 # We avoid depending on 'configure' here, because it gets rebuilt
@@ -1885,7 +1887,7 @@ config.status: configure.ac
 # 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 && \
+	$(QUIET_GEN)$(CONFIGURE_RECIPE) && \
 	if test -f config.status; then \
 	  ./config.status --recheck; \
 	else \

-Peff

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [BUG] Infinite make recursion when configure.ac changes
  2013-02-21  6:04 ` Jeff King
@ 2013-02-21  6:10   ` Junio C Hamano
  2013-02-21  6:26     ` Jeff King
  0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2013-02-21  6:10 UTC (permalink / raw)
  To: Jeff King
  Cc: Stefano Lattarini, Git List, Martin von Zweigbergk,
	Jonathan Nieder

Jeff King <peff@peff.net> writes:

> I can easily replicate it here.
>
>> This seems due to the change in commit v1.7.12.4-1-g1226504: the
>> issue is still present there, but no longer is in the preceding
>> commit 7e201053 (a.k.a. v1.7.12.4).
>> 
>> I haven't investigated this any further for the moment.
>
> Hmm. It looks like config.status depends on configure.ac. So make
> rebuilds config.status, which runs "make configure", which includes
> "config.mak.autogen", leading "make" to think that it should rebuild the
> include file to make sure it is up to date. The include file depends on
> "config.status", which needs to be rebuilt due to configure.ac, which
> entails running "make configure", and so on.

I noticed this while looking at the other autoconf patch yesterday,
but I was otherwise occupied in the evening and did not pursue it
further.  Thanks for looking into it.

This may be an unrelated issue, but I've always thought that it was
strange and extremely unintuitive that running "make configure" once
only creates config.mak.autogen, while running it once again after
that (i.e. while having config.mak.autogen in the tree) seems to run
the resulting "./configure" as well. Maybe it is just me.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [BUG] Infinite make recursion when configure.ac changes
  2013-02-21  6:10   ` Junio C Hamano
@ 2013-02-21  6:26     ` Jeff King
  2013-02-21  7:59       ` Junio C Hamano
  0 siblings, 1 reply; 5+ messages in thread
From: Jeff King @ 2013-02-21  6:26 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Stefano Lattarini, Git List, Martin von Zweigbergk,
	Jonathan Nieder

On Wed, Feb 20, 2013 at 10:10:42PM -0800, Junio C Hamano wrote:

> I noticed this while looking at the other autoconf patch yesterday,
> but I was otherwise occupied in the evening and did not pursue it
> further.  Thanks for looking into it.

Here's the patch with a commit message. I'm pretty sure this is the
right thing to do. I was tempted to find a way of telling make "no,
don't include config.mak.autogen, but that actually _isn't_ right. It
might be defining $(RM) or some other variable that we use in the
recipe. By including it inline, we will use whatever is in the current
config.mak.autogen (that we read when we started this make invocation),
which is better than nothing.

> This may be an unrelated issue, but I've always thought that it was
> strange and extremely unintuitive that running "make configure" once
> only creates config.mak.autogen, while running it once again after
> that (i.e. while having config.mak.autogen in the tree) seems to run
> the resulting "./configure" as well. Maybe it is just me.

It's not the "configure" target that causes it to run. It's the fact
that we include config.mak.autogen, which we then tell make can be
rebuilt by running ./configure. _But_ we only tell it so if one already
exists (since otherwise everybody would get automake cruft). This is the
"ifdef AUTOCONFIGURED" you see.

So I think the rule makes sense. Once you have told the Makefile that
you are interested in autoconf (by running configure once), then it
rebuilds it as necessary, and it should be consistent. You can opt back
out of it be removing config.mak.autogen.

Anyway, here is the patch to fix the loop.

-- >8 --
Subject: [PATCH] Makefile: avoid infinite loop on configure.ac change

If you are using autoconf and change the configure.ac, the
Makefile will notice that config.status is older than
configure.ac, and will attempt to rebuild and re-run the
configure script to pick up your changes. The first step in
doing so is to run "make configure". Unfortunately, this
tries to include config.mak.autogen, which depends on
config.status, which depends on configure.ac; so we must
rebuild config.status. Which leads to us running "make
configure", and so on.

It's easy to demonstrate with:

  make configure
  ./configure
  touch configure.ac
  make

We can break this cycle by not re-invoking make to build
"configure", and instead just putting its rules inline into
our config.status rebuild procedure.  We can avoid a copy by
factoring the rules into a make variable.

Signed-off-by: Jeff King <peff@peff.net>
---
 Makefile | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 3b2c92c..ee1c0b0 100644
--- a/Makefile
+++ b/Makefile
@@ -1871,12 +1871,14 @@ configure: configure.ac GIT-VERSION-FILE
 	mv $@+ $@
 endif # NO_PYTHON

+CONFIGURE_RECIPE = $(RM) configure configure.ac+ && \
+		   sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
+			configure.ac >configure.ac+ && \
+		   autoconf -o configure configure.ac+ && \
+		   $(RM) configure.ac+
+
 configure: configure.ac GIT-VERSION-FILE
-	$(QUIET_GEN)$(RM) $@ $<+ && \
-	sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
-	    $< > $<+ && \
-	autoconf -o $@ $<+ && \
-	$(RM) $<+
+	$(QUIET_GEN)$(CONFIGURE_RECIPE)

 ifdef AUTOCONFIGURED
 # We avoid depending on 'configure' here, because it gets rebuilt
@@ -1885,7 +1887,7 @@ config.status: configure.ac
 # 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 && \
+	$(QUIET_GEN)$(CONFIGURE_RECIPE) && \
 	if test -f config.status; then \
 	  ./config.status --recheck; \
 	else \
-- 
1.8.1.4.4.g265d2fa

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [BUG] Infinite make recursion when configure.ac changes
  2013-02-21  6:26     ` Jeff King
@ 2013-02-21  7:59       ` Junio C Hamano
  0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2013-02-21  7:59 UTC (permalink / raw)
  To: Jeff King
  Cc: Stefano Lattarini, Git List, Martin von Zweigbergk,
	Jonathan Nieder

Jeff King <peff@peff.net> writes:

> Anyway, here is the patch to fix the loop.
>
> -- >8 --
> Subject: [PATCH] Makefile: avoid infinite loop on configure.ac change
>
> If you are using autoconf and change the configure.ac, the
> Makefile will notice that config.status is older than
> configure.ac, and will attempt to rebuild and re-run the
> configure script to pick up your changes. The first step in
> doing so is to run "make configure". Unfortunately, this
> tries to include config.mak.autogen, which depends on
> config.status, which depends on configure.ac; so we must
> rebuild config.status. Which leads to us running "make
> configure", and so on.
>
> It's easy to demonstrate with:
>
>   make configure
>   ./configure
>   touch configure.ac
>   make
>
> We can break this cycle by not re-invoking make to build
> "configure", and instead just putting its rules inline into
> our config.status rebuild procedure.  We can avoid a copy by
> factoring the rules into a make variable.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---

Thanks.  This needs to go to both v1.8.1.x and v1.8.2.

Will apply.

>  Makefile | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 3b2c92c..ee1c0b0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1871,12 +1871,14 @@ configure: configure.ac GIT-VERSION-FILE
>  	mv $@+ $@
>  endif # NO_PYTHON
>  
> +CONFIGURE_RECIPE = $(RM) configure configure.ac+ && \
> +		   sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
> +			configure.ac >configure.ac+ && \
> +		   autoconf -o configure configure.ac+ && \
> +		   $(RM) configure.ac+
> +
>  configure: configure.ac GIT-VERSION-FILE
> -	$(QUIET_GEN)$(RM) $@ $<+ && \
> -	sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
> -	    $< > $<+ && \
> -	autoconf -o $@ $<+ && \
> -	$(RM) $<+
> +	$(QUIET_GEN)$(CONFIGURE_RECIPE)
>  
>  ifdef AUTOCONFIGURED
>  # We avoid depending on 'configure' here, because it gets rebuilt
> @@ -1885,7 +1887,7 @@ config.status: configure.ac
>  # 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 && \
> +	$(QUIET_GEN)$(CONFIGURE_RECIPE) && \
>  	if test -f config.status; then \
>  	  ./config.status --recheck; \
>  	else \

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-02-21  7:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-20  8:44 [BUG] Infinite make recursion when configure.ac changes Stefano Lattarini
2013-02-21  6:04 ` Jeff King
2013-02-21  6:10   ` Junio C Hamano
2013-02-21  6:26     ` Jeff King
2013-02-21  7:59       ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).