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

From: Andrea Corallo <[email-masked]>
Date: Fri, 20 Nov 2020 09:03:03 +0000

Stefan Kangas <[email-masked]> writes:

> From cd2965916119dc60e3d3457fd96662d562430e16 Mon Sep 17 00:00:00 2001
> From: Stefan Kangas <[email-masked]>
> Date: Thu, 19 Nov 2020 22:09:37 +0100
> Subject: [PATCH 2/4] Support native compilation of packages on install
>
> * lisp/emacs-lisp/package.el (package-unpack)
> (package--native-compile): Native compile packages on install, if the
> feature is available.  (Bug#44676)
> (package-native-compile): New defcustom.
> ---
>  etc/NEWS                   |  4 ++++
>  lisp/emacs-lisp/package.el | 17 +++++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/etc/NEWS b/etc/NEWS
> index 7aa5488250..8d90466312 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -823,6 +823,10 @@ equivalent to '(map (:sym sym))'.
>  
>  ** Package
>  
> +*** Native compile packages after installation.
> +This happens automatically but can be turned off by customizing the
> +user option `package-native-compile'.

This reminds me I've still not written a single NEWS entry for this
branch :/

>  +++
>  *** New commands to filter the package list.
>  The filter command key bindings are as follows:
> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index a381ca01f3..b03a7c5321 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -389,6 +389,12 @@ package-selected-packages
>    :version "25.1"
>    :type '(repeat symbol))
>  
> +(defcustom package-native-compile t
> +  "Non-nil means to native compile packages on installation."
> +  :type '(boolean)
> +  :risky t
> +  :version "28.1")

I don't have a strong feeling but I'm really not convinced this should
be t by default.  Despite personal preferences the nil equivalent of
this is what we already do for vast majority of the .el distributed with
Emacs. WDYT?

>  (defcustom package-menu-async t
>    "If non-nil, package-menu will use async operations when possible.
>  Currently, only the refreshing of archive contents supports
> @@ -968,6 +974,8 @@ package-unpack
>          ;; E.g. for multi-package installs, we should first install all packages
>          ;; and then compile them.
>          (package--compile new-desc)
> +        (when package-native-compile
> +          (package--native-compile-async new-desc))
>          ;; After compilation, load again any files loaded by
>          ;; `activate-1', so that we use the byte-compiled definitions.
>          (package--load-files-for-activation new-desc :reload)))
> @@ -1052,6 +1060,15 @@ package--compile
>          (load-path load-path))
>      (byte-recompile-directory (package-desc-dir pkg-desc) 0 t)))
>  
> +(defun package--native-compile-async (pkg-desc)
> +  "Native compile installed package PKG-DESC asynchronously.
> +This assumes that `pkg-desc' has already been activated with
> +`package-activate-1'."
> +  (when (and (featurep 'nativecomp)
> +             (native-comp-available-p))
> +    (let ((warning-minimum-level :error))
> +      (native-compile-async (package-desc-dir pkg-desc) t))))
> +
>  ;;;; Inferring package from current buffer
>  (defun package-read-from-string (str)
>    "Read a Lisp expression from STR.

Otherwise LGTM

  Andrea