git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Stefano Lattarini <stefano.lattarini@gmail.com>,
	Git List <git@vger.kernel.org>,
	Martin von Zweigbergk <martinvonz@gmail.com>,
	Jonathan Nieder <jrnieder@gmail.com>
Subject: Re: [BUG] Infinite make recursion when configure.ac changes
Date: Thu, 21 Feb 2013 01:26:14 -0500	[thread overview]
Message-ID: <20130221062614.GA8827@sigill.intra.peff.net> (raw)
In-Reply-To: <7vk3q2w61p.fsf@alter.siamese.dyndns.org>

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

  reply	other threads:[~2013-02-21  6:26 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2013-02-21  7:59       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130221062614.GA8827@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=martinvonz@gmail.com \
    --cc=stefano.lattarini@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).