bug#44676: [PATCH 4/4] Support native compilation of packages on install

[Andrea Corallo]

Stefan Kangas <stefan@marxist.se> writes:

> Andrea Corallo <akrl@sdf.org> writes:
>
>>> However, perhaps we could make the LOAD parameter a simple boolean in
>>> the user-facing `native-compile-async' function?  I.e., we hide away
>>> `late' for internal use only.
>>
>> Agree that's probably the best option.
>
> How does the attached look?

Hi Stefan,

> From 6ac20230fae2b700d301cd35238b283f287a6e81 Mon Sep 17 00:00:00 2001
> From: Stefan Kangas <stefan@marxist.se>
> Date: Thu, 19 Nov 2020 22:18:50 +0100
> Subject: [PATCH 4/4] Make load argument of native-compile-async internal
>
> * lisp/emacs-lisp/comp.el (native--compile-async-internal): New
> defun extracted from native-compile-async.
> (native-compile-async): Remove load argument and use above new defun.
> * src/comp.c (maybe_defer_native_compilation): Use above new
> defun.  (Bug#44676)
> ---
>  lisp/emacs-lisp/comp.el | 20 ++++++++++++++++++--
>  src/comp.c              |  6 +++---
>  2 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el
> index 2f1e8965c1..9bebc5ff1b 100644
> --- a/lisp/emacs-lisp/comp.el
> +++ b/lisp/emacs-lisp/comp.el
> @@ -3485,8 +3485,7 @@ batch-byte-native-compile-for-bootstrap
>          (`(,tempfile . ,target-file)
>           (rename-file tempfile target-file t))))))
>  
> -;;;###autoload
> -(defun native-compile-async (paths &optional recursively load)
> +(defun native--compile-async-internal (paths &optional recursively load)

For constistency with `native--compile' and the other functions in the
file containing -- I'd go for `native--compile-async' here.

>    "Compile PATHS asynchronously.
>  PATHS is one path or a list of paths to files or directories.
>  
> @@ -3553,6 +3552,23 @@ native-compile-async
>      (when (zerop (comp-async-runnings))
>        (comp-run-async-workers))))
>  
> +;;;###autoload
> +(defun native-compile-async (paths &optional recursively load)
> +  "Compile PATHS asynchronously.
> +PATHS is one path or a list of paths to files or directories.
> +
> +If optional argument RECURSIVELY is non-nil, recurse into
> +subdirectories of given directories.
> +
> +If optional argument LOAD is non-nil, request to load the file
> +after compiling.
> +
> +The variable `comp-async-jobs-number' specifies the number
> +of (commands) to run simultaneously."
> +  ;; Normalize: we only want to pass t or nil, never e.g. 'late.
> +  (setq load (not (not load)))

Nit, for my taste I'd rather use let or &aux, or probably just put `(not
(not load))` directly in the following function call.

> +  (native--compile-async-internal paths recursively load))
> +
>  (provide 'comp)
>  
>  ;;; comp.el ends here
> diff --git a/src/comp.c b/src/comp.c
> index 6ddfad528b..89f8a4f3cd 100644
> --- a/src/comp.c
> +++ b/src/comp.c
> @@ -4677,13 +4677,13 @@ maybe_defer_native_compilation (Lisp_Object 
> function_name,
>        /* Comp already loaded.  */
>        if (!NILP (delayed_sources))
>       {
> -       CALLN (Ffuncall, intern_c_string ("native-compile-async"),
> +       CALLN (Ffuncall, intern_c_string ("native--compile-async-internal"),
>                delayed_sources, Qnil, Qlate);
>         delayed_sources = Qnil;
>       }
>        Fputhash (function_name, definition, Vcomp_deferred_pending_h);
> -      CALLN (Ffuncall, intern_c_string ("native-compile-async"), src, Qnil,
> -          Qlate);
> +      CALLN (Ffuncall, intern_c_string ("native--compile-async-internal"),
> +          src, Qnil, Qlate);
>      }
>    else
>      {

Sorry for the nit picking.  LGTM with these two.

Thanks

  Andrea