cc @tadfisher

I guess I'm confused; if package-native-compile is nil, shouldn't comp-enable-subr-trampolines be set to nil during package-unpack? I wouldn't expect us to have to set that explicitly.

package-native-compile is a package.el option that defaults to nil, while comp-enable-subr-trampolines is a native-comp option that defaults to t. If you have a file containing (advice-add 'top-level :before 'identity), merely loading that file anywhere will trigger a trampoline compilation in gccemacs (even in batch mode!) unless you explicitly disable it. I will probably ask the maintainer soon if comp-enable-subr-trampolines should default to nil in non-interactive mode, but I wanted to space out my emails to bug-gnu-emacs :)

To be clear, the present PR is just a workaround. Failing to compile a trampoline didn't previously result in package installation failing (as evidenced by the fact that trampolines have always failed to compile during package installation in the gccemacs Nix setup). But in current gccemacs, any error during package installation (including demoted errors that wouldn't normally result in non-zero exit codes) triggers a bug related to autoloads in gccemacs, and this one causes another error that results in a non-zero exit code when compiling git-commit recent changes in package.el caused such an error to interrupt package installation. I asked about the trampoline compilation behaviour.

Update: Changes in package.el (emacs-mirror/emacs@9973019 and emacs-mirror/emacs@7d7bfbf) made it so that certain errors (related to advices and autoloads) at package install time interrupt the installation, both in master and in feature/native-comp (I previously thought this was native-comp-specific due to a mistake in bisecting the change). The emacs-git-commit bug only manifests in emacsGcc solely because of the trampoline compilation error is native-comp-specific.

The gccemacs maintainer argues that failure to compile trampolines should be an error because it could change behaviour, and I think I agree. So for Nix I see two options:

• Disable trampoline compilation when installing packages, thus disabling the ability to advice some primitive functions at package install time. This is what this PR implements. This in theory could change package behaviour, but very likely won't.
• Have EMACSNATIVELOADPATH be set up before package installation, so compiling trampolines does not error out. This makes Nix compilation match the async compilation behaviour, but is a bit annoying because if package A adds an advice and package B requires package A, then the trampoline will (sometimes?) be compiled when installing package B. Therefore, is a package that uses advices is required by many others, we could have several copies of the compiled trampoline.

if package A adds an advice and package B requires package A, then the trampoline will (sometimes?) be compiled when installing package B.

This could be solved by appending package A's native-lisp directory after package B's in comp-eln-load-path, as the first writable directory is chosen from that list for .eln output, but all entries should be searched for .eln input. I'll give it a shot and see if that fixes the issue.

Ah, I think I see the problem now. An alternative approach could be to exercise trampoline generation by loading the package after installing it, which I would hope would generate trampolines to all advised functions called at require-time.

I was running this patch for a while, but just determined that it was causing me some trouble. The actual issue I was experiencing was that company-complete wasn't working specifically on dot characters with lsp-mode as the completion backend with Rust code and rust-analyzer as the LSP server. Completion worked in other situations, and could be made to work again by using (setq load-no-native t), but no completions were offered with a dot (or colon) character. I eventually found that if I ran eval-buffer on lsp-completion.el, then completion on the dot character would work again without needing to explicitly disable loading of native code.

There were no errors or warnings obviously pointing to missing requires when compiling lsp-mode, but in staring at all the implicated code I noticed that lsp-mode.el itself has a few top-level advice-add forms. This reminded me of pulling in this patch several weeks ago, so I just tried backing it out and no longer see the problem where completions are not being offered on a dot character. I do not know how to reliably determine if native libraries are being used, so this may well be covering up some other problem.

@acowley This is very interesting, thanks for the investigation! So if this PR causes problems, I think setting up EMACSNATIVELOADPATH at package install time is the only option. I think doing so is a good option even if it duplicates trampolines, and we can always try to remove duplication later with @tadfisher's idea.

One idea to reduce duplication would be to try this workaround Andrea suggested in an earlier bug: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=44238#8

@flatwhatson From the above bug, I assume you were doing the trampoline precompilation for Guix packaging reasons. From your experience, are we missing a solution here? We are having trouble with packages that require other packages with top-level advices, causing trampoline compilations at package install time (the infrastructure currently expects eln files to be generated only when batch-native-compileing).

@acowley By the way, do you disable deferred compilation in your init file?

@collares I've tried it both ways several times in trying to debug things. It is disabled at this moment, but I ran with it enabled yesterday. I'm happy to try variations!

I admit I don't really understand its exact impact, so whenever I notice an issue I try it both ways with a few emacs restarts each way, and keep an eye on whether or not there's an Async compilation buffer and what it's doing.