radian-software / radian

🍉 Dotfiles that marry elegance and practicality.
MIT License
491 stars 47 forks source link

Byte and native compilations issues #516

Closed haji-ali closed 1 year ago

haji-ali commented 1 year ago

I finally manged to compile radian with my config. But now I am getting the warning (I am using Emacs 28.1):

Failed to byte-compile: radian.el:2413:41: Warning: reference to free variable ‘radian-lsp-disable’

Even though it's defined here https://github.com/radian-software/radian/blob/b227bcd7b807633f12693fb0e7e3a17a0a27877c/emacs/radian.el#L2332-L2336

If I move the definition to outside the radian-use-package, everything works as expected. I believe the reason is because I have lsp-mode in radian-disabled-packages.

Another issue I am seeing is this error in *Aync-native-compile-log* when native-compiling:

In toplevel form:
radian.el:22:17: Warning: reference to free variable ‘radian-lib-file’
~/Downloads/radian/emacs/radian.el: Error: Symbol's value as variable is void radian-lib-file
Compilation finished.
Compiling ~/Downloads/radian/emacs/radian.el...
raxod502 commented 1 year ago

That's odd. Byte-compilation has a lot of weird issues and it might be a good idea to do some significant restructuring to make problems like this cause fewer issues.

In your checkout, I don't suppose there is something relevant at radian.el:2413:41, is there? For me, it just shows https://github.com/radian-software/radian/blob/b227bcd7b807633f12693fb0e7e3a17a0a27877c/emacs/radian.el#L2413

I am also afraid to say I do not know what could be causing the native compilation error. I am very open to fixes.

haji-ali commented 1 year ago

Ah sorry, I added some comments before which shifted the line-numbers, for me the line is

https://github.com/radian-software/radian/blob/b227bcd7b807633f12693fb0e7e3a17a0a27877c/emacs/radian.el#L2345-L2346

lsp-disable is used on the second line above, even though the error cites the first one.

Perhaps the reason is that radian-use-package expands the body regardless of the package being loaded or not. Since lsp-mode is in the disabled packages, radian-defhook expression is expanded with radian-lsp-disable not being defined which causes the error. I am not sure how to fix this thought.

I am also afraid to say I do not know what could be causing the native compilation error. I am very open to fixes.

Am I alone in this issue or is it working in your setup somehow? I think that the current structure is not amenable to native-compilation since the definitions are in a different file while radian.el would be compiled separately (essentially in an emacs -Q session) without any of the definitions in init.el.

This is not a super important feature to have for me (even with just byte-compilation my radian start-time is just 0.3 sec while my additional config of 7K lines is 0.16 sec). But if I am correct, it would be good to keep in mind for any future restructuring.

raxod502 commented 1 year ago

I personally have used native compilation with Emacs 28 and Radian and no error before. However, I've had weird issues in the past around compilation.

radian-use-package should refrain from expanding the body if the package is disabled... there may be a bug in that logic and how it interacts with native compilation, though.

raxod502 commented 1 year ago

Ok, looking at this more seriously, I can definitely reproduce the error simply by disabling lsp-mode, as you said... there seems to be a bug in radian-use-package or more specifically radian-protect-macros-maybe that fails to protect the sub-forms from byte-compilation processing. Then again, maybe the macros are protected, and the variable definition check is something else entirely. I hope to investigate in a bit.

raxod502 commented 1 year ago

Fixed the Failed to byte-compile: radian.el:2413:41: Warning: reference to free variable ‘radian-lsp-disable’ problem, most likely a lot of others as well, with https://github.com/radian-software/radian/commit/8723da432f1f2d3f9dfc55784f07491bfaf1a23a

raxod502 commented 1 year ago

Take a look at your native compilation error and see if it goes away also with the recent commit.

haji-ali commented 1 year ago

Yep, byte-compiling works now. Thanks!

Native compilation is a separate issue for me (it fails at the outset in the *Async-native-compile-log* buffer). This requires further investigation on my part which I haven't had a chance to do.

I will close the issue for now and reopen it when I have more information

haji-ali commented 1 year ago

Just realized that radian.el has no-native-compile t, so I guess you are not expecting the file to be native-compiled. Not sure why the native-compile-async is trying to native-compile it anyways. Will need more investigations.