[libvirt] [PATCH] do not require two ./autogen.sh runs to permit "make"
I tracked down the source of the two-autogen.sh-runs-required bug. Here's the fix:
From 0583de2de2038d4f8875268391fd4994329d39bf Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Tue, 16 Mar 2010 21:08:31 +0100 Subject: [PATCH] do not require two ./autogen.sh runs to permit "make"
* autogen.sh (bootstrap_hash): New function. Running bootstrap may update the gnulib SHA1, yet we were computing t=$(git submodule status ...) *prior* to running bootstrap, and then recording that sometimes-stale value in the stamp file upon a successful bootstrap run. That would require two (lengthy!) bootstrap runs to update the stamp file. --- autogen.sh | 21 ++++++++++++++------- 1 files changed, 14 insertions(+), 7 deletions(-) diff --git a/autogen.sh b/autogen.sh index ff94678..b93cdba 100755 --- a/autogen.sh +++ b/autogen.sh @@ -62,20 +62,27 @@ else fi fi +# Compute the hash we'll use to determine whether rerunning bootstrap +# is required. The first is just the SHA1 that selects a gnulib snapshot. +# The second ensures that whenever we change the set of gnulib modules used +# by this package, we rerun bootstrap to pull in the matching set of files. +bootstrap_hash() +{ + git submodule status | sed 's/^[ +-]//;s/ .*//' + git hash-object bootstrap.conf +} + # Ensure that whenever we pull in a gnulib update or otherwise change to a # different version (i.e., when switching branches), we also rerun ./bootstrap. curr_status=.git-module-status -t=$(git submodule status|sed 's/^[ +-]//;s/ .*//'; \ - git hash-object bootstrap.conf) +t=$(bootstrap_hash) if test "$t" = "$(cat $curr_status 2>/dev/null)"; then : # good, it's up to date, all we need is autoreconf autoreconf -if else - echo running bootstrap... - ./bootstrap && echo "$t" > $curr_status || { - echo "Failed to bootstrap gnulib, please investigate." - exit 1; - } + echo running bootstrap... + ./bootstrap && bootstrap_hash > $curr_status \ + || { echo "Failed to bootstrap gnulib, please investigate."; exit 1; } fi cd "$THEDIR" -- 1.7.0.1
On Tue, Mar 16, 2010 at 09:42:32PM +0100, Jim Meyering wrote:
I tracked down the source of the two-autogen.sh-runs-required bug. Here's the fix:
From 0583de2de2038d4f8875268391fd4994329d39bf Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Tue, 16 Mar 2010 21:08:31 +0100 Subject: [PATCH] do not require two ./autogen.sh runs to permit "make"
* autogen.sh (bootstrap_hash): New function. Running bootstrap may update the gnulib SHA1, yet we were computing t=$(git submodule status ...) *prior* to running bootstrap, and then recording that sometimes-stale value in the stamp file upon a successful bootstrap run. That would require two (lengthy!) bootstrap runs to update the stamp file. --- autogen.sh | 21 ++++++++++++++------- 1 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/autogen.sh b/autogen.sh index ff94678..b93cdba 100755 --- a/autogen.sh +++ b/autogen.sh @@ -62,20 +62,27 @@ else fi fi
+# Compute the hash we'll use to determine whether rerunning bootstrap +# is required. The first is just the SHA1 that selects a gnulib snapshot. +# The second ensures that whenever we change the set of gnulib modules used +# by this package, we rerun bootstrap to pull in the matching set of files. +bootstrap_hash() +{ + git submodule status | sed 's/^[ +-]//;s/ .*//' + git hash-object bootstrap.conf +} + # Ensure that whenever we pull in a gnulib update or otherwise change to a # different version (i.e., when switching branches), we also rerun ./bootstrap. curr_status=.git-module-status -t=$(git submodule status|sed 's/^[ +-]//;s/ .*//'; \ - git hash-object bootstrap.conf) +t=$(bootstrap_hash) if test "$t" = "$(cat $curr_status 2>/dev/null)"; then : # good, it's up to date, all we need is autoreconf autoreconf -if else - echo running bootstrap... - ./bootstrap && echo "$t" > $curr_status || { - echo "Failed to bootstrap gnulib, please investigate." - exit 1; - } + echo running bootstrap... + ./bootstrap && bootstrap_hash > $curr_status \ + || { echo "Failed to bootstrap gnulib, please investigate."; exit 1; } fi
cd "$THEDIR"
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Daniel P. Berrange wrote:
On Tue, Mar 16, 2010 at 09:42:32PM +0100, Jim Meyering wrote:
I tracked down the source of the two-autogen.sh-runs-required bug. Here's the fix:
Subject: [PATCH] do not require two ./autogen.sh runs to permit "make"
* autogen.sh (bootstrap_hash): New function. Running bootstrap may update the gnulib SHA1, yet we were computing t=$(git submodule status ...) *prior* to running bootstrap, and then recording that sometimes-stale value in the stamp file upon a successful bootstrap run. That would require two (lengthy!) bootstrap runs to update the stamp file. ...[40 lines elided]...
ACK
Thanks. Pushed. Just a good-natured reminder: everyone I know prefers review feedback that removes quoted context for which there is no new comment. Even here, where the patch was small and easy to "see", omitting the 40-50 lines after the embedded Subject: or log would have made it a tiny bit easier to read.
On Wed, Mar 17, 2010 at 11:20:33AM +0100, Jim Meyering wrote:
Daniel P. Berrange wrote:
On Tue, Mar 16, 2010 at 09:42:32PM +0100, Jim Meyering wrote:
I tracked down the source of the two-autogen.sh-runs-required bug. Here's the fix:
Subject: [PATCH] do not require two ./autogen.sh runs to permit "make"
* autogen.sh (bootstrap_hash): New function. Running bootstrap may update the gnulib SHA1, yet we were computing t=$(git submodule status ...) *prior* to running bootstrap, and then recording that sometimes-stale value in the stamp file upon a successful bootstrap run. That would require two (lengthy!) bootstrap runs to update the stamp file. ...[40 lines elided]...
ACK
Thanks. Pushed.
Just a good-natured reminder: everyone I know prefers review feedback that removes quoted context for which there is no new comment. Even here, where the patch was small and easy to "see", omitting the 40-50 lines after the embedded Subject: or log would have made it a tiny bit easier to read.
Depends, when approving a patch, and if it's small (one page or so) I tend to just ACK at the end, it allows to see the context. But if it's more than one page, I don't like that too much. Basically I think that one should see some input from the replier on any page displayed, otherwise it's just lost time and space. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Tue, Mar 16, 2010 at 09:42:32PM +0100, Jim Meyering wrote:
I tracked down the source of the two-autogen.sh-runs-required bug.
Yay ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Jim Meyering