Re: [PATCH] build: Speed up .go compilation.

[Taylan Ulrich Bayırlı/Kammer]

Mathieu Lirzin <address@hidden> writes:

> Let me say first that I found this an amazing work!

Thanks. :-)

> address@hidden (Taylan Ulrich "Bayırlı/Kammer") writes:
>
>> -AM_V_GUILEC = $(AM_V_GUILEC_$(V))
>> -AM_V_GUILEC_ = $(AM_V_GUILEC_$(AM_DEFAULT_VERBOSITY))
>> -AM_V_GUILEC_0 = @echo "  GUILEC" $@;
>> -
>> -# Flags passed to 'guild compile'.
>> -GUILD_COMPILE_FLAGS =                               \
>> -  -Wformat -Wunbound-variable -Warity-mismatch
>> -
>
> It would make sense to me to keep compile options in Makefile.am and
> pass them as a command line argument to compile-all.scm.  Maybe renaming
> it GUILECFLAGS or GUILEC_FLAGS or GUILE_COMPILE_FLAGS would be better (I
> think Automake won't like GUILE_CFLAGS).

Hmm, the flags are a list of symbols in the Scheme code.  This has no
obvious representation in the Makefile.  I guess the closest is a
whitespace separated list of strings that would be turned into a list of
symbols.  Do you think it's worth this added complexity, or is there a
better way?

>>  # Unset 'GUILE_LOAD_COMPILED_PATH' altogether while compiling.  Otherwise, 
>> if
>>  # $GUILE_LOAD_COMPILED_PATH contains $(moduledir), we may find .go files in
>>  # there that are newer than the local .scm files (for instance because the
>> @@ -358,14 +346,16 @@ GUILD_COMPILE_FLAGS =                          \
>>  #
>>  # XXX: Use the C locale for when Guile lacks
>>  # 
>> <http://git.sv.gnu.org/cgit/guile.git/commit/?h=stable-2.0&id=e2c6bf3866d1186c60bacfbd4fe5037087ee5e3f>.
>    ^^^
>
>> -.scm.go:
>> -    $(AM_V_GUILEC)$(MKDIR_P) `dirname "$@"` ;                       \
>> +%.go: make-go ; @:
>> +make-go: $(MODULES) guix/config.scm guix/tests.scm
>> +    @echo "Compiling Scheme modules..." ;                           \
>>      unset GUILE_LOAD_COMPILED_PATH ;                                \
>>      LC_ALL=C                                                        \
>         ^^^
>
> This is present because (scripts compile) from "old" Guile doesn't do it
> automatically.  What about copying the code from the link above in
> compile-all.scm and removing this from Makefile.am ?

I should be using the whole (catch ...) expression, right?  Done, thanks
for the heads up.

>> +;;; To work around <http://bugs.gnu.org/15602> (FIXME), we want to load all
>> +;;; files to be compiled first.  We do this via resolve-interface so that 
>> the
>> +;;; top-level of each file (module) is only executed once.
>> +(define (load-module-file file)
>> +  (let ((module (file->module file)))
>> +    (format #t "  LOAD ~a~%" module)
>                          ^^^
>> +    (resolve-interface module)))
>> +
>> +(define (compile-file* file output-mutex)
>> +  (let ((go (scm->go file)))
>> +    (with-mutex output-mutex
>> +      (format #t "  GUILEC ~a~%" go)
>                              ^^^
> I know this was already aligned this way before, but IMO It would look
> cleaner to use the same alignment as Automake default silent rules (CC,
> GEN, MAKEINFO...) with 11 characters including space before ‘~A’:
>
>   (format #t "  LOAD     ~A~%" source)
>   (format #t "  GUILEC   ~A~%" source)

Done.

> Thanks,

Thanks for looking over it!

> --
> Mathieu Lirzin

Taylan