Straight.el: Issues with new native-comp support

[alphapapa]

As I mentioned in comments on https://github.com/raxod502/straight.el/pull/507, the changes merged there are somewhat inadvisable.

• It makes no sense to block while native compilation happens. That defeats the purpose of the asynchronous compilation that Andrea has implemented. It means minutes of waiting while Emacs is blocked for no good reason.
• A few minor improvements to comp.el would support the desired behavior in which packages would be compiled at install-time (rather than deferring compilation until load-time), while not forcing packages to be loaded when compilation finishes (unless the user has already loaded their byte-compiled forms while waiting for native compilation).
• As well, adding code for this may be premature, because symbols may be renamed at any time. I already contributed a patch to comp.el that renamed some, and some other symbols, including some referenced in that PR, should also be renamed for clarity and consistency. I would expect such renaming and refactoring to take place at least a few times before the branch is merged.

So I would generally recommend that we make the necessary, minor improvements to comp.el to support this use case (which would also benefit package.el), and that Straight should refrain from adding references to comp.el symbols until it is merged to Emacs master.

[alphapapa]

I did not intend for this to be labeled bug, but that seems to have happened automatically, and I can't change it.

[gagbo]

I don't totally understand the issue here. Is it not okay if straight provides a handle to compile AOT ?

I mean, as a user of straight.el you can also pass the :no-native-compile flag in the recipe, and then enable deferred compilation in order to enjoy asynchronous compilation happening as provided in comp.elif I understand #507 correctly

[flatwhatson]

Blocking until compilation has finished is optional, though I'll grant that the API is currently awkward. Setting straight--wait-for-async-jobs to nil will completely prevent blocking. I agree that blocking might not be desirable in the common case, so maybe this should be the default?

I added blocking because I call straight from a command-line process so that package installation never impacts my regular use of emacs. Without blocking, this process exits immediately and nothing is compiled.

As for depending on work-in-progress APIs, I think if there is breakage here it will only affect people who are a) using a bleeding-edge build of emacs and b) using the develop branch of straight. These crazy folks deserve any breakage!

[alphapapa]

I don't totally understand the issue here. Is it not okay if straight provides a handle to compile AOT ?

I don't understand why it seems so hard to communicate about this issue. I've explained it several times. Rather than explaining it for a 4th time, I refer you to my previous comments.

I added blocking because I call straight from a command-line process so that package installation never impacts my regular use of emacs. Without blocking, this process exits immediately and nothing is compiled.

That makes sense for batch mode, but not interactive mode.

As for depending on work-in-progress APIs, I think if there is breakage here it will only affect people who are a) using a bleeding-edge build of emacs and b) using the develop branch of straight. These crazy folks deserve any breakage!

I don't think anyone deserves breakage. I think code that is likely to break shouldn't be committed. Instead of doing that, as I've said 3 times, we should upstream necessary enhancements to comp.el so there won't be any breakage.

@raxod502 I don't even use Straight. I happened to see this PR on GitHub, and since I've been corresponding with Andrea and contributing to comp.el, I thought I would offer some suggestions that seem useful, to try to save you and Straight users some trouble. For some reason, communication about these issues seems extremely difficult, so I give up. I hope _you'll_ be able to understand what I mean, and then you can make whatever decision is best for Straight.

[yantar92]

As a user of native-comp branch, I confirm here that this change was surprising. I though that the breakage was in comp.el, not in straight.el. Now, after accidently finding this thread, I finally understand what is going on and why emacs hangs on startup suddenly.

I believe that default nil value for straight--wait-for-async-jobs would be better for general use.

[flatwhatson]

After some consideration I'm convinced that it's best to remove support for blocking for native compilation from straight.el. In standard usage it's better to not block, and removal of the code related to the async compilation queue should address @alphapapa's concerns about our API exposure.

[flatwhatson]

Reading through the implementation, I've finally understood your point about late loading @alphapapa. Sincerest apologies that this has taken so long to sink in, and thanks for pointing it out and offering to help.

To re-iterate, the scenario in question goes like this:

1. Byte-compile "foo.el"
2. Start async compilation of "foo.el" manually
3. Load "foo.elc" (possibly triggering deferred compilation)
4. Finish async compilation (hopefully loading the native forms)

When comp-deferred-compilation is set, this actually works as expected. Our original call to native-compile-async requests 'late loading, and deferred compilation does the same. The redundant entries to the compilation queue are ignored, but the functions are all added to comp-deferred-pending-h which marks them for native replacement.

When comp-deferred-compilation is nil, the functions are not added to comp-deferred-pending-h so the native forms are not loaded until Emacs is restarted. This is the case which could be better-supported by comp.el.

Are you planning to address this upstream, or would you rather I raise it on the bug tracker?

[raxod502]

For some reason, communication about these issues seems extremely difficult, so I give up. I hope you'll be able to understand what I mean, and then you can make whatever decision is best for Straight.

Please be nice, we are all trying our best.

But I think what you suggest is reasonable and I believe the problem is solved by https://github.com/raxod502/straight.el/pull/512. Compilation is now unconditionally asynchronous and started automatically by straight.el at installation time.

[AndreaCorallo]

Hi all,

I discovered you guys have a branch supporting `native-comp', this is just amazing!!

I've been signaled a problem using native-comp with deferred-compilation for compiling org.

I discussed what is my understanding of the situation here

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=40838#29

and added a customize that I think should help into propagating the org hack into the async workers.

I just wanted to make you aware of this, thanks!

Andrea

[andreyorst]

Can we also add eln-* to .gitignore?

Each time I update all packages I have to press d to delete this directory from straight repo, as straight thinks that there are modifications in the source tree. I see that *.elc is there, so it makes sense to ignore results of native compilation as well.

[raxod502]

Done.

[kadircancetin]

Org-roam is unstable under native compile. I used before last native compile branch with :no-native-compile t but is not working on the last version which builds caches under /eln-cache.

Installed source on archlinux : https://aur.archlinux.org/packages/emacs-native-comp-git/
I think the update 11 is broke it: http://akrl.sdf.org/gccemacs.html#org4b11ea1

(use-package org-roam
  :straight ( :no-native-compile t ))

No matter what :no-native-compile flag, It compile and load native compiled version.

Also straight-disable-native-compilation is not working.

edit: I have to disable byte-compile with :no-byte-compile t

[raxod502]

So what you're saying is enabling regular byte-compilation automatically enables native compilation as well, even if you've explicitly disabled native compilation? That would most certainly be a bug, if so.

[kadircancetin]

Oh, I actually said that but after digging the problem I think problem is comp-deferred-compilation true by default after update 11.

comp-deferred-compilation is a variable defined in ‘C source code’.

Documentation:
If non-nil compile asyncronously all .elc files
being loaded.

Once compilation happened each function definition is updated to the
native compiled one.

I think it is not about straight to fix it but it will be good on the documentation. After set the comp-deferred-compilation to nil, the problem solved.

[flatwhatson]

Yes unfortunately comp-deferred-compilation won't respect straight's :no-native-compile feature, and with deferred compilation now enabled by default it's now "broken" out-of-the-box.

We can prevent deferred compilation for particular files by adding appropriate patterns to comp-deferred-compilation-black-list, perhaps this should happen automatically for :no-native-compile packages?

[flatwhatson]

I've proposed #569 to automatically blacklist :no-native-compile packages from deferred compilation.

[raxod502]

After a delay, sorry about that, I've merged the linked pull request. Does that fix this issue?

[kadircancetin]

Sorry about my delayed response, I have moved emacs 27 so I have needed to recompile native-comp-branch and try.

Today I tried and it seems the problem is fixed on develop branch. Very thanks @flatwhatson and @raxod502. Even if there are so few people use it, your hardworks make the straight best package manager.

[raxod502]

Are there further issues to be resolved regarding native compilation, or should this issue be closed?

[flatwhatson]

I'm not aware of anything outstanding. Anything else should be raised as a separate issue.

[raxod502]

Nice :sparkles: