bug#55305: 28.0.50: With async nativecomp, package manager fails to load hyperbole-autoloads.el before compilation

[Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

First, sorry for not chiming in earlier (I don't subscribe to the
emacs-bugs list, so I only see those bugs that are explicitly forwarded
to me).

Robert Weiner [2022-05-07 16:05:17] wrote:
> Tested under Emacs 28.1 and a recent tip of the Emacs git repo for Emacs
> 29 with asynchronous native compilation enabled:
>
> M-x package-install RET hyperbole RET

Hmm... I tried to reproduce it here, with `emacs -Q` this gives me
(during the normal compilation), among a bunch of lesser warnings:

    Compiling file ~/.emacs.d/elpa/hyperbole-8.0.0/test/kexport-tests.el at Sun May 15 11:01:59 2022
    kexport-tests.el:20:2: Error: Cannot open load file: Aucun fichier ou dossier de ce type, el-mock

I also noticed the following warning in *Messages*:

    hibtypes:0: Warning: Not registering prefix "pa".  Affects: ("parse-label-and-file" "pathname" "pathname-line-and-column" "patch-msg")

which points at some namespace uncleanliness in your code.
Oh, and:

    Warning: Eager macro-expansion skipped due to cycle:
      … => (load "hbut.el") => (macroexpand-all …) => (macroexpand (eval-and-compile …)) => (load "hbdata.el") => (load "hgnus.el") => (load "hvar.el") => (load "hsettings.el") => (load "hui-em-but.el") => (load "hbut.el")
    Waiting for git... [2 times]

You might wan to try and fix this one.

> fails to load the hyperbole-autoloads.el file before the
> async native compiler and byte compiler produce these errors since
> the autoloaded var:append function is not defined:

Indeed.

> Warning (comp): ~/.emacs.d/elpa/hyperbole-8.0.0/hui-em-but.el: Error:
> Symbol's function definition is void var:append Disable showing Disable
> logging

It took a bit of while to get there (many other things to
native-compile before this, apparently), but yes, I'm able to
reproduce it.

Looking at `comp-run-async-workers` in `comp.el`, I see that the async
compilation basically does:

    emacs -q -l <temp-file>

where <temp-file>'s content is basically the `expr` below:

         do (let* ((expr `((require 'comp)
                           ,(when (boundp 'backtrace-line-length)
                              `(setf backtrace-line-length ,backtrace-line-length))
                           (setf comp-file-preloaded-p ,comp-file-preloaded-p
                                 native-compile-target-directory ,native-compile-target-directory
                                 native-comp-speed ,native-comp-speed
                                 native-comp-debug ,native-comp-debug
                                 native-comp-verbose ,native-comp-verbose
                                 comp-libgccjit-reproducer ,comp-libgccjit-reproducer
                                 comp-async-compilation t
                                 native-comp-eln-load-path ',native-comp-eln-load-path
                                 native-comp-compiler-options
                                 ',native-comp-compiler-options
                                 native-comp-driver-options
                                 ',native-comp-driver-options
                                 load-path ',load-path
                                 warning-fill-column most-positive-fixnum)
                           ,native-comp-async-env-modifier-form
                           (message "Compiling %s..." ,source-file)
                           (comp--native-compile ,source-file ,(and load t))))

so the sync compilation is careful to preserve the current load-path
via:

                                 load-path ',load-path

which is why many of the files can be compiled correctly but it doesn't
load the packages's autoloads like a normal session does.

I suspect we should add a call to `package-activate-all` somewhere
in the above code (and probably preserve `package-directory-list` and
`package-user-dir` as well).

I just tried to re-trigger the problem after applying the patch below
[which also make this part of the code obey our 80-column convention,
while at it] and it appears to be fixed (e.g. `hui-em-but.el` was
successfully compiled).
Andrea, any comment?


        Stefan


diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el
index 237de52884b..aa49607462c 100644
--- a/lisp/emacs-lisp/comp.el
+++ b/lisp/emacs-lisp/comp.el
@@ -3926,22 +3926,27 @@ comp-run-async-workers
                   (file-newer-than-file-p
                    source-file (comp-el-to-eln-filename source-file)))
          do (let* ((expr `((require 'comp)
-                           ,(when (boundp 'backtrace-line-length)
-                              `(setf backtrace-line-length ,backtrace-line-length))
-                           (setf comp-file-preloaded-p ,comp-file-preloaded-p
-                                 native-compile-target-directory ,native-compile-target-directory
-                                 native-comp-speed ,native-comp-speed
-                                 native-comp-debug ,native-comp-debug
-                                 native-comp-verbose ,native-comp-verbose
-                                 comp-libgccjit-reproducer ,comp-libgccjit-reproducer
-                                 comp-async-compilation t
-                                 native-comp-eln-load-path ',native-comp-eln-load-path
-                                 native-comp-compiler-options
-                                 ',native-comp-compiler-options
-                                 native-comp-driver-options
-                                 ',native-comp-driver-options
-                                 load-path ',load-path
-                                 warning-fill-column most-positive-fixnum)
+                           (setq comp-async-compilation t)
+                           (setq warning-fill-column most-positive-fixnum)
+                           ,(let ((set (list 'setq)))
+                              (dolist (var '(comp-file-preloaded-p
+                                             native-compile-target-directory
+                                             native-comp-speed
+                                             native-comp-debug
+                                             native-comp-verbose
+                                             comp-libgccjit-reproducer
+                                             native-comp-eln-load-path
+                                             native-comp-compiler-options
+                                             native-comp-driver-options
+                                             load-path
+                                             backtrace-line-length
+                                             package-user-dir
+                                             package-directory-list))
+                                (when (boundp var)
+                                  (push var set)
+                                  (push `',(symbol-value var) set)))
+                              (nreverse set))
+                           (package-activate-all)
                            ,native-comp-async-env-modifier-form
                            (message "Compiling %s..." ,source-file)
                            (comp--native-compile ,source-file ,(and load t))))
@@ -3994,7 +3999,7 @@ comp-run-async-workers
     (run-hooks 'native-comp-async-all-done-hook)
     (with-current-buffer (get-buffer-create comp-async-buffer-name)
       (save-excursion
-        (let ((buffer-read-only nil))
+        (let ((inhibit-read-only t))
           (goto-char (point-max))
           (insert "Compilation finished.\n"))))
     ;; `comp-deferred-pending-h' should be empty at this stage.