Re: [PATCH] {maint} bootstrap: add convenience make target

[Stefano Lattarini]

Hi Eric, thanks for the quick feedback.

On 04/27/2012 07:49 PM, Eric Blake wrote:
> On 04/27/2012 11:35 AM, Stefano Lattarini wrote:
>> Dependencies in the Automake build system are not completely specified
>> (see for example the commit log of recent commit 'v1.12-10-gab14841',
>> "build: avoid too greedy rebuilds in the testsuite").  In fact, some of
>> them cannot even be; for example, Makefile is generated at configure
>> time from Makefile.in, which should be regenerated by our bleeding-edge
>> automake script, which is generated by out Makefile -- specifying the
>> complete chain of dependencies here would bring to a circular dependency
>> issue.
>>
>> For this reason, before testing or deploying a change, we are often
>> forced to perform a full re-bootstrap of the Automake package, to ensure
>> all our files are actually up-to-date.  Until now, this has to be done
>> manually, thus causing wasted keystrokes and more possibilities of error.
>>
>> With this change, we introduce a new 'bootstrap' make target to
>> automatize all the (easy) steps of this re-bootstrapping (plus some
>> minor bells & whistles since we are at it).
>>
>> * GNUmakefile: Rewrite to allow an easy bootstrapping and clean rebuild
>> of the whole package, in particular with the help of ...
>> (bootstrap): ...  this new target.
>>
>> Signed-off-by: Stefano Lattarini <address@hidden>
>> ---
>>
>>   I will push this by tomorrow if there is no objection.
> 
> Question: libvirt just hit a problem where we temporarily bumped to
> requiring gettext 0.18, then backed that out to 0.17.  Unfortunately, if
> you successfully built the tree during the 0.18 window, then
> incrementally update, running './bootstrap' was still insufficient to
> clean up the damage, since 'autopoint --force' refuses to downgrade any
> installed .m4 files.  Are there any such files installed by automake's
> boostrap that need to be hand-cleaned, so an incremental bootstrap run
> will be guaranteed to install the file as if it were a fresh checkout
> instead of leaving the incremental garbage behind?
>
No, such a situation shouldn't happen with Automake.  Still, we could
render the bootstrap process even safer, by disabling the autom4te cache
for the automake build system.  Do you think that would be an improvement
or a regression?

Anyway, the best and safest way to ensure a clean rebuild remains to run
"git clean -fdx" before running bootstrap; maybe I can add an advice about
that in HACKING.

>> -# If the user runs GNU make but has not yet run ./configure,
>> -# give them an helpful diagnostic instead of a cryptic error.
>> -am--Makefile := $(wildcard Makefile)
>> -ifeq ($(am--Makefile),)
>> -  $(warning There seems to be no Makefile in this directory.)
>> -  $(warning You must run ./configure before running 'make'.)
>> -  $(error Fatal Error)
>> +ifeq ($(wildcard Makefile),)
>> +  ifeq ($(filter bootstrap,$(MAKECMDGOALS)),bootstrap)
>> +    # Allow the user (or more likely the developer) to ask for a bootstrap
>> +    # of the package; of course, this can happen before configure is run,
>> +    # and in fact even before it is created.
>> +  else
>> +    # Else, If the user runs GNU make but has not yet run ./configure,
>> +    # give them an helpful diagnostic instead of a cryptic error.
>> +    $(warning There seems to be no Makefile in this directory.)
>> +    $(warning You must run ./configure before running 'make'.)
>> +    $(error Fatal Error)
>> +  endif
>>  else
>> -  include ./Makefile
>> -  include $(srcdir)/syntax-checks.mk
>> +    include ./Makefile
>> +    include $(srcdir)/syntax-checks.mk
> 
> Why the reindentation?
>
Leftover from a previous version of the patch; will fix.

>>  endif
>> +
>> +srcdir ?= .
>> +am__cd ?= CDPATH=. && unset CDPATH && cd
>> +AM_DEFAULT_VEBOSITY ?= 0
>> +V ?= $(AM_DEFAULT_VERBOSITY)
>> +
>> +ifeq ($(V),0)
>> +  AM_V_BOOTSTRAP = @echo "  BOOTSTRAP";
>> +  AM_V_CONFIGURE = @echo "  CONFIGURE";
>> +  AM_V_REMAKE    = @echo "  REMAKE";
>> +else
>> +  AM_V_BOOTSTRAP =
>> +  AM_V_CONFIGURE =
>> +  AM_V_REMAKE    =
>> +endif
>> +
>> +# Must be phony, not to be confused with the 'bootstrap' script.
>> +.PHONY: bootstrap
>> +bootstrap:
>> +    $(AM_V_BOOTSTRAP)$(am__cd) $(srcdir) && ./bootstrap
>> +    $(AM_V_CONFIGURE)set -e; \
>> +    am__bootstrap_configure () { \
>> +      $(srcdir)/configure $${1+"$$@"} $(BOOTSTRAP_CONFIGURE_FLAGS); \
>> +    }; \
>> +    if test -f $(srcdir)/config.status; then \
>> +      : config.status should return a string properly quited for eval; \
> 
> s/quited/quoted/
>
Fixed, thanks for spotting it.

>> +      old_configure_flags=`$(srcdir)/config.status --config`; \
>> +    else \
>> +      old_configure_flags=""; \
>> +    fi; \
>> +    eval am__bootstrap_configure "$$old_configure_flags"
>> +    $(AM_V_REMAKE)$(MAKE) clean && $(MAKE) check TESTS=t/get-sysconf
> 
> The idea looks reasonable to me, although I just skimmed the patch
> rather than closely reviewing how it all works.
>
It should work correctly and well enough for now; incremental updated can
be made later, if needed.

Below is what I've squashed in my patch before pushing; note that I've
also added a couple of comments.

Regards,
  Stefano

-*-*-*-

diff --git a/GNUmakefile b/GNUmakefile
index 79cb009..402b340 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -28,10 +28,11 @@ ifeq ($(wildcard Makefile),)
     $(error Fatal Error)
   endif
 else
-    include ./Makefile
-    include $(srcdir)/syntax-checks.mk
+  include ./Makefile
+  include $(srcdir)/syntax-checks.mk
 endif

+# To allow bootstrapping also in an unconfigured tree.
 srcdir ?= .
 am__cd ?= CDPATH=. && unset CDPATH && cd
 AM_DEFAULT_VEBOSITY ?= 0
@@ -56,10 +57,12 @@ bootstrap:
          $(srcdir)/configure $${1+"$$@"} $(BOOTSTRAP_CONFIGURE_FLAGS); \
        }; \
        if test -f $(srcdir)/config.status; then \
-         : config.status should return a string properly quited for eval; \
+         : config.status should return a string properly quoted for eval; \
          old_configure_flags=`$(srcdir)/config.status --config`; \
        else \
          old_configure_flags=""; \
        fi; \
        eval am__bootstrap_configure "$$old_configure_flags"
+       # The "make check" below is to ensure all the testsuite-required
+       # files are rebuilt.
        $(AM_V_REMAKE)$(MAKE) clean && $(MAKE) check TESTS=t/get-sysconf