Re: [Automake-NG] [PATCH] [ng] built sources: avoid unconditional recursive make invocation

[Stefano Lattarini]

Hi Akim.

On 07/04/2012 08:53 AM, Akim Demaille wrote:
> 
> Le 3 juil. 2012 à 17:01, Stefano Lattarini a écrit :
> 
>> With this change, the times for a null-build of GNU coreutils (as
>> averaged from ten builds on an aging Debian system) drops from
>> ~3.8 seconds to ~2.6 seconds.
>>
>> With this change, the semantic of $(BUILT_SOURCES) support is slightly
>> altered, in that $(BUILT_SOURCES) will now be built not only before
>> the "all", "check" or "install" targets, but before *any* target.
>>
>> We believe that not only this change in semantics is justified by
>> the enhanced performance, but that the new semantics is actually
>> better than the old one.  So a double win.  The new and more complex
>> implementation is a price worth paying for those improvements.
>>
>> * lib/am/all-target.am: New implementation of $(BUILT_SOURCES) support,
>> using more GNU make features (mostly, automatic remake of include
>> files and automatic restart in the face of rebuilt makefiles).  This
>> new implementation should ensure that $(BUILT_SOURCES) will be built
>> before before any other target.
> 
> I know you mean very early, but "before before" might not be needed :)
>
Eh eh :-)

>> +$(foreach x,$(am.built-early),$(eval -include .am/built-sources/$(x)))
> 
> We really need to hit the file-system?
>
Well, I tried a follow-up patch like this to simplify the code an minimize
filesystem accesses:

    diff --git a/automake.in b/automake.in
    index 6e7da4b..2e330a8 100644
    --- a/automake.in
    +++ b/automake.in
    @@ -3934,8 +3934,9 @@ sub handle_install ()
     }

     # Generate "make all" and "make check" rules.
    -sub handle_all_and_check ()
    +sub handle_all_and_check ($)
     {
    +  my $makefile = basename (shift);
       my @local_headers = ();
       foreach my $spec (@config_headers)
         {
    @@ -3946,7 +3947,8 @@ sub handle_all_and_check ()

       $output_verbatim .= preprocess_file ("$libdir/am/all-target.am",
                                            'ALL-DEPS' => "@all",
    -                                       'LOCAL-HEADERS' => 
"@local_headers");
    +                                       'LOCAL-HEADERS' => "@local_headers",
    +                                       'MAKEFILE' => $makefile);

       $output_verbatim .= preprocess_file ("$libdir/am/check-target.am",
                                            'CHECK-DEPS' => "@check",
    @@ -6958,7 +6960,7 @@ sub generate_makefile ($$)

       handle_footer;
       # Special targets "all" and "check".
    -  handle_all_and_check;
    +  handle_all_and_check $makefile;

       # FIXME: Gross!
       if (var ('lib_LTLIBRARIES') && var ('bin_PROGRAMS'))
    diff --git a/lib/am/all-target.am b/lib/am/all-target.am
    index 9e69726..0816558 100644
    --- a/lib/am/all-target.am
    +++ b/lib/am/all-target.am
    @@ -32,10 +32,7 @@ ifeq ($(am__make_dryrun),true)
     # completely accurate.
     all check install: | $(am.built-early)
     else
    -$(foreach x,$(am.built-early),$(eval -include .am/built-sources/$(x)))
    -.am/built-sources/%: | %
    -   @$(am__ensure_target_dir_exists)
    -   @touch $@
    +%MAKEFILE%: | $(am.built-early)
     endif

     all-am: all-local %ALL-DEPS%

It almost works, but unfortunately it breaks the 'remake-gnulib-add-acsubst.sh'
test case, i.e., a scenario that Gnulib generated Makefiles need to support.
So no good, unless we manage to find another idiom that Gnulib can use to have
its use case still working.  Fell free to give it a shot, as I'd be happy to
apply the above simplification.

> There no way to have PHONY targets first?  About Makefile.
>
I'm not sure I'm following.  You mean having a way to ensure some target are
considered before all the other ones (apart their dependencies, of course)?
Like, in an hypothetical version of GNU make 3.83, something like this:

    .EARLY: $(BUILT_SOURCES)

> If not, then maybe it would be a nice feature request for GNU Make.
> 
>> +.am/built-sources/%: | %
> 
> Wow, order-only pattern rules...
> 
> 
> nice job!

Thanks,
  Stefano