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

[Mathieu Lirzin]

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

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).

>  # 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 ?

> +     host=$(host) srcdir="$(top_srcdir)"                             \
>       $(top_builddir)/pre-inst-env                                    \
> -     $(GUILD) compile -L "$(top_builddir)" -L "$(top_srcdir)"        \
> -       $(GUILD_COMPILE_FLAGS) --target="$(host)"                     \
> -       -o "$@" "$<"
> +     $(GUILE) -L "$(top_builddir)" -L "$(top_srcdir)"                \
> +       --no-auto-compile                                             \
> +       -s "$(top_srcdir)"/build-aux/compile-all.scm $^
>  
>  SUFFIXES = .go
>  
> @@ -457,6 +447,6 @@ assert-final-inputs-self-contained:
>       $(top_builddir)/pre-inst-env "$(GUILE)"                         \
>         "$(top_srcdir)/build-aux/check-final-inputs-self-contained.scm"
>  
> -.PHONY: sync-descriptions gen-ChangeLog gen-AUTHORS clean-go
> +.PHONY: sync-descriptions gen-ChangeLog gen-AUTHORS clean-go make-go
>  .PHONY: assert-no-store-file-names assert-binaries-available
>  .PHONY: assert-final-inputs-self-contained
> diff --git a/build-aux/compile-all.scm b/build-aux/compile-all.scm
> new file mode 100644
> index 0000000..924ec39
> --- /dev/null
> +++ b/build-aux/compile-all.scm
[...]
> +;;; 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)


Thanks,

--
Mathieu Lirzin